1317340 - IRC URL links are broken in Firefox 38 when adblockplus is installed

Status: RESOLVED FIXED

(Other Applications :: ChatZilla, defect)

Description

[James Ross]

I am betting this is because of bug 1095484 but either way, a user on Firefox 38 can no longer open bookmarks to irc: as of ChatZilla 0.9.93.

Version: ChatZilla 0.9.93 [Firefox 38.0.5/20150525141253]​​”
Host: Mozilla Firefox 38.0.5, Windows
OS: Windows NT 6.1 (Win32)

Comment 1

[James Ross]

May or may not be related, but dropping the links from bookmarks onto ChatZilla shows the error:

> Invalid IRC URL ``​irc://bla.net/##whatever''.

Comment 2

[:Gijs (he/him)]

FTR, 38 is no longer supported, 45 is the most recent ESR and 52 (the next one) is branching to aurora today.

Is it also broken on a more recent version of Firefox if you turn off e10s? Because that would be more worrying...

Comment 3

[James Ross]

Gecko/Firefox 38 was selected as the most recent LTS/ESR that included current SeaMonkey releases.

Firefox 49.0.2 with Multiprocess Windows = 0/1 (Disabled by add-ons) opens ChatZilla just fine from IRC URL links.

Comment 4

[James Ross]

Hmm, so in my testing:
- 38.0.1esr works
- 38.1.0esr works
- 38.5.1esr works
- 38.5.2esr works
- 38.0.5 works

So I'm not sure if there is actually a bug here, and instead maybe the user just had a startup-cache problem or something.

Comment 5

[:Gijs (he/him)]

Let's mark incomplete, and we can reopen if we get more reports / more details.

Comment 6

[James Ross]

Further information from the user indicates that the problem only occurs when Adblock Plus is installed (version 2.8.1 in this case, but probably not dependent on that version specifically).

Comment 7

[Wladimir Palant]

Adblock Plus 2.8.2 changed handling of "unusual" protocol schemes so the version number might not be irrelevant.

Comment 8

[James Ross]

Per comments from the user on IRC, this bug occurs with Adblock Plus 2.7.3 but not 2.7.2. No betas in-between were tested.

I had a brief look at the commits on https://github.com/adblockplus/adblockplus/compare/2.7.2...2.7.3 but obviously am unfamiliar with the codebase. Wladimir, anything stand out in that range?

Comment 9

[Wladimir Palant]

I was able to reproduce that issue with Firefox 38 (not SeaMonkey 2.40). There is an error at startup:

> ReferenceError: ChatZillaProtocols is not defined
> chatzilla-protocol-script.js:11:0

That's a process script introduced in bug 1095484 and the problematic code is:

> Cu.import("chrome://chatzilla/content/lib/js/protocol-handlers.jsm");
> ChatZillaProtocols.init();

This looks like scoping issues. I changed it as follows:

> let {ChatZillaProtocols} = Cu.import("chrome://chatzilla/content/lib/js/protocol-handlers.jsm", {});
> ChatZillaProtocols.init();

With this change it works correctly. Not sure how exactly it clashes with Adblock Plus but Adblock Plus uses process scripts as well - and all process scripts run in the same global scope. So messing with the global scope in a process script is generally a bad idea.

Comment 10

[:Gijs (he/him)]

Attachment: define local variables for imported protocol-handlers.jsm,

Comment 11

[James Ross]

Comment on attachment 8815681 [details] [diff] [review]
define local variables for imported protocol-handlers.jsm,

Review of attachment 8815681 [details] [diff] [review]:
-----------------------------------------------------------------

Hmm, odd that using the MDN common syntax would screw up (or maybe not?) but this change looks fine.

::: js/lib/chatzilla-protocol-script.js
@@ +4,5 @@
>   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
>  
>  const { utils: Cu } = Components;
>  
> +let {ChatZillaProtocols} = Cu.import("chrome://chatzilla/content/lib/js/protocol-handlers.jsm", {});

Nit: Style is let { ChatZillaProtocols } I think?

Comment 12

[:Gijs (he/him)]

 https://hg.mozilla.org/chatzilla/rev/2d773c6bb6851f545034db27d60346df4beeb490

Comment 13

[Frank-Rainer Grahl (:frg)]

For what's it worth. This now fails in SeaMonkey 2.50 in a new profile. I didn't try a fresh Firefox yet. Was fine before in ESR 45 and up. 

> Error: TypeError: ChatZillaProtocols is undefined
> Source File: chrome://chatzilla/content/lib/js/chatzilla-protocol-script.js Line: 10

Comment 14

[:Gijs (he/him)]

(In reply to Frank-Rainer Grahl from comment #13)
> For what's it worth. This now fails in SeaMonkey 2.50 in a new profile. I
> didn't try a fresh Firefox yet. Was fine before in ESR 45 and up. 
> 
> > Error: TypeError: ChatZillaProtocols is undefined
> > Source File: chrome://chatzilla/content/lib/js/chatzilla-protocol-script.js Line: 10

And it didn't fail before this change? What version of Gecko is 2.50 running? :-\

Comment 15

[Frank-Rainer Grahl (:frg)]

>> And it didn't fail before this change?

No. I wondered why only in a new profile but this is because it uses the bundled cZ with the changeset there. My other profiles still had the one from amo or an older version before the change in it.

2.50 is FF 53/Gecko 53 aka bleeding edge mozilla-central.

Comment 16

[Frank-Rainer Grahl (:frg)]

To confirm it I backed out the bug locally. Error is gone and I was again also able to open cZ clicking on an IRC link.

Comment 17

[:Gijs (he/him)]

(In reply to Frank-Rainer Grahl from comment #15)
> >> And it didn't fail before this change?
> 
> No. I wondered why only in a new profile but this is because it uses the
> bundled cZ with the changeset there. My other profiles still had the one
> from amo or an older version before the change in it.
> 
> 2.50 is FF 53/Gecko 53 aka bleeding edge mozilla-central.

Odd, it works for me on current Firefox Nightly.

Are there any other errors? Does it work if you turn e10s on/off (assuming SeaMonkey supports that)? Is SeaMonkey doing anything particular about how it loads JS components (in particular, the js script version it uses for doing so, or something) ?

Comment 18

[:Gijs (he/him)]

Never mind, I misread comment #16. I can reproduce. There's a scoping problem. Making ChatZillaProtocols in the jsm use 'var' rather than 'const' fixes it. Looking at when this changed...