Last Comment Bug 1176702 - adblock plus does not work properly(invalid assignment left-hand side)
: adblock plus does not work properly(invalid assignment left-hand side)
Status: VERIFIED FIXED
[Fixed in Adblock Plus 2.6.9.3955 (dev)]
:
Product: Tech Evangelism
Classification: Other
Component: Add-ons (show other bugs)
: Trunk
: Unspecified Unspecified
-- normal (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
:
:
Mentors:
: 1177643 1181103 1185214 (view as bug list)
Depends on:
Blocks: abp 1146136
  Show dependency treegraph
 
Reported: 2015-06-22 07:08 PDT by zhoubcfan@163.com
Modified: 2015-08-08 05:11 PDT (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments

Description User image zhoubcfan@163.com 2015-06-22 07:08:13 PDT
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: require@bootstrap.js:137:7 < @main.js:32:1 < require@bootstrap.js:137:7 < startup@bootstrap.js:21:3 < XPI_callBootstrapMethod@XPIProvider.jsm:4654:9 < XPI_startup@XPIProvider.jsm:2375:13 < callProvider@AddonManager.jsm:221:12 < _startProvider@AddonManager.jsm:703:5 < AMI_startup@AddonManager.jsm:871:9 < AMP_startup@AddonManager.jsm:2511:5 < AMC_observe@addonManager.js:55:7
Comment 1 User image Jeff Walden [:Waldo] (remove +bmo to email) 2015-06-22 14:17:30 PDT
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 User image Wladimir Palant 2015-06-23 04:04:10 PDT
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 User image Wladimir Palant 2015-06-23 04:17:52 PDT
Created review under https://codereview.adblockplus.org/29321023/.
Comment 4 User image Wladimir Palant 2015-06-23 04:54:20 PDT
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 User image Virtual_ManPL [:Virtual] - (ni? me) 2015-06-23 12:17:05 PDT
Thank you very much for fast fix!
Comment 6 User image Cameron McCormack (:heycam) (away 25 Feb–5 Mar) 2015-06-25 23:30:46 PDT
*** Bug 1177643 has been marked as a duplicate of this bug. ***
Comment 7 User image Guilherme Lima 2015-07-07 19:55:08 PDT
*** Bug 1181103 has been marked as a duplicate of this bug. ***
Comment 8 User image Wladimir Palant 2015-08-07 13:00:29 PDT
*** Bug 1185214 has been marked as a duplicate of this bug. ***
Comment 9 User image Cameron McCormack (:heycam) (away 25 Feb–5 Mar) 2015-08-07 18:36:02 PDT
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 User image Jeff Walden [:Waldo] (remove +bmo to email) 2015-08-07 18:55:36 PDT
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 User image Wladimir Palant 2015-08-08 04:34:19 PDT
(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 User image Jens Glathe 2015-08-08 04:44:39 PDT
(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 User image Wladimir Palant 2015-08-08 04:50:57 PDT
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 User image Jens Glathe 2015-08-08 04:56:00 PDT
(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 User image Wladimir Palant 2015-08-08 05:06:19 PDT
I guess you need to turn off xpinstall.signatures.required preference - our development builds haven't been signed by Mozilla.
Comment 16 User image Jens Glathe 2015-08-08 05:11:15 PDT
(In reply to Wladimir Palant from comment #15)
That was it, thanks! It is installed now and appears to be working.

Note You need to log in before you can comment on or make changes to this bug.