986886 - addEventListener in GM script would not work if javascript.enabled = false

Status: NEW

(Core :: DOM: Core & HTML, defect, P5)

Description

[Alice0775 White]

Build Identifier:
https://hg.mozilla.org/releases/mozilla-release/rev/5f7c149b07ba
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:28.0) Gecko/20100101 Firefox/28.0 ID:20140314220517

This problem does not happen in Firefox27.

Steps To Reproduce:
1. Install Greasemonkey https://addons.mozilla.org/en-US/firefox/addon/greasemonkey/
   And Restart browser to finish install process
2. Install GM script which is using addEventListener

3. Open a web page and click body
   --- observe alert message

4. Disable JavaScript from about:config
   javascript.enabled = false

5. Open the web page and click body
   --- observe alert message

Actual Results:
addEventListener in GM script would not work if javascript.enabled = false

Expected Results:
addEventListener in GM script should work even if javascript.enabled = false

Regression window(m-i)
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/0d477f84d533
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:28.0) Gecko/20100101 Firefox/28.0 ID:20131112162707
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/cd0e2e0ef136
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:28.0) Gecko/20100101 Firefox/28.0 ID:20131112164406
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=0d477f84d533&tochange=cd0e2e0ef136

Triggered by: Bug 840488

Comment 1

[Alice0775 White]

Attachment: GM script using addEventListener, An alert box displays when click body

Comment 2

[Alice0775 White]

See reports http://userscripts.org/topics/198991

Comment 3

[Boris Zbarsky [:bzbarsky]]

This might well be an issue kinda similar to bug 986542: a global with no docshell, which therefore used to get script enabled automatically, but now gets it based on its principal or something and isn't privileged...

Comment 4

[Bobby Holley (:bholley)]

No, I think the issue is that GM runs scripts with a content principal, which, now that we've fixed various bugs, don't run when JS is disabled for content.

Anthony has some plans to run GM scripts with nsExpandedPrincipal, which should solve this problem. Anthony, do you have a GM build for that somewhere that we could use to test this issue?

Comment 5

[Sylvestre Ledru [:Sylvestre]]

Not tracking for now. I don't think we will block a release for that (disabling javascript seems pretty rare to me).

Comment 6

[trespassersW]

Attachment: testcase for userscript startup code and Click/Timeout event handlers.

Comment 7

[Anthony Lieuallen]

Attachment: Greasemonkey build with Expanded Principal support.

A build with Expanded Principal support added to support testing with the available reproduction steps.

Comment 8

[Bobby Holley (:bholley)]

(In reply to Anthony Lieuallen from comment #7)
> A build with Expanded Principal support added to support testing with the
> available reproduction steps.

Does this build turn on Expanded Principals by default, such that they'd be used for the affected script?

Assuming that's the case - Alice, does the problem go away with this build?

Comment 9

[Anthony Lieuallen]

Yes, 1.16beta2 turns on Expanded Principals for all privileged sandboxes.  No by default, just always, for sandboxes containing privileged functions.

The "this Unit Test" link at http://userscripts.org/topics/198991 uses a privileged sandbox, as does script linked from attachment 8396509 [details].

(But personally, for whatever reason, I could never reproduce this error before trying Expanded Principals.)

Comment 10

[Alice0775 White]

(In reply to Bobby Holley (:bholley) from comment #8)
> (In reply to Anthony Lieuallen from comment #7)
> Alice, does the problem go away with this build?

Yes. attachment 8396509 [details] fixes this problem.
GM script(incl. addEventListener, setTimeout) works even if javascript disabled.

Comment 11

[Marti]

Confirmed Arrayed nsIPrincipal for Sandbox working with addEventListener in:

Mozilla/5.0 (X11; Linux x86_64; rv:28.0) Gecko/20100101 Firefox/28.0

and

Mozilla/5.0 (X11; Linux x86_64; rv:28.0) Gecko/20100101 Firefox/28.0 SeaMonkey/2.25 with gmport for SeaMonkey

Nice find Anthony!

@Bobby,
Can we get a starter document piece to work on in https://developer.mozilla.org/docs/Components.utils.Sandbox or https://developer.mozilla.org/docs/XPCOM_Interface_Reference/nsIPrincipal since this fix appears undocumented at this time on MDN for expanded principals?

Comment 12

[trespassersW]

Comment on attachment 8396509 [details]
Greasemonkey build with Expanded Principal support.

greasemonkey.js [line 75]:
    var contentSandbox = new Components.utils.Sandbox(
        aContentWin,
        {
change to:
    var contentSandbox = new Components.utils.Sandbox(
        [aContentWin],
        {

Comment 13

[Bobby Holley (:bholley)]

(In reply to Marti from comment #11)
> @Bobby,
> Can we get a starter document piece to work on in
> https://developer.mozilla.org/docs/Components.utils.Sandbox or
> https://developer.mozilla.org/docs/XPCOM_Interface_Reference/nsIPrincipal
> since this fix appears undocumented at this time on MDN for expanded
> principals?

Will is the documentation lead on this stuff. He's on vacation, but hopefully can look at this when he gets back next week.

Comment 14

[Anthony Lieuallen]

FWIW This is mentioned, but not by name, on the Sandbox page, the fourth bullet.  A page/section calling out all the differences between Sandbox(principal) and Sandbox([principal]) would be very valuable to me, though.

Also, re: comment #12 (whoops I just realize now I never pushed):
https://github.com/greasemonkey/greasemonkey/commit/a0d93c59eb0c679dfb03a6c6715e3e5ba0014096

Comment 15

[Marti]

(In reply to Bobby Holley (:bholley) from comment #13)
> Will is the documentation lead on this stuff. He's on vacation, but
> hopefully can look at this when he gets back next week.
Thank you.

Another issue has been pointed out with Add-on compatibility. When NoScript 2.6.8.19 is added to the mix in a clean profile with Greasemonkey the addEventListener issue disappears entirely including with // @grant none. NoScripts versions ( https://addons.mozilla.org/firefox/addon/noscript/versions/?page=1#version-2.6.8.18rc2 and forward ) claim there are some issues in the core that Giorgio and his team corrects. Having the difference of principals "called out" clearly on MDN would be especially useful to help potentially resolve this compatibility issue.


(In reply to Anthony Lieuallen from comment #14)
> Also, re: comment #12:
> https://github.com/greasemonkey/greasemonkey/commit/
> a0d93c59eb0c679dfb03a6c6715e3e5ba0014096
I tried to get this part pushed to GH on https://github.com/greasemonkey/greasemonkey/issues/1882 but I think it needs to be called out somewhere by you proactively. I think what trespasserW may have been implying with the code post in comment #12 is that with // @grant none and site JavaScript disabled this will prevent addEventListener and related methods from functioning since that doesn't have the "Expanded Principal" on it like the privileged Sandbox does. Is this still the desired behavior?

Comment 16

[Giorgio Maone [:ma1]]

(In reply to Marti from comment #15)
> Another issue has been pointed out with Add-on compatibility. When NoScript
> 2.6.8.19 is added to the mix in a clean profile with Greasemonkey the
> addEventListener issue disappears entirely including with // @grant none.

I'm not sure I understand: is this a good or a bad thing?
If this is not the desired outcome, which is?
And if it's "good" instead, what kind of info do you need from me?

Comment 17

[Marti]

(In reply to Giorgio Maone from comment #16)
> (In reply to Marti from comment #15)
> I'm not sure I understand: is this a good or a bad thing?
> If this is not the desired outcome, which is?
> And if it's "good" instead, what kind of info do you need from me?

I'm not entirely sure logistically speaking and I defer to Anthonys judgment and inquiry at https://github.com/greasemonkey/greasemonkey/issues/1882#issuecomment-38916855 ... but it is a little unusual that NoScript can affect the content principal mentioned in comment 4 in this manner in .19 but .17 doesn't. I was hoping that you could shed some light on why .17 appears to follow the principals between add-ons but .19 appears not to if that is okay.

Comment 18

[Giorgio Maone [:ma1]]

(In reply to Marti from comment #17)
> (In reply to Giorgio Maone from comment #16)
> > (In reply to Marti from comment #15)
> > I'm not sure I understand: is this a good or a bad thing?
> > If this is not the desired outcome, which is?
>  I was hoping that you could shed some light on why .17 appears to follow the
> principals between add-ons but .19 appears not to if that is okay.

Since CAPS is about to be deprecated entirely, and it doesn't already work anyway on Nightlies for script blocking, NoScript .19 on Firefox 28 doesn't rely on it anymore, and uses the Components.utils.blockScriptForGlobal(window) API instead, which doesn't care about principals. So some differences (and I susped and fear, surprises) are to be expected and are going to be handled case by case.

Comment 19

[Will Bamberg [:wbamberg]]

(In reply to Marti from comment #11)
> Confirmed Arrayed nsIPrincipal for Sandbox working with addEventListener in:
> 
> Mozilla/5.0 (X11; Linux x86_64; rv:28.0) Gecko/20100101 Firefox/28.0
> 
> and
> 
> Mozilla/5.0 (X11; Linux x86_64; rv:28.0) Gecko/20100101 Firefox/28.0
> SeaMonkey/2.25 with gmport for SeaMonkey
> 
> Nice find Anthony!
> 
> @Bobby,
> Can we get a starter document piece to work on in
> https://developer.mozilla.org/docs/Components.utils.Sandbox or
> https://developer.mozilla.org/docs/XPCOM_Interface_Reference/nsIPrincipal
> since this fix appears undocumented at this time on MDN for expanded
> principals?


I've updated https://developer.mozilla.org/en-US/docs/Components.utils.Sandbox to talk about expanded principals. It's not perfect but it's a start. I'm not sure where this stuff should live eventually, but this seems like the best place for it now.

Comment 20

[Firefox Bug Husbandry Bot]

https://bugzilla.mozilla.org/show_bug.cgi?id=1472046

Move all DOM bugs that haven’t been updated in more than 3 years and has no one currently assigned to P5.

If you have questions, please contact :mdaly.