Closed
Bug 236839
Opened 20 years ago
Closed 5 years ago
Disabling JavaScript should not prevent JavaScript contained in chrome XBL from running
Categories
(Core :: XBL, enhancement)
Core
XBL
Tracking
()
RESOLVED
FIXED
People
(Reporter: glazou, Unassigned)
References
(Blocks 1 open bug)
Details
The editor disables JavaScript because it makes no sense to edit a dynamic document. That also disables the execution of all JavaScript code contained in XBL bindings, wherever these bindings come from, chrome or not. In particular, the constructors/destructors of the bindings are not executed... This is a blocker for the implementation of templates in Composer. I could use workarounds for methods and handlers, but hardly for constructors and destructors. And XBL remains the simplest solution to do that; the cost of a c++ implementation is prohibitive in comparison with the XBL solution. It is then proposed to enable JavaScript code contained in chrome XBL bindings attached through a chrome stylesheet. Given the existence of bug 208864, I think this mechanism should not be limited to the editor, and the browser should keep running XBLs when they come from the chrome even if JS is disabled.
Comment 1•20 years ago
|
||
So to summarize the discussion we've had up to now, as I understand it: 1) Disabling JS for the document and disabling JS for the nsJSContext should not be the same thing. Editor wants to do the former not the latter (it does the latter now). 2) The nsJSContext api for compiling/evaluating/etc scripts should always take an nsIPrincipal for the caller and only use the ScriptGlobalObject principal if the caller doesn't pass one. 3) The "JS disabled for the document" check should allow execution of JS with chrome principals. 4) XBL should pass in principals as needed. I'm happy to do steps 2 and 4, but I'm not quite sure what people would like to do for step 1, and step 3 yet.....
Comment 2•20 years ago
|
||
I don't think that's quite what we agreed on (unless I missed some emails). I don't think we should have more than one "JS is disabled" flag per document/context, we need another flag that's separate from whether JS is enabled or not that just tells if the document is "interactive" or not, and that affects much more than just JS execution (link handling, form submission, etc). I'd like to first settle on the two separate "states" we want. I proposed adding an "interactive" property to nsIDocument (i.e. IsEnabled() and SetIsEnabled(PRBool)), can we agree on this so far? As for #3, I don't think we want that, if JS is disabled, JS is disabled, no matter who calls it. IMO.
Comment 3•20 years ago
|
||
Hmm.. that is what we'd come to, yes. My apologies for misremembering. So with your proposal, the situation, as far as I can tell, is as follows: PROPOSED ARCHITECTURE: 1) JS can be disabled on nsJSContext 2) JS can be disabled via prefs; security manager handles this 3) Document can be non-interactive With those three settings all sort of orthogonal to each other. The behavior we are proposing is then: 1) If JS is disabled on nsJSContext, no JS executes, period. 2) If JS is enabled on nsJSContext but the document is non-interactive, only execute JS that's coming from a chrome principal (fixes composer's issues). 3) If JS is enabled on nsJSContext and the document is interactive, still allow chrome JS execution no matter what the user prefs are set to (fixes XBL form control issues and bug 208864). The only issue with all that is that <marquee> would animate in composer with that proposal. This can probably be solved by just setting the -moz-binding on it to some other value in the editor's stylesheets. I'm in favor of SetInteractive() and Interactive() myself (rather than enabled). Seems to describe the situation more clearly....
Comment 4•20 years ago
|
||
So that wouldn't solve the issue for mail? I remember a mail bug (don't have the number right now) where context menus on scrollbars weren't disabled on Mac because the XBL JS for the scrollbars didn't run unless you enabled JS in mails. Or would mail use the non-interactive setting? (I'm asking because from your proposal it seems non-interactive disables some stuff that mail might want enabled) The scrollbar bug got fixed in some other way btw, so it's probably a FIXED bug.
Comment 5•20 years ago
|
||
(In reply to comment #3) > The only issue with all that is that <marquee> would animate in composer with > that proposal. Does text-decoration:blink blink in composer? if it does, why should marquee not animate?
Comment 6•20 years ago
|
||
Because it's possible to edit blinking text but not really possible to edit moving text? ;)
Comment 7•20 years ago
|
||
Um, I don't know where I pulled that IsEnabled/SetEnabled from, I meant to say IsInteractive/SetInteractive. I think I'd prefer IsInteractive() over just Interactive, but I think I can deal either way :-)
Comment 8•20 years ago
|
||
One issue here.... If the XBL JS is compiled with a chrome principal, does that mean that a page can call a method on the binding and have it execute with chrome priveleges? If so, we need to carefully audit all methods that are exported on chrome bindings that content can load.
Blocks: 264817
Comment 10•19 years ago
|
||
Any chance of reviving work on this bug? It's blocking XUL error pages in addition to some other interesting bugs.
Comment 11•19 years ago
|
||
This bug doesn't really affect error pages in any way. Whoever added that dependency is a little confused. This bug _is_ important, and there _is_ interest in working on it. It's not clear what should happen here, however, and it's a hard problem to solve, in general, without opening up security holes. We're probably going to try to solve it in the 1.9 cycle, though.
Comment 12•19 years ago
|
||
Boris, I've just run into this problem (or something extremely similar) during my rewrite of Image Toolbar for Firefox. Asqueella pointed out this bug and something occurred to me that may be relevent. Currently, I'm using nsIStyleSheetService to overlay custom CSS (which includes XBL bindings) onto content documents. This has the advantage of making my code extremely efficient and work in 100% of the cases it should (AFAIK). With JavaScript disabled, XBL event JavaScript fails to fire, which means that essentially, my extension is rendered useless. If you're working on this in the future then I can hackaround or put up with the emails until then. Possible suggestion: If CSS (w/XBL) is being inserted from the chrome, it would make sense to me if at some point during that insertion it was placed inside a wrapper (if that even makes sense in this context!) and executes in the chrome scope. It seems that if code is being injected from chrome then it should retain that scope (and security access) so long as it's unmodified. Making sure it's inserted in this way would probably simplify this issue somewhat. Disclaimer: I'm hardly an authority on Mozilla, C++ or security, but it would make sense to me if this bug could be partially fixed that way.
Comment 13•19 years ago
|
||
> and executes in the chrome scope.
And this is the part that's got security hole potential like no tomorrow...
Especially if the page can trick said XBL on operating on whatever content the
page passes it.
There's the separate issue that if the XBL is on the chrome wrapper (so has
chrome privileges), then the unprivileged page can't call those XBL methods.
Which is very undesirable in a lot of cases.
Comment 14•19 years ago
|
||
(In reply to comment #13) > There's the separate issue that if the XBL is on the chrome wrapper (so has > chrome privileges), then the unprivileged page can't call those XBL methods. > Which is very undesirable in a lot of cases. If that is ever allowed, we need to have some security review of the XBL code so that unpriv. code can't trick the chrome xbl into executing stuff with chrome privs (like my recent PFS code injection bug).
Comment 15•19 years ago
|
||
(In reply to comment #13) > > and executes in the chrome scope. > > And this is the part that's got security hole potential like no tomorrow... > Especially if the page can trick said XBL on operating on whatever content the > page passes it. > Notice that the very popular (and loved by Asa) GreaseMonkey extension (and its children, like Patyplus) doesn't work when/where JS is disabled. FlashBlock (a Macromedia Flash content blocker) doesn't work either, and this is in my opinion less acceptable because someone who wants to block Flash selectively on some pages may reasonably want to prevent JavaScritp execution as well. Both extensions use XBL to modify page content and inject scripts. They need JS XBL-injected by the chrome to run even if JS is disabled.
Comment 16•19 years ago
|
||
(In reply to comment #13) > > and executes in the chrome scope. > > And this is the part that's got security hole potential like no tomorrow... > Especially if the page can trick said XBL on operating on whatever content the > page passes it. > Notice that the very popular (and loved by Asa) GreaseMonkey extension (and its children, like Patyplus) doesn't work when/where JS is disabled. FlashBlock (a Macromedia Flash content blocker) doesn't work either, and this is in my opinion less acceptable because someone who wants to block Flash selectively on some pages may reasonably want to prevent JavaScritp execution as well. Both extensions use XBL to modify page content and inject scripts. They need JS XBL-injected by the chrome to run even if JS is disabled.
Comment 17•19 years ago
|
||
Bug 159932 either dependency or duplicate? "disabling javascript in browser doesn't allow chrome:// to add eventListners to window"
Comment 18•19 years ago
|
||
Neither dep nor dup.
Comment 19•19 years ago
|
||
We need to do a bunch of XBL work for 1.9, and this bug is just part of that lot. cc'ing peterv and sicking. This bug really needs a new assignee. /be
Target Milestone: --- → mozilla1.9alpha
What I think we should do for now (until we dare go for bug 59701) is to let JS from chrome XBL run even when JS is disabled, but with page privileges. This is how XBL behaves when JS is disabled on docshell level.
Updated•16 years ago
|
Flags: blocking1.9.1?
Updated•16 years ago
|
Assignee: hyatt → nobody
QA Contact: ian → xbl
Comment 22•15 years ago
|
||
(In reply to comment #16) > FlashBlock (a Macromedia Flash content blocker) doesn't work either, and this is > in my opinion less acceptable because someone who wants to block Flash > selectively on some pages may reasonably want to prevent JavaScritp execution as > well. +1. I'm on this situation. Is there a hope to see this old bug to be solved one day?
Comment 23•15 years ago
|
||
(In reply to comment #22) > (In reply to comment #16) > > someone who wants to block Flash selectively on some pages > > may reasonably want to prevent JavaScript execution as well. > > +1. I'm on this situation. Is there a hope to see this old bug to be solved one > day? Notice that my comment #16 is obsolete, because NoScript can replace FlashBlock: http://noscript.net/features#contentblocking
Updated•15 years ago
|
Flags: wanted1.9.2?
Target Milestone: mozilla1.9alpha1 → ---
Comment 24•15 years ago
|
||
comment #16 may be obsolete, but this does still prevent me from writing a FlashBlock-style extension for controlling GIF/APNG animations and then using it alongside NoScript. (Not true blocking, of course. Just "Display only first frame unless an overlaid icon in the corner is clicked")
Updated•13 years ago
|
Flags: wanted1.9.2?
Comment 25•13 years ago
|
||
Other than extensions that the user has to explicitly install, what else can load anything into chrome:// URLs? Heck, why can external pages even access chrome:// URLs? And the fact that someone has written a workaround for a popular extension doesn't mean this isn't still a bug.
Comment 26•13 years ago
|
||
> what else can load anything into chrome:// URLs? Nothing. > Heck, why can external pages even access chrome:// URLs? At this point, because the chrome package in question explicitly chose to allow that. Access is blocked by default.
Comment 27•12 years ago
|
||
(In reply to Giorgio Maone from comment #23) > Notice that my comment #16 is obsolete, because NoScript can replace > FlashBlock: http://noscript.net/features#contentblocking This bug still causes problems for users who use FlashBlock with YesScript (https://addons.mozilla.org/en-US/firefox/addon/yesscript/), since YesScript doesn't include a Flash blocker. For those who aren't familiar with it, YesScript provides a JavaScript blacklist rather than a whitelist.
Comment 28•12 years ago
|
||
PDF.js, feed subscribe pages and HTML5 audio/video keep slapping me in the face with my NoScript use and I’d like to look at fixing it ‐ How hard would this be to tackle for someone of little Firefox hacking experience? How bitrotted are comment 3 and comment 20 ?
Comment 29•12 years ago
|
||
Comment general arch proposal should still be ok. Comment 20 should be ok as well, actually. As far as how hard it is, that depends on which set of problems you're tackling. The videocontrols can just be solved by comment 20, which should not be too bad to do. Doing the entirety of comment 3 might take a bit more work. In particular, I'm not sure how PDF.js hooks itself up. The feed subscribe pages is just a recent regression, by the way. See bug 786009.
Comment 30•12 years ago
|
||
(In reply to Boris Zbarsky (:bz) from comment #29) > The feed subscribe pages is just a recent regression, by the way. See bug > 786009. Ooh, ta.
Updated•11 years ago
|
Depends on: XBL-scopes
Updated•11 years ago
|
No longer depends on: XBL-scopes
Comment 31•5 years ago
|
||
This was fixed by bug 834697.
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•