1176702 - adblock plus does not work properly(invalid assignment left-hand side)

Status: VERIFIED FIXED

(WebExtensions :: General, defect)

Description

[[e-mail]]

User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:38.0) Gecko/20100101 Firefox/38.0
Build ID: 20150622093246

Steps to reproduce:

Inbound builds
1434981892297	addons.xpi	WARN	Exception running bootstrap method startup on {d10d0bf8-f5b5-c8b4-a8b2-2b9879e08c5d}: ReferenceError: invalid assignment left-hand side (resource://gre/modules/addons/XPIProvider.jsm -> jar:file:///C:/Users/John/AppData/Roaming/Mozilla/Firefox/Profiles/kzk15geq.default/extensions/%7Bd10d0bf8-f5b5-c8b4-a8b2-2b9879e08c5d%7D.xpi!/bootstrap.js -> jar:file:///C:/Users/John/AppData/Roaming/Mozilla/Firefox/Profiles/kzk15geq.default/extensions/%7Bd10d0bf8-f5b5-c8b4-a8b2-2b9879e08c5d%7D.xpi!/lib/ui.js:460:9) JS Stack trace: [e-mail]:137:7 < @main.js:32:1 < [e-mail]:137:7 < [e-mail]:21:3 < [e-mail]:4654:9 < [e-mail]:2375:13 < [e-mail]:221:12 < [e-mail]:703:5 < [e-mail]:871:9 < [e-mail]:2511:5 < [e-mail]:55:7

Comment 1

[Jeff Walden [:Waldo]]

Note documentation of this change -- should be an easy enough fix, although depending how far mis-parenthesization has spread, it might be a bunch of mini-changes to make:

http://whereswalden.com/2015/06/20/new-changes-to-make-spidermonkeys-and-firefoxs-parsing-of-destructuring-patterns-more-spec-compliant/

Comment 2

[Wladimir Palant]

Interesting, I read that blog post but didn't realize that we ever parenthesize destructuring assignments. I added parentheses here to avoid parser errors. I checked and apparently I used a slightly different approach in lib/sync.js:

  ({Service} = Cu.import("resource://services-sync/service.js"));

That one is still legit so I'll use it here as well. Couldn't find any other instances where we had this issue.

Comment 3

[Wladimir Palant]

Created review under https://codereview.adblockplus.org/29321023/.

Comment 4

[Wladimir Palant]

And pushed: https://hg.adblockplus.org/adblockplus/rev/5434ef6e1545

This change will be in development builds within 30 minutes, and it will be part of the next release. Up to you when you want to consider that issue as fixed.

Comment 5

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

Thank you very much for fast fix!

Comment 9

[Cameron McCormack (:heycam)]

Bug 1146136 is in Firefox 41.  Is it likely that ABP's updated version will get reviewed in time?  If not, should we consider backing out bug 1146136?

Comment 10

[Jeff Walden [:Waldo]]

I can't speak to AMO review turnaround times, but it's nearly completely impractical to try to back out that patch.  It touched/refactored/fixed a wide swath of code in the parser in ways that are pretty finicky.  There's no flip-a-switch to this code.  Backing it out would be a drawn-out exercise in very careful work, for which I'd be hesitant to budget any less than a full day's time (and probably more).  Plus I'd have a very hard time justifying the risk/reward tradeoff when requesting uplift approval.  The alternative of attempting to rewrite the current code to emulate the old behavior could well easily be better, but that's even greater risk, because the result wouldn't be a state we've tested/shipped anywhere for any length of time.

If we're going to do anything to address this, it seems much better to expedite the addon review process.  But I'd like to note that it seems pretty problematic if the addon review process takes longer than an entire cycle, because that actively impedes standards-compliance work requiring addon changes, no matter how trivial the change (as here).

Comment 11

[Wladimir Palant]

(In reply to Cameron McCormack (:heycam) from comment #9)
> Bug 1146136 is in Firefox 41.  Is it likely that ABP's updated version will
> get reviewed in time?  If not, should we consider backing out bug 1146136?

Am I missing something? Adblock Plus 2.6.10 came out on July 28th (that's more than a week ago), got reviewed on the same day.

Comment 12

[Jens Glathe]

(In reply to Wladimir Palant from comment #11)
> Am I missing something? Adblock Plus 2.6.10 came out on July 28th (that's
> more than a week ago), got reviewed on the same day.

No, all is fine (from my POV), except maybe for the Element Hiding Helper. This doesn't work with the current Nightly. Good thing to have when it works, though.

with best regards

Jens

Comment 13

[Wladimir Palant]

Element Hiding Helper release is scheduled for next Tuesday (August 11th). Rather extensive changes were required there so feel free to download a development build and help us test it: https://adblockplus.org/development-builds/element-hiding-helper-s-support-for-multiprocess-firefox

Comment 14

[Jens Glathe]

(In reply to Wladimir Palant from comment #13)
Tried to download it, doesn't install. Nightly says something about the AddOn being corrupted. Tried some older builds, too. Same result.

Comment 15

[Wladimir Palant]

I guess you need to turn off xpinstall.signatures.required preference - our development builds haven't been signed by Mozilla.

Comment 16

[Jens Glathe]

(In reply to Wladimir Palant from comment #15)
That was it, thanks! It is installed now and appears to be working.