Url: github.com/...user.js - error in the Browser Console

[janekptacijarabaci]

Example:
https://github.com/w35l3y/userscripts/blob/master/scripts/Neopets_Price_Checker/112692.user.js

Browser console:
#1875 (comment)
(line 278)

[Ventero]

Some additional information so that I don't forget it:

RemoteScript#cleanup is called three times in cases like that. The three code paths are roughly the following:

1. Once we detect the page is actually an HTML page, the DownloadListener's completion callback is called. This callback is RemoteScript#_downloadScriptCb, which calls RemoteScript#cleanup
2. Inside cleanup, all download channels are cancelled, which leads to the download listener's onStopRequest being called with a status code indicating an error. Thus, it calls cleanup itself directly.
3. Immediately afterwards, onStopRequest additionally calls the completion callback, which as I already mentioned goes through _downloadScriptCb and lands in cleanup.

I'd say number 1 and 2 are definitely unnecessary: Instead of calling the completion handler explicitly in onStartRequest we should just cancel the request and let onStopRequest handle the rest. Additionally, the download listener shouldn't really have to know if the script needs to be cleaned up and just let _downloadScriptCb decide.

Unless someone objects to this or has a better idea, I'll send a patch with these two changes.

PS: In my opinion, the whole script installation process might need some refactoring, as all these callbacks make the code very hard to follow. I understand that script installation is an inherently asynchronous process, but I think by dividing up the responsibilites a bit cleaner, the code can maybe be simplified a bit.

[Ventero]

Ventero@92cbf36 should fix this issue. I tested the following cases with this patch, which were all handled fine:

• successful installation
• script itself 404's
• require/resource dependency 404's
• icon 404's

Additionally, Ventero@b8a0936 fixes a small issue with an XPCOM status check.

[arantius]

This fix is now included in version 3.1beta1. Please browse to Greasemonkey's AMO page and open "Development Channel" near the bottom to install the beta version. Can you confirm the fix? Thank you.

[janekptacijarabaci]

Unfortunately, there is still something wrong.

Example:
https://github.com/w35l3y/userscripts/blob/master/scripts/Neopets_Price_Checker/112692.user.js
(F5 page reload)

Browser console:

[arantius]

Moving to 3.2 to fix whatever bits remain.

[arantius]

Ah I totally missed one detail!

So https://developer.mozilla.org/en-US/Firefox/Multiprocess_Firefox/Frame_script_environment says " docShell The nsIDocShell associated with the browser". And https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Reference/Interface/nsIDocShell#loadURI says there's a loadURI() method which takes four arguments. But docShell also (??) is a nsIWebNavigation https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Reference/Interface/nsIWebNavigation#loadURI%28%29 which has a loadURI() method which takes five arguments!