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

VERIFIED FIXED

Status

Tech Evangelism
Add-ons
VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: zhoubcfan@163.com, Unassigned)

Tracking

(Blocks: 1 bug)

Trunk
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [Fixed in Adblock Plus 2.6.9.3955 (dev)])

(Reporter)

Description

2 years ago
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
(Reporter)

Updated

2 years ago
Blocks: 1146136
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/
Component: General → Add-ons
Product: Core → Tech Evangelism
Blocks: 467520
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 2

2 years ago
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

2 years ago
Created review under https://codereview.adblockplus.org/29321023/.

Comment 4

2 years ago
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.
Thank you very much for fast fix!
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Whiteboard: [Fixed in Adblock Plus 2.6.9.3955 (dev)]
Status: RESOLVED → VERIFIED
Duplicate of this bug: 1177643

Updated

2 years ago
Duplicate of this bug: 1181103

Updated

2 years ago
Duplicate of this bug: 1185214
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?
Flags: needinfo?(jwalden+bmo)
Flags: needinfo?(jorge)
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).
Flags: needinfo?(jwalden+bmo)

Comment 11

2 years ago
(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

2 years ago
(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

2 years ago
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
Flags: needinfo?(jorge)

Comment 14

2 years ago
(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

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

Comment 16

2 years ago
(In reply to Wladimir Palant from comment #15)
That was it, thanks! It is installed now and appears to be working.
You need to log in before you can comment on or make changes to this bug.