[Webkit-unassigned] [Bug 237513] [GTK][WPE] Add initial support for PDF documents using PDF.js

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Mar 8 06:41:19 PST 2022


https://bugs.webkit.org/show_bug.cgi?id=237513

--- Comment #12 from Carlos Garcia Campos <cgarcia at igalia.com> ---
(In reply to Michael Catanzaro from comment #11)
> (In reply to Carlos Garcia Campos from comment #7)
> > It's enabled unconditionally for GTK and WPE. We can discuss if we want to
> > add api to enable/disable it, though. But for now, I think it's easier this
> > way. Cocoa doesn't do this because they have :
> > 
> > // Allow images to load.
> > response.addHTTPHeaderField(HTTPHeaderName::AccessControlAllowOrigin, "*");
> > 
> > in the bundle resource loader. I  did it this way for consistency with
> > gresources, since webkit-pdfjs-viewer are gresources too in the end.
> 
> Hmmm, OK, I see why you did this. I'm not actually certain, but I *suspect*
> this wasn't the best approach. There are a few other considerations:
> 
>  * A webkit-pdf-viewer: URI has a host component and therefore it should
> probably not be treated as a unique origin, i.e. shouldTreatAsUniqueOrigin
> should return false (...right?). Your change accomplishes this only for GTK
> and WPE, but leaves Cocoa ports with the opposite result. Understanding that
> Cocoa ports don't *need* it for PDF.js to work, I think they still *want*
> this. (I assume LegacySchemeRegistry::schemeIsHandledBySchemeHandler will
> always return true for webkit-pdf-viewer, causing shouldTreatAsUniqueOrigin
> to return false if we don't have a special case for webkit-pdfjs-viewer
> URIs.)
> 
>  * Matching the Cocoa port behavior allows us to share the same set of bugs
> between ports, which is valuable.
> 
>  * You mention you are seeing CORS errors in the console, but surely those
> will go away if we add AccessControlAllowOrigin=* to match Cocoa ports, so
> we should probably do that too, right? In fact, I wonder if maybe we should
> do that for all GResources. We spent ages trying to make PDF.js dodge CORS,
> and even wound up adding new public API webkit_web_view_set_cors_allowlist()
> to facilitate it. None of that would have been needed if GResources just did
> the right thing. Resource content should be trusted and allowed to embed
> content from any origin it wants, right?

No, the initial CORS errors prevented the resourced from being loaded, those are fixed either adding the HTTP header or making the scheme as not unique. The errors I'0m still seeing happen with the HTTP header too. I'll submit a new bug for those.

> So even if your way works, I think it'd be nice to match behavior between
> ports anyway. Normally we use #if only when something either *has* to be
> different between ports, or at least really *wants* to be different. I don't
> think that's the case here. I won't block you on this, though (you still
> have r+ regardless).

I'm fine switching to cocoa behavior, I haver to check it also works for the inspector resources.

> > I think it's unlikely new file types are added in new versions. I've tried
> > hard to not add any file we don't need to the bundle, because it's already
> > big enough. If it breaks after an upgrade we can easily fix it.
> 
> What files do we not need besides the *.map and the example PDF? If you want
> to maintain an allowlist of file extensions, then please maintain a denylist
> as well, and fail the build if a file doesn't match one list or the other,
> so we'll be sure to notice when something is wrong. It's not much extra code
> now, but will make things easier for us in the future if it's really needed.
> 
> I'm sure whoever winds up updating PDF.js now that it has moved to WebKit is
> not going to want to deal with important files discarded with no warning...
> *especially* as that will only happen for GTK/WPE, so someone from Apple
> would really have to chance to avoid breaking us if there is no build
> failure.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20220308/5d315c1f/attachment-0001.htm>


More information about the webkit-unassigned mailing list