Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Slideshow pane breaks on CSS #301

Closed
jeff-zucker opened this issue Aug 27, 2021 · 17 comments · Fixed by #344
Closed

Slideshow pane breaks on CSS #301

jeff-zucker opened this issue Aug 27, 2021 · 17 comments · Fixed by #344

Comments

@jeff-zucker
Copy link
Contributor

jeff-zucker commented Aug 27, 2021

CSS does not advertise the media-type of contained resources so when slideshowPane looks for images, it finds none since it is looking in the container for triples listing the item as of type ...iana/image... and CSS does not provide such triples in the container. I suggest we use the mime-types library to examine the path extension rather than looking for triples.

@jeff-zucker
Copy link
Contributor Author

jeff-zucker commented Aug 28, 2021

I mean mime-types media-types, not content-type, but the issue is the same. This and related issues about containers are being discussed at solid/specification#227.

@TallTed
Copy link
Contributor

TallTed commented Sep 1, 2021

You actually mean media types in today's language, which were formerly known as mime types.

I suggest editing your original post (editing is available through the three-dots at the upper right of your comments) to reflect your correction, as this will help future readers more quickly understand the issue.

@josephguillaume
Copy link
Contributor

The media type now seems to be correct, e.g. CSS is providing the container triples <View_from_balcony3.jpg> a ldp:Resource, <http://www.w3.org/ns/iana/media-types/image/jpeg#Resource>; dc:modified "2014-12-30T01:12:47.000Z"^^xsd:dateTime. and the slideshow pane is activated.

However, slideshow pane is still broken on CSS for private images because it gets a 401 Unauthorized for every image, i.e. the DPOP token is not being provided in the GET request.
And that's obviously because the slideshow pane simply inserts an img element, and on NSS this currently works because requests are still authenticated with a same-site cookie.
https://github.com/solid/solid-panes/blob/6f5fa4b9ccfc8dae13689bc5bc5009db783b04fb/src/slideshow/slideshowPane.js#L59-L60

@josephguillaume
Copy link
Contributor

I suggest we move this to using an authenticated fetch of the image data and set it as a blob as recommended by Ruben in the linked issue above and discussed at https://forum.solidproject.org/t/how-to-add-inline-content-in-an-authenticated-solid-app/4929/5
i.e. something along the lines of

const response = await fetch(src);
const blob = await response.blob();
imageData = URL.createObjectURL(blob);
img.setAttribute('src', imageData) 

If this sounds ok, I'll put a PR on my todo list.

Note that the original issue is addressed in CSS by adding the MetadataExtender
https://github.com/solid/community-server-recipes/blob/b38888064d1ee666ee475d85e8becc1ccab65d37/mashlib/config-mashlib.json#L94
I think it is safe to assume that a CSS+Mashlib instance will be using this config, but if Mashlib is used to visit an external CSS wihout MetadataExtender then images still won't be found.

@bourgeoa
Copy link
Contributor

bourgeoa commented Dec 10, 2021

  • does the problem arise also in audio, videos ?
  • Is there a possibility to use crossOrigin like ?
const img = figure.appendChild(dom.createElement('img')) 
img.setAttribute( 'crossOrigin', 'use-credentials')
img.setAttribute('src', images[i].uri) 

@josephguillaume
Copy link
Contributor

The underlying problem is that we need to use the authenticated fetch function provided e.g. by solid-client-authn

The problem therefore applies to any inline content that does not know that the authenticated fetch exists.

As far as I understand, include-credentials does not provide any mechanism to specify that an arbitrary authenticated function should be used, so it would only become a solution with browser support.
The example you cite would somehow need to specify what the authenticated fetch is.

Btw, it needs to be a function, not fixed credentials because the new authentication generates a new dpop token for every request.

@bourgeoa
Copy link
Contributor

To my knowledge the old authentication already produced an authenticated-fetch.
The only new thing is that there are no cookie and we need to indicate to the browser to use the credentials.

@jeff-zucker have you any hints.

And will the blob work for sized videos or music.

@josephguillaume
Copy link
Contributor

Sorry, I may have misunderstood. Do you want to support videos and/or audio in the slideshow pane?
My understanding is that currently only http://purl.org/dc/dcmitype/Image is supported:
https://github.com/solid/solid-panes/blob/6f5fa4b9ccfc8dae13689bc5bc5009db783b04fb/src/slideshow/slideshowPane.js#L57
https://github.com/solid/solid-ui/blob/5d46e60a66efe156ba585d4261c18318ae0ada4c/src/widgets/buttons.ts#L1402

With the old authentication the slideshow pane did not use the authenticated fetch. It relied on the cookie.
So in principle, yes, the change I am suggesting would be backwards compatible because an authenticated fetch using the old authentication could also be used.

@timea-solid
Copy link
Member

Trying to understand the problem I checked recently my css pod in mashlib and my image loads both on Firefox and Chrome.
grafik
I see in the console a log like "humanReadablePane: c-t:image/jpeg" which comes from the code at: https://github.com/solid/solid-panes/blob/d34c15d9aacf3eceb520930cc424e3b60f869769/src/humanReadablePane.js#L79
@jeff-zucker can it be that with the fix of the fetcher problem this is also fixed?

@josephguillaume
Copy link
Contributor

Hi @theRealImy, is that a public or private image?

@jeff-zucker
Copy link
Contributor Author

@theRealImy - I suppose that it's possible the fetch fix fixed this, I don't have any evidence either way.

@timea-solid
Copy link
Member

Hi @theRealImy, is that a public or private image?

@josephguillaume it is public.
If the picture is private, I get a 401 even when I am logged in. So, the behavior has changed but it is still not the desired outcome.

@josephguillaume
Copy link
Contributor

Thanks for confirming. It sounds like the state is still the same as in my earlier comment: #301 (comment)

@timea-solid
Copy link
Member

@josephguillaume ok, I understand. It is the same problem and your/Ruben's proposed solution is good! Could you plan a PR -> I will test it then.

@jeff-zucker
Copy link
Contributor Author

I do not understand why this issue was closed. As far as I can see the original problem still exists - CSS does not list the media-type of resources in the GET of a container so therefore it is not possible to e.g. generate a slideshow from a list of videos because we don't know from the container which are videos. Mashlib could get around this by checking file by file but that seems inordinately cumbersome. @bourgeoa - am I missing something?

@bourgeoa
Copy link
Contributor

bourgeoa commented Jul 17, 2022

There where 2 issues. One related to authenticated fetch (I think it was resolved, and a second one related to being able to get media-types in Get container.

This last point could be described in a new issue with some caveats.

Using mashlib on a pod should imply the pod to run a mashlib compatible filesystem. In such situation the media-types should be available, this is what does CSS recipes for mashlib in v4.0.
It is not a MUST in solid 0.9 specification.

To my understanding the media-types will be included by default in CSS with a filesystem on version 5.0.
To be confirmed. @joachimvh

@jeff-zucker
Copy link
Contributor Author

@bourgeoa, @joachimvh - I can confirm that v4.0.1 of the community server DOES provide media-types in the container listing when used with the mashlib recipe configuration and that the databrowser slideshow works on CSS. Closing the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants