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

[bugzilla-daemon at webkit.org]

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

--- Comment #4 from Michael Catanzaro <mcatanzaro at gnome.org> ---
Comment on attachment 453949
  --> https://bugs.webkit.org/attachment.cgi?id=453949
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=453949&action=review

Do you plan to investigate adapting Epiphany for this? We'll be able to remove a lot of code there....

> Source/WebCore/loader/ResourceLoader.cpp:872
> -#if PLATFORM(COCOA)
> +#if PLATFORM(COCOA) || PLATFORM(GTK) || PLATFORM(WPE)

No need for #if here: we have a runtime setting PdfJSViewerEnabled, so should check that instead.

> Source/WebCore/page/SecurityOrigin.cpp:124
> +        || url.protocolIs("webkit-pdfjs-viewer")

This is surely required for all ports using PDF.js, not just GTK and WPE. Better check PdfJSViewerEnabled instead of using #if.

> Source/WebCore/platform/LegacySchemeRegistry.cpp:148
> -#if PLATFORM(COCOA)
> +#if PLATFORM(COCOA) || PLATFORM(GTK) || PLATFORM(WPE)
>          "webkit-pdfjs-viewer"_s,
>  #endif

If you change this from std::array to std::vector, then you can check the runtime setting PdfJSViewerEnabled here too.

> Source/WebKit/UIProcess/WebPageProxy.cpp:1915
> +    if (m_preferences->pdfJSViewerEnabled() && MIMETypeRegistry::isPDFMIMEType(mimeType))
> +        return true;

Note: this requires owner approval to land.

> Source/WebKit/WebProcess/WebPage/WebPage.cpp:6460
> +    if (m_page->settings().pdfJSViewerEnabled() && MIMETypeRegistry::isPDFMIMEType(mimeType))
> +        return true;

This too.

> Tools/glib/generate-pdfjs-gresource-manifest.py:35
> +        if os.path.splitext(resource)[1] not in VALID_EXTENSIONS:
> +            return True

Hm, I don't like this: we'll likely drop desired files by mistake during future updates of PDF.js. I'd either (a) drop this altogether and just include everything, or (b) use a denylist containing just '.map' as a sanity-check to ensure we don't include those pre-generated map files by mistake.

> Tools/glib/generate-pdfjs-gresource-manifest.py:54
> +            # separator, is properly replaced

Nit: no comma here

-- 
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/20220307/15bcac7e/attachment.htm>