javascript.enabled and docShell.allowJavascript do not disable all event handlers

RESOLVED FIXED in mozilla27



12 years ago
6 years ago


(Reporter: mikeperry.unused, Assigned: bzbarsky)


(Depends on 1 bug, {privacy})

Dependency tree / graph
Bug Flags:
wanted1.9 +

Firefox Tracking Flags

(firefox5-, firefox6-, blocking2.0 -)


(Whiteboard: [firebug-p3], )


(1 attachment)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv: Gecko/20071222 Remi/ Firefox/
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv: Gecko/20071222 Remi/ Firefox/

Javascript event handlers registered with addEventListener are not disabled when either javascript.enabled or docShell.allowJavascript are toggled. Other types of event handlers, such as those registered via the DOM event handler attributes are properly canceled.

Reproducible: Always

Steps to Reproduce:
1. open about:config
2. Visit
3. Click on the page
4. Toggle javascript.enabled to false
5. Click on the firefox bug page again. Observe that some event handlers have been disabled, others have not
Actual Results:  
While some types of event listeners are blocked, other event listeners still fire.

Expected Results:  
No client-side javascript code should execute at all.

This bug is critical to Tor security. The Torbutton extension ( relies on the ability to prevent pages from executing Javascript and other dynamic content once Tor has been disabled, to prevent pages from performing network activity after the user has changed Tor state.
Confirming bug.

Have you considered adapting the "session restore" functionality and simply closing existing pages, then resurrecting the user's state when they turn off the Tor button? Even if you kill off the remaining event listeners for existing pages I'd still be nervous about letting Tor users interact with them.
Component: Security → DOM: Events
Product: Firefox → Core
QA Contact: firefox → events
Ever confirmed: true
What we could probably do is make AutoScriptEvaluate::StartEvaluating return an error if script can't be executed on that cx with the relevant principal.  That somewhat relies on the right cx being used, but we certainly do that with the cx pusher for event listeners.

Unfortunately, other callbacks into JS may not be so lucky (XMLHttpRequest readystate handlers come to mind, though there could also be issues in any DOM API that allows passing in page-implemented callbacks and might call them async... Or even that doesn't return for a long time (e.g. one of the callbacks does sync XMLHttpRequests or some such).  With this last, for example, treewalker's nodefilters could be attacked.  Or the namespace resolver from the proposed DOM API for Selectors.  Or any of a number of other callbacks.  I guess in the long-running situation the calling cx will still be on the stack, so things might be ok and we only need to worry about propagating the right cx into async callbacks...

That would only affect the docshell setting and per-site settings, though.  If we fail to get the right cx but are really async, we'll get the safe context and that's affected by the general "turn off JS" pref.  So at least as a first approximation doing the checks in StartEvaluating would work.

That said, nsXPCWrappedJSClass::CallQueryInterfaceOnJSObject does an OBJ_GET_PROPERTY before calling StartEvaluating.  That doesn't seem safe...  brendan, am I missing something there?
Requesting blocking, but I'm not completely sure it should, to be honest...
Flags: blocking1.9?
Not blocking as this is how we've always behaved and it's late in the release cycle. But wanted as it would be nice to have if someone came up with a safe fix.
Flags: wanted1.9+
Flags: blocking1.9?
Flags: blocking1.9-
Would like to point out that many people do depend on this functionality outside of Tor users. Users of Javascript toggling extensions such as QuickJava come to mind as well.
Keywords: privacy
BZ or Jonas, do you have anyone in mind who could work on this?
Smaug might be a good candidate.
Assignee: nobody → Olli.Pettay
Or jst maybe. Jst: is there any way we can disable this on a javascript level? So that no matter which entry points we miss (what about timers) it'll still be blocked?
Any progress?
any news during the last few years?
It's a pretty low priority.  I suspect that in the end someone who actually cares about dynamically enabling/disabling JS while a site is live will need to step up and fix this.  I'm happy to point to relevant code as needed.
docShell.allowJavascript = false;

doesn't prevent JavaScript in the onload="" attribute of images either.  This doesn't just affect websites, but XUL <editor>s as well.  (ScribeFire, in particular.)

I've filed a separate bug for this, but it might be better included in this bug.  I hope someone will fix it, because it greatly impacts major features of the <editor> element, including image drag and drop.
Yeah, that's different from this bug, since comment 0 explicitly says that attribute on* handlers _are_ prevented in the case this bug is filed about.
Whiteboard: [firebug-p1]
It seems that jsdIStackFrame.executionContext.scriptsEnabled may also be a victim.
We added this code to Firebug 1.5:
  context.eventSuppressor = context.window.getInterface(Ci.nsIDOMWindowUtils);
  if (context.eventSuppressor)
in addition to 
  executionContext.scriptsEnabled = false;
when we stop on a breakpoint. 

This immediately solved one bug and we've not seen bad side effects. The only funny business was puzzing: if we call context.window.getInterface() after returning from the debuggers nested event loop, it fails. That is why we save the interface ref in context.eventSuppressor so we can use that to free the window after we return.
Whiteboard: [firebug-p1] → [firebug-p3]
(In reply to comment #15)
> We added this code to Firebug 1.5:
>   context.eventSuppressor = context.window.getInterface(Ci.nsIDOMWindowUtils);
>   if (context.eventSuppressor)
>       context.eventSuppressor.suppressEventHandling(true);

John, I've tried doing this call on the contentWindow attribute of the relevant tabbrowser.docShell attribute, but it is disabling all hotkeys in the entire browser window. For example, control-t will not cause that browser window to open a new tab if the tab itself is the currently selected widget (as opposed to the URL bar, for example).

Did you notice this side effect as well? If not, which window are you actually calling suppressEventHandling on?
(In reply to comment #16)
> ...but it is disabling all hotkeys in the entire
> browser window.
> Did you notice this side effect as well?

(Not that this is related to this bug, but) preventing all hotkeys if the focused windows has events suppressed is by design, not a side effect.
Mike, is a great place to ask such questions. And I'll confirm what Olli said as what we also see.
My apologies. I was just trying to keep some historical record for other people who hit this bug and try the workaround, since it is going on 3 years old, and I've long since lost any hope for an actual fix :)

I will try to investigate more and if needed continue workaround discussion somewhere else...
Flags: blocking1.9.0.19?
Flags: blocking1.9.0.19?
Status needs to be changed to Confirmed.
blocking1.9.1: --- → ?
blocking1.9.2: --- → ?
blocking2.0: --- → ?
OS: Linux → All
Hardware: x86 → All
Not blocking 1.9.3 on this bug.
blocking2.0: ? → -
blocking1.9.1: ? → ---
blocking1.9.2: ? → ---
The Tor Project / Electronic Frontier Foundation is paying to have this bug fixed.

"If you know C++ and/or Firefox internals, we should be able to pay you for your time to address these issues and shepherd the relevant patches through Mozilla's review process."

The original test case no longer works. But if it did we could see if Firebug 1.7 can block the events. I guess yes, we use nsIDOMWindowUtils suppressEventHandling(true).
Flags: blocking1.9-
Posted file Minimal testcase
Since the original test case has disappeared, I took the liberty of making a new one.

Note that this isn't just a privacy issue, it's also a performance issue: sometimes you need to disable Javascript to prevent the browser from slowing to a crawl. The current bug means that it's never possible for the user to completely disable Javascript when it was previously enabled, unless he reloads ALL open pages.
Duplicate of this bug: 812056
Duplicate of this bug: 652049
Depends on: ParisBindings
Depends on: 971650
Fixed by bug 862627.
Closed: 6 years ago
Depends on: 862627
Resolution: --- → FIXED
No longer depends on: 971650
Assignee: bugs → bzbarsky
Target Milestone: --- → mozilla27
You need to log in before you can comment on or make changes to this bug.