Bug 251887

(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.

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

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.

<rdar://problem/105469701>

(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.

(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.

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.

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?

(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();
```

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.

(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.

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

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

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