Closed Bug 1317340 Opened 4 years ago Closed 4 years ago

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

Categories

(Other Applications :: ChatZilla, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: bugzilla-mozilla-20000923, Assigned: Gijs)

References

Details

(Keywords: regression)

Attachments

(1 file)

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)
May or may not be related, but dropping the links from bookmarks onto ChatZilla shows the error:

> Invalid IRC URL ``​irc://bla.net/##whatever''.
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...
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.
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.
Let's mark incomplete, and we can reopen if we get more reports / more details.
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → INCOMPLETE
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).
Status: RESOLVED → REOPENED
Resolution: INCOMPLETE → ---
Summary: IRC URL links are broken in Firefox 38 → IRC URL links are broken in Firefox 38 when adblockplus is installed
Adblock Plus 2.8.2 changed handling of "unusual" protocol schemes so the version number might not be irrelevant.
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?
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.
Attachment #8815681 - Flags: review?(bugzilla-mozilla-20000923)
Assignee: bugzilla-mozilla-20000923 → gijskruitbosch+bugs
Status: REOPENED → ASSIGNED
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?
Attachment #8815681 - Flags: review?(bugzilla-mozilla-20000923) → review+
 https://hg.mozilla.org/chatzilla/rev/2d773c6bb6851f545034db27d60346df4beeb490
Status: ASSIGNED → RESOLVED
Closed: 4 years ago4 years ago
Resolution: --- → FIXED
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
(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? :-\
Flags: needinfo?(frgrahl)
>> 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.
Flags: needinfo?(frgrahl)
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.
(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) ?
Flags: needinfo?(frgrahl)
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...
Flags: needinfo?(frgrahl)
Blocks: 1324217
You need to log in before you can comment on or make changes to this bug.