Closed Bug 1212707 Opened 9 years ago Closed 9 years ago

NoScript has stopped being able to unblock sites on the latest Firefox trunk (Nightly)

Categories

(Core :: JavaScript Engine, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Tracking Status
firefox44 --- affected

People

(Reporter: cks+mozilla, Unassigned)

Details

The trunk version of Firefox recently broke NoScript, such that NoScript's
stopped being able to unblock sites. If you visit a site with blocked
JavaScript, such as www.google.com (if you have that blocked), the
NoScript menu (either in the general content button-3 popup menu or from
its button) no longer populates with the site name and attempts to do
things like temporarily unblock the site don't do anything.

Inspection of the Browser Console shows a stream of errors of the form:

	TypeError: this.dom is undefined noscriptService.js:6301:11
	TypeError: this.dom is undefined noscriptService.js:6301:11
	TypeError: this.dom is undefined noscriptService.js:6301:11
	TypeError: this.dom is undefined noscriptService.js:6301:11
	TypeError: ns.dom is undefined noscriptOverlay.js:2491:19
	TypeError: this.dom is undefined noscriptService.js:6301:11

All of these are NoScript JS code of the form:
	var docShell = ns.dom.getDocShellForWindow(content);

(sometimes other functions are called)

Possibly this is incorrect code in NoScript to start with, but NoScript
is a rather popular extension.

Due to tree build problems in various revisions, the closest 'hg bisect'
could narrow this down to is:

Due to skipped revisions, the first bad revision could be any of:
changeset:   266443:3bcc3881b95d
user:        Shu-yu Guo <shu@rfrn.org>
date:        Tue Oct 06 14:00:28 2015 -0700
summary:     Bug 589199 - Make a global lexical scope and hook it up to JS entry points. (r=efaust)

changeset:   266444:b9f647fe2d10
user:        Shu-yu Guo <shu@rfrn.org>
date:        Tue Oct 06 14:00:29 2015 -0700
summary:     Bug 589199 - Parse and emit bytecode for global lexicals. (r=efaust)

changeset:   266445:b2d8f1cd3afc
user:        Shu-yu Guo <shu@rfrn.org>
date:        Tue Oct 06 14:00:29 2015 -0700
summary:     Bug 589199 - Support global lexicals in the interpreter. (r=efaust)

changeset:   266446:6e4a8943d496
user:        Shu-yu Guo <shu@rfrn.org>
date:        Tue Oct 06 14:00:29 2015 -0700
summary:     Bug 589199 - Support global lexicals in Baseline. (r=jandem)

changeset:   266447:26fc971a24c4
user:        Shu-yu Guo <shu@rfrn.org>
date:        Tue Oct 06 14:00:29 2015 -0700
summary:     Bug 589199 - Support global lexicals in Ion. (r=jandem)

changeset:   266448:3228ac384c92
user:        Shu-yu Guo <shu@rfrn.org>
date:        Tue Oct 06 14:00:29 2015 -0700
summary:     Bug 589199 - Fix eval static scope to play with the global lexical scope. (r=efaust)

changeset:   266449:04eb8f524122
user:        Shu-yu Guo <shu@rfrn.org>
date:        Tue Oct 06 14:00:29 2015 -0700
summary:     Bug 589199 - Fix up the global lexical scope when merging off-thread compiled scripts. (r=bhackett)

changeset:   266450:c609df6d3895
user:        Shu-yu Guo <shu@rfrn.org>
date:        Tue Oct 06 14:00:29 2015 -0700
summary:     Bug 1202902 - Support non-syntactic extensible lexical scopes. (r=billm)
I'm hitting this in latest nightly, as foretold in comment 0.  (But only with e10s *disabled*.)

STR:
 1. Install NoScript from green "INSTALL" button at upper-left of https://noscript.net/
    (Restart to complete installation)
 2. Disable e10s in about:preferences.
    (Restart to complete e10s disabling)
 3. Visit e.g. http://blizzard.com/ and then click the NoScript icon on the toolbar, or its "Options" button on the info-bar that appears.

EXPECTED RESULTS: "Allow blizzard.com" should appear somewhere in the menu.
ACTUAL REUSLTS: That menu item is missing.

ni=shu given regression range in comment 0 (though it's possible the change is just exposing a bug in the addon, too)
Component: General → JavaScript Engine
Flags: needinfo?(shu)
Product: Firefox → Core
Summary: NoScript has stopped being able to unblock sites on the latest Firefox trunk (soon to be Nightly) → NoScript has stopped being able to unblock sites on the latest Firefox trunk (Nightly)
Actually, with e10s enabled, the menu entry shows up *but* it seems to have no effect if I click it.

A better test page is https://talky.io/ which just renders as blank if scripts are disabled.
NoScript's use of 'const' is no longer spec compliant. NoScript also seems to have written its own loading logic like INCLUDE and LAZY_INCLUDE:

 29 function LAZY_INCLUDE(name) {
 30   if (arguments.length > 1)
 31     for (var j = 0, len = arguments.length; j < len; j++)
 32       arguments.callee(arguments[j]);
 33   else if (!(name in this)) {
 34     __defineGetter__(name, function() {
 35       delete this[name];
 36       // dump(name + " kickstarted at " + (new Error().stack));
 37       INCLUDE(name);
 38       return this[name];
 39     });
 40   }
 41 }

The this.dom issue is because the global property "DOM", which the getter "dom()" uses, is LAZY_INCLUDE'd, which expects the "const DOM" defined in content/DOM.js to be a property.

What is this NI for? To fix NoScript?
Flags: needinfo?(shu)
(Clearly not. :)  The needinfo=you was to see if you could take a look & see if this was maybe a bug in the patch, or a bug in the addon that was triggered by your patch [& if the latter, if we can suggest something for them to fix].)

Seems like it's the latter.  ni=maone to look into addressing comment 3
Flags: needinfo?(g.maone)
(See also bug 1212968 and bug 1212848.  Per bug 1212848 comment 6 and bug 1212848 comment 8, if NoScript happens to use JPM [no idea], it'll also need to be rebuilt with an updated JPM version.)
(In reply to Daniel Holbert [:dholbert] from comment #4)
> (Clearly not. :)  The needinfo=you was to see if you could take a look & see
> if this was maybe a bug in the patch, or a bug in the addon that was
> triggered by your patch [& if the latter, if we can suggest something for
> them to fix].)
> 
> Seems like it's the latter.  ni=maone to look into addressing comment 3

My recommendation for fix: change all the stuff that's INCLUDE'd and LAZY_INCLUDE'd into vars.
I've fixed it, but I cannot upload 2.6.9.38rc1 to AMO (and therefore to make it signed and installable) because I get a game-breaking validation error: "Sub-package must be signed" :(
Does anybody know what it means? (just asked in IRC #amo, no answer yet)
Flags: needinfo?(g.maone)
BTW, if anybody is willing to check the fixed version, the unsigned package is here: https://noscript.net/betas/noscript-2.6.9.38rc1.xpi
(In reply to Giorgio Maone from comment #8)
> BTW, if anybody is willing to check the fixed version, the unsigned package
> is here: https://noscript.net/betas/noscript-2.6.9.38rc1.xpi

Just checked it -- looks like the bug's fixed there. Sites show up in noscript's menu again, & I can selectively enable scripts. (checked w/ e10s enabled and with it disabled)

Good luck with the signing issue; not sure what that means.
It was actually an AMO issue, for which I filed bug 1213163.
I found a pro-tempore work-around, so I'm gonna publish 2.6.9.38 stable later today.
Thanks for reporting.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.