Bug 251887 – REGRESSION(253223@main): PDF.js viewer fails to load PDF: [Error] TypeError: null is not an object (evaluating 'PDFViewerApplication.eventBus.on')

• Status: RESOLVED
• Resolution: FIXED
• Component: PDF

[Michael Catanzaro]

Opening a PDF document in a new tab (Ctrl+click) generally works reliably, but opening them in the current tab does not work anymore. Not sure when this broke. For example, visit https://dor.mo.gov/forms/?formName=&category=&year=99 and left-click on any PDF link to open in the current tab. The web inspector will contain several errors, the first of which is:

[Error] webviewerloaded: SecurityError: Blocked a frame with origin "webkit-pdfjs-viewer://pdfjs" from accessing a cross-origin frame. Protocols, domains, and ports must match.

	webViewerLoad (viewer.js:13878)

Frustratingly, that error message does not show the name of the target frame, but I added a debug print in WebKit to show it:

crossDomainAccessErrorMessage: Blocked a frame with origin "webkit-pdfjs-viewer://pdfjs" from accessing a cross-origin frame. : target=https://dor.mo.gov

Somehow the URI scheme must not be fully exempt from CORS.

[Michael Catanzaro]

(In reply to Michael Catanzaro from comment #0)
> Opening a PDF document in a new tab (Ctrl+click) generally works reliably

Um, not even this seems to be true. There is some race condition I guess. Sometimes it works, and sometimes not.

[Michael Catanzaro]

Oh and if opening the document does work successfully, then press Back and try to open the document again. Second time it should fail.

[Michael Catanzaro]

The failing check is in canAccessDocument in JSDOMBindingSecurity.cpp:

active.document()->securityOrigin().isSameOriginDomain(targetDocument->securityOrigin())

Here the current document's origin is webkit-pdfjs-viewer://pdfjs while the target document's origin is https://dor.mo.gov (in the example from the first comment). The check always fails when loading any PDF document, but it only sometimes results in failure to display the document.

I guess expected behavior is for webkit-pdfjs-viewer:// to be treated as internal and have superpower to access all domains. (This is how the obsoleted ephy-pdf-viewer:// worked.)

If I try with 2.38.3, then the same check never fails, so something regressed. I'll attempt to bisect it.

[Radar WebKit Bug Importer]

<rdar://problem/105469701>

[Michael Catanzaro]

(In reply to Michael Catanzaro from comment #3)
> I'll attempt to bisect it.

It broke in 253223@main "Introduce postMessage mechanism for PDF.js viewer and use it for context menu items". Need to look closer at what it's doing.

[Michael Catanzaro]

(In reply to Michael Catanzaro from comment #0)
> [Error] webviewerloaded: SecurityError: Blocked a frame with origin
> "webkit-pdfjs-viewer://pdfjs" from accessing a cross-origin frame.
> Protocols, domains, and ports must match.
> 
> 	webViewerLoad (viewer.js:13878)

Amazingly this was a preexisting problem from bug #237643. It's not responsible for the failure to display PDF content.

[Michael Catanzaro]

OK the real problem here is actually:

[Error] TypeError: null is not an object (evaluating 'PDFViewerApplication.eventBus.on')

	init (content-script.js:60)
	Global Code (content-script.js:108)

I improperly assumed that I should be laser-focused on the first error that I saw in the web inspector. This error occurs only when PDF load fails and not when it succeeds.

[Michael Catanzaro]

Created attachment 465033 [details]
Screenshot providing the missing variable does exist

So if I select viewer.html in the bottom-right corner of the web inspector, then attempt to evaluate PDFViewerApplication.eventBus.on, it does work. So I assume PDFViewerApplication gets created at the wrong time, after the content script runs.

I don't know how to fix this. Hi Tim, how amenable would you be to a revert for now?

[Tim Nguyen (:ntim)]

(In reply to Michael Catanzaro from comment #8)
> Created attachment 465033 [details]
> Screenshot providing the missing variable does exist
> 
> So if I select viewer.html in the bottom-right corner of the web inspector,
> then attempt to evaluate PDFViewerApplication.eventBus.on, it does work. So
> I assume PDFViewerApplication gets created at the wrong time, after the
> content script runs.
> 
> I don't know how to fix this. Hi Tim, how amenable would you be to a revert
> for now?

I'd prefer to not revert, there's a bunch of commits on top, and it's also what allows the custom context menu to work (which WebKitGTK might have as well fwiw).

It sounds like there is some kind of race condition where the content script is injected before `PDFViewerApplication.eventBus` is initialized from the error you're seeing. Figuring out why that is the case would be nice.

The content-script.js file is injected when the iframe loads: https://searchfox.org/wubkat/rev/cccd00688012d25c2cac5d6aa11b86f937927770/Source/WebCore/html/PDFDocument.cpp#117

Something you could try is:
```
        if (PDFViewerApplication.eventBus) {
            PDFViewerApplication.eventBus.on("pagesinit", () => {
                this.autoResize();
            });
        } else
            this.autoResize();
```

[Michael Catanzaro]

We need to delay the entire init() function.

I found documentation on what we're supposed to do here:

https://github.com/mozilla/pdf.js/wiki/Third-party-viewer-usage#initialization-promise

but it doesn't work, and I can't figure out how to make it work properly, so I'll try using a timeout instead.

[Michael Catanzaro]

(In reply to Michael Catanzaro from comment #10)
> We need to delay the entire init() function.

Well, actually window.addEventListener() really does need to be connected to immediately. So I came up with this:

    init() {
        let id = setInterval(() => {
            if (PDFViewerApplication.initialized) {
                this.overrideSettings();
                PDFViewerApplication.eventBus.on("pagesinit", () => {
                    this.autoResize();
                });
                clearInterval(id);
            }
        }, 200);

That seems to work properly.

[Michael Catanzaro]

Pull request: https://github.com/WebKit/WebKit/pull/10281

[EWS]

Committed 260485@main (f6ae4b26c602): <https://commits.webkit.org/260485@main>

Reviewed commits have been landed. Closing PR #10281 and removing active labels.