User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/75.0.3770.80 Safari/537.36

Steps to reproduce:

1. Install the attached unpacked extension.
2. Browse to https://jellby.altervista.org/ebooks/reader.php?epub=Don%20Quijote.epub

Actual results:

The webpage doesn't work correctly, the book cover isn't displayed and the navigation buttons don't work.

The following message is displayed in the console for the page: Loading failed for the <script> with source “blob:https://jellby.altervista.org/72f4ec63-dbf9-4d40-9e72-030f3a5d7a78”. reader.php:1:1

Expected results:

The website works correctly, the book cover is displayed and the navigation links work.

Notes:

• The extension does not break the website on Chrome 75.
• See the related Adblock Plus bug: https://gitlab.com/eyeo/adblockplus/adblockpluschrome/issues/29

Hi @Dave Barker, checked the issue and got the results:
[platform affected]: Windows 10, Ubuntu 18.04, MAC OS X
[Environments]: nightly 70.0a1, beta 69.0b8 & release 68.0.1

• on all machines and FF versions the issue can be reproduced; no cover book will appear, the right-left arrows won't work, on console appears the error message from description.
  Additionally, I've checked on Chrome => the issue cannot be reproduced. therefore, could be related with: 1226547.
  I will set a component, if isn't a proper one please fell free to change it.
  Thanks.

This doesn't seem related to the blob URI, a script with any url seems to produce the same error.

It looks like we might be injecting the content script too early when this kind of operation is not allowed, due to something not being set up yet, but that would need some c++ debugging.

Hey Dave, do you know if this is something ABP started doing recently, or it was always done, but only recently started breaking in Firefox?

I think this is specific to this page, ABP has been using this injection technique since October 2017.

(In reply to Tom Schuster [:evilpie] from comment #3)

I think this is specific to this page, ABP has been using this injection technique since October 2017.

Thanks, I was going to look that commit up this morning but you saved me a job!

The priority flag is not set for this bug.
:jimm, could you have a look please?

For more information, please visit auto_nag documentation.

This doesn't look too widespread, but may indicate a more serious issue in our code, setting P1 for now till I investigate.

Hey Boris, looks like this got regressed by bug 1528146, can you please take a look at what's going on?

The STR from the first comment reliably reproduces, though the Blob URI is a red herring, it breaks when trying to insert a <script> from any url, even a data: URI.

I tried some debugging to figure out what's happening: the stream loading the inserted <script> is closed right after getting opened, from what looks like a call to Cancel(), coming from stopping of all navigation in one of the Document::Open steps, which is confusing because I can't figure out what navigation is happening to get canceled.

(I tried to figure out the expected new document.open() behavior from the related bug 1489308, but soon bumped into limits of my understanding)

The seams to get the document stuck in the "interactive" readyState forever, and the rest of the website is likely broken because of it.

I tried installing the attached extension via the following steps:

1. Load about:addons
2. Click on the "Extensions" tab
3. Click the little gear icon with "V" next to it.
4. Select "Install Add-On From File..."
5. Select the .zip file (I also tried renaming it to .xpi)

I get a message about the addon not being able to be installed because it appears to be corrupt. In the terminal, I see:

 Invalid XPI: Error: File /tmp/test.xpi does not contain a valid manifest(resource://gre/modules/addons/XPIInstall.jsm:669:11) 

I would really appreciate clear steps to reproduce for someone not very familiar with our extension setup.

about:addons is only for signed addons from AMO, use these steps:

0. Download and unpack the .zip file

1. Load about:debugging

2. Click on "This Nightly" in the left column

3. Click "Load Temporary Add-on"

4. Select any file from the directory where you unpacked files (except the .zip file itself)

5. Load https://jellby.altervista.org/ebooks/reader.php?epub=Don%20Quijote.epub

6. Observe the "Loading failed for the <script> with source 'blob:..." error in the web console

Note that two instances of the same extension content script will run, one on the tab page, the other in the inserted iframe. The second instance of the content script trying to insert a <script> element (with the blob url) into the iframe's document is what seems to break loading.

Will that install the extension in my profile, or just for the one session? Ideally I want to install it in the profile, so I can do the install in an opt build but then debug in a debug build... Otherwise it will take tens of minutes to do the install, I expect.

You shouldnt need to unpack it to load in about:debugging.

If testing on nightly, you can also set xpinstall.signatures.required to false and just install the xpi as normal.

Do note that firefox-blob-inject-problem.zip attached to this bug can't be loaded directly without unpacking. This zip file has the common problem of having the manifest.json nested inside another folder, and not directly at the root level of the zip file.

Ideally I want to install it in the profile, so I can do the install in an opt build but then debug in a debug build...

Ah sorry, in that case, here's a .zip you should be able to use from about:addons (in non-release builds) after setting the xpinstall.signatures.required pref to false.

Thank you, that's very helpful!

What's going on here is that the script loader is blocking onload, then not properly unblocking it, so we never fire the load event.

Specifically, the sequence of events is as follows:

1. We start a document load (about:blank in the extension case, just the overall pageload in my testcase).
2. This calls BeginDeferringScripts on the scriptloader, which blocks the load event.
3. The load of the async script (extension-injected, or just script-inserted in my testcase) starts.
4. Document parsing completes, calls ScriptLoader::ParsingComplete with aTerminated == false.
5. This leaves the various script hashtables in place (so in particular mLoadingAsyncRequests.isEmpty() is false), sets mDocumentParsingDone to true, and calls ProcessPendingRequests().
6. ProcessPendingRequest does NOT unblock the load event, because we still have a pending script request. It leaves mDocumentParsingDone true.
7. The document.open call happens, calls BeginDeferringScripts again, which blocks the load event again.
8. The document.close call happens, calls ParsingComplete, but we again don't unblock the load event, because we still have a pending script load. mDocumentParsingDone remains true.
9. The canceled async script load notifies the scriptloader, which calls ProcessPendingRequests and unblocks the load event, finally setting mDocumentParsingDone to false.

So the upshot is that we block the load event twice and only unblock it once, and therefore never fire the load event.

Oh, and the really important part here is that the open() call happen after the parsing of the document is done, so there is no parser termination and corresponding clearing of the scriptloader, but at the same time early enough that the async script has not loaded yet.

Dave, Tomislav, thank you again for the extensions that showed the problem!

Do we need to backport this to branches too?

https://hg.mozilla.org/mozilla-central/rev/46f7b0bdf2a3

(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #19)

Do we need to backport this to branches too?

If it's not too involved, I think it would probably be worth it.

ABP is still the single most popular addon we have, and this is not even restricted to just extensions, but I can't really estimate how widespread it is.

Comment on attachment 9089504 [details]
Bug 1568171. Fix handling of load events if document.open() is called while async scripts are still loading for the original pageload. r=smaug

Beta/Release Uplift Approval Request

• User impact if declined: Some addons (notably ABP) will break some pages.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky): This is pretty finicky code, but as far as I know the only case in which we can reach it is the buggy case observed here.
• String changes made/needed: None.

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Possible web page breakage with addons.
• User impact if declined: Some addons (notably ABP) will break some pages.
• Fix Landed on Version: 71
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky): This is pretty finicky code, but as far as I know the only case in which we can reach it is the buggy case observed here.

I can't think of a safer patch than this one.

• String or UUID changes made by this patch: None

Comment on attachment 9089504 [details]
Bug 1568171. Fix handling of load events if document.open() is called while async scripts are still loading for the original pageload. r=smaug

Fix for regression from 67, adds new tests. We're still in early beta so I'm ok with taking this (for beta 6)

https://hg.mozilla.org/releases/mozilla-beta/rev/bc11f09acf75

When verifying this fix on Windows 10 using the test case from comment 15, it appears unfixed on Beta:

• Nightly v71.0a1 (2019-09-13) (64-bit): <<document.readyState should be "complete", is actually "interactive">>
• Beta v70.0b6 (64-bit): <<document.readyState should be "complete", is actually "interactive">>

Is the test case correct? May there be something wrong with the fix? Please give feedback. Thanks!

The testcase is pretty timing-dependent. Let me post a testcase that should be more reliable.

But note that the real test is installing the extension from comment 13 and then seeing whether the problem reported in comment 0 reproduces.

I have reverified this issue with the test case ic comment 32 on Nightly v71.0a1 from 2019-09-15 and Beta v70.0b6 and it appeared fixed (document.readyState should be "complete", is actually "complete"), while it appeared expectedly not fixed in Release v69.0 (document.readyState should be "complete", is actually "interactive").

Furthermore, I have reproduced the issue on Nightly v71.0a1 (2019-09-09) (64-bit) by installing the extension from comment 13 and then loaded the "https://jellby.altervista.org/ebooks/reader.php?epub=Don%20Quijote.epub" webpage. The book would not be rendered correctly and could not be read. Then I have verified the fix by the same steps on the Nightly v71.0a1 from 2019-09-15 and Beta v70.0b6. The website works correctly, the book cover is displayed and the navigation links work.

Thank you.

Comment on attachment 9089504 [details]
Bug 1568171. Fix handling of load events if document.open() is called while async scripts are still loading for the original pageload. r=smaug

Verified on Nightly and Beta and no known regressions, approved for 68.2esr.

https://hg.mozilla.org/releases/mozilla-esr68/rev/2afcde2a40ea

Also verified in ESR68 v68.2.0esr Build ID: 20190919151825 on Windows 10.