880070 - NS_ERROR_XPC_BAD_OP_ON_WN_PROTO in Addon Content Script

Status: RESOLVED INVALID

(Firefox :: Extension Compatibility, defect)

Description

[David Skidmore]

I maintain an addon that uses jQuery in the content scripts to examine the DOM, then communicate results back to the chrome scripts. A JavaScript Module uses loadFrameScript to load something like the following:

    var loader = Components.classes["@mozilla.org/moz/jssubscript-loader;1"]
            .getService(Components.interfaces.mozIJSSubScriptLoader);
    
    var executionContext = Object.create(content);
    loader.loadSubScript("chrome://my-package/content/libs/jquery-2.0.2.js", executionContext);
    loader.loadSubScript("chrome://my-package/content/logic.js", executionContext);

In Firefox 21 and below, my logic.js file proceeds to use jQuery and talk back to the addon using sendAsyncMessage. After downloading the Firefox 22 beta I get the following error in my console:

    Error: NS_ERROR_XPC_BAD_OP_ON_WN_PROTO: Illegal operation on WrappedNative prototype object
    Source File: chrome://my-package/content/libs/jquery-2.0.2.js
    Line: 829

The line number refers to the "not-minified" debug version of jQuery 2.0.2, which seems to be using setTimeout on that line. I used the browser debugger and confirmed that "typeof setTimeout === 'function'", and so I am confused about what is illegal in this operation. To be clear, I do not think this is a jQuery bug, but rather this was the first script I loaded which triggered the behavior.

The workaround I have started is to rewrite things to use the add-on SDK. Otherwise, I am not sure how to change this file to load scripts into the context of the "content" object without losing access to globals like sendAsyncMessage.

Comment 1

[Loic]

Could you attach a simple extension (testcase) reproducing the issue, please.
Maybe there is a regression in Firefox or SDK.

Comment 2

[David Skidmore]

Attachment: Simple test-case addon-on.

This is a quick-and-dirty addon for triggering the bug. It creates a toolbar button that you may need to add manually. In Firefox 21 it will load some logic and pass some messages between the content and chrome scripts. In Firefox 22 it fails with the exception I mentioned.

Comment 3

[Loic]

Regression range:

good=2013-03-22
bad=2013-03-23
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=0e9badd3cf39&tochange=3825fdbcec62

Suspected: bug 658909

There is a ton of patches, bisection needed maybe.

Comment 4

[Bobby Holley (:bholley)]

Bisecting that bug is unlikely to be helpful. :-)

Does the addon still break if you remove the background.js and logic.js stuff?

Comment 5

[David Skidmore]

(In reply to Bobby Holley (:bholley) from comment #4)
> Bisecting that bug is unlikely to be helpful. :-)
> 
> Does the addon still break if you remove the background.js and logic.js
> stuff?

It "breaks" insofar as most of the addon logic is split between those two files.

Comment 6

[Bobby Holley (:bholley)]

Oh I see. So background.js loads inject.js as a subscript, which itself loads jquery and logic.js as subscripts.

Anyway, looking at this again, this almost certainly has to do with using |content| as the proto to Object.create(). You can't set DOM objects as the proto for random JS objects and expect to invoke DOM operations on the derived objects. Indeed, bug 658909 was all about dropping support for that behavior (which isn't in either the JS or DOM specs). We started dropping support for this back in bug 622301, but it's been a long road. :-)

If you remove that, does the addon work?

Comment 7

[David Skidmore]

Well... ****. How would I go about loading jQuery as a content script with access to the DOM and still retain the ability to use sendAsyncMessage (logic.js) to talk a back to background.js? I think I break the scope chain when I start loading scripts into |content|.

Comment 8

[Bobby Holley (:bholley)]

Try evaluating all this stuff in a Sandbox with sandboxPrototype set to the content. We've got special magic to make sandboxPrototype work correctly. :-)

Comment 9

[David Skidmore]

Sandboxes seem a little bit awkward for loading jQuery, but I followed an example of using XMLHttpRequest to read the file and evaluate it. I am still not sure how to send asynchronous messages though. Importing functions like this does not seem to help:

    var sandbox = Components.utils.Sandbox(browser.contentWindow, {
        sandboxPrototype: browser.contentWindow
    });
    sandbox.addMessageListener = browser.messageManager.addMessageListener;
    sandbox.sendAsyncMessage = browser.messageManager.sendAsyncMessage;

I get the error "NS_NOINTERFACE: Component does not have requested interface [nsIMessageListenerManager.addMessageListener]". Is it possible to use complex messaging with sandboxes?

Comment 10

[Bobby Holley (:bholley)]

addMessageListener and sendAsyncMessage are methods on the messageManager, so you can't just stick them on the sandbox and treat them as functions. Try doing .bind?

Comment 11

[David Skidmore]

Ah, I see what you mean. So, giving the sandbox a bound copy of the methods I want would be more like:

    var mManager = browser.messageManager;
    var sandbox = Components.utils.Sandbox(browser.contentWindow, {
        sandboxPrototype: browser.contentWindow
    });
    sandbox.addMessageListener = mManager.addMessageListener.bind(mManager);
    sandbox.sendAsyncMessage = mManager.sendAsyncMessage(mManager);

However, debugging scripts once they get inside of the sandbox seems very problematic. So far I can tell addMessageListener and sendAsyncMessage are getting called, but my chrome scripts are not receiving any messages from either the browser or window message managers.

Comment 12

[Bobby Holley (:bholley)]

(In reply to David Skidmore from comment #11)
> However, debugging scripts once they get inside of the sandbox seems very
> problematic. So far I can tell addMessageListener and sendAsyncMessage are
> getting called, but my chrome scripts are not receiving any messages from
> either the browser or window message managers.

I'm not really following. If you do:

sandbox.foopy = function foopy() { someActionInYourChromeScope()? }

Can you successfully invoke |foopy|? If so, then you should be fine - there's no difference between invoking foopy from the sandbox and from the chrome scope itself.

Comment 13

[Hector Zhao [:hectorz]]

(In reply to Bobby Holley (:bholley) from comment #8)
> Try evaluating all this stuff in a Sandbox with sandboxPrototype set to the
> content. We've got special magic to make sandboxPrototype work correctly. :-)

Works for us, thanks!

Comment 14

[David Skidmore]

Attachment: Updated test addon to use a sandbox.

I adjusted my testcase addon to use a sandbox around the the |contentWindow| object of the browser. Rather than use loadFrameScript to bring in a file which in turn brings in the logic, it directly loads the logic into the sandbox. However, sharing the message manager with the sandbox does not seem to be working for me.

Comment 15

[Bobby Holley (:bholley)]

You need to use evalInSandbox, rather than loadSubscript.

Do your callbacks get called?

Comment 16

[David Skidmore]

Attachment: Adjusted addon again to use evalInSandbox.

Alright, switching to evalInSandbox and placing breakpoints on the callbacks I give the sandbox, I think they are getting called but the message manager is not passing messages between the sandbox and chrome scripts.

Comment 17

[Bobby Holley (:bholley)]

(In reply to David Skidmore from comment #16)
> Alright, switching to evalInSandbox and placing breakpoints on the callbacks
> I give the sandbox, I think they are getting called but the message manager
> is not passing messages between the sandbox and chrome scripts.

How is the sandbox involved, exactly? If the calls to the message manager are happening in functions defining in the chrome script, then there's no difference between invoking those functions from the sandbox and invoking them directly from the chrome script. As far as the engine is concerned, a function defined in a scope runs with the principal and characteristics of that scope.

Comment 18

[David Skidmore]

Now that you mention it, of course I can do away with the message manager for moving data back and forth. I was still stuck in thinking I needed it because I was thinking chrome vs content scripts. Debugging everything that gets evaluated in the sandbox is still problematic, but at least I am up and running again.

Comment 19

[Bobby Holley (:bholley)]

Updating flags - this isn't really a regression per-se. I rewrote some fundamental XPConnect architecture in bug 658909, which changes behavior in various cases. I added compat hacks for all the cases that came up, but there are still some rare corner cases (like this one) where the difference is observerable.

It sounds like David's got himself up and running with sandboxes, so I don't think there's any reason to track this bug in particular. We should definitely watch out for addon-compat reports though, in case there are any other compat shims we need to stick in at the last minute.

Comment 20

[Jorge Villalobos [:jorgev] (he/him)]

Sounds good to me. Shouldn't we close this bug, then?

Comment 21

[David Skidmore]

In the short term I just bound sendAsyncMessage in one context to the handler in the other, and long term I ought to look into using the add-on SDK. Thanks a ton for all the patience while I figured out how sandboxes work.

Comment 22

[Godmar Back]

We are also getting NS_ERROR_XPC_BAD_OP_ON_WN_PROTO exceptions in jQuery code that accesses the DOM.

We are already using a Sandbox/evaluateInSandbox, so the proposed fix will not apply to us.

Specifically, I found that all instances of 'setTimeout', 'setInterval', etc. inside the jQuery code cause this error.  

I was able to suppress this error by changing 'setTimeout' to 'window.setTimeout' etc. for all but one instance; namely, this "fix" does not appear to work for 'clearInterval'. The error appears whether I say 'window.clearInterval' or 'clearInterval', as in the original jQuery 1.7.2 code.

This breakage appeared somewhere between Firefox 22 and 24, but wasn't reported to us until now. It basically means that jQuery's animations (show/hide) no longer work when jQuery is run inside a sandbox. We're using jQuery for pretty much our entire UI for actions we place into the page.

I would appreciate any pointers to documentation about what you've changed so that jQuery no longer works. The error message doesn't make sense to me. Why is clearInterval an illegal operation?

Here's more info:

We build our sandbox like so:

https://github.com/godmar/LibX/blob/libx2/src/base/core/global/ff/libapp/sandbox.js#L41-63

As 'win' we pass in e.window where 'e' is the DOMContentLoaded event of the document being loaded.
Within this sandbox, we evaluate this jQuery code: 
http://libx.org/libx2/libx2-git/src/base/bootstrapped/scripts/jquery-latest.js
The error appears on line 8970 with the call to window.clearInterval.

You can also test that by installing http://libx.org/releases/ff/libx2-latest.xpi?edition=vt and visiting, for instance, Google.

Comment 23

[Bobby Holley (:bholley)]

(In reply to Godmar Back from comment #22)
> We are already using a Sandbox/evaluateInSandbox, so the proposed fix will
> not apply to us.

> We build our sandbox like so:
> 
> https://github.com/godmar/LibX/blob/libx2/src/base/core/global/ff/libapp/
> sandbox.js#L41-63

Your code appears to do: |this.sandBox.__proto__ = win;| This is what's causing the breakage.

Instead, you need to pass {sandboxPrototype: win} as your SandboxOptions when instantiating the Sandbox. This should fix the problem.

Comment 24

[John Nagle]

I'm getting this from an add-on under Firefox 25/Win7/64-bit

[22:25:10.549] [Exception... "Illegal operation on WrappedNative prototype object"  nsresult: "0x8057000c (NS_ERROR_XPC_BAD_OP_ON_WN_PROTO)"  location: "JS frame :: chrome://adrater/content/script-compiler.js :: nodechanged :: line 887"  data: no] @ chrome://adrater/content/script-compiler.js:887

This is from an old add-on of mine compiled with the old Greasemonkey compiler.  The bug seems to be down inside the Greasemonkey compiler ("script-compiler.js"), not in my code. 

If this change to Firefox breaks the Greasemonkey compiler, it's going to break other things.

Comment 25

[Bobby Holley (:bholley)]

What is the greasemonkey script compiler? Is it still under active development?

This is is the best I can find on google: https://github.com/ginatrapani/greasemonkey-multi-script-compiler , but the last commit appears to be three years old:

Anyway, the fix is simple:

https://github.com/ginatrapani/greasemonkey-multi-script-compiler/blob/master/templates/content--script-compiler.js.tpl#L98

The line |sandbox.__proto__=sandbox.window;| is what's causing the breakage. See comment 23.

John, if this is something that's still used, can you get in touch with Gina and see if she update it?

Comment 26

[Bobby Holley (:bholley)]

(Even though there's no actual Gecko bug here, it's probably best to leave this bug open, so that folks like John will find it when they search on bugzilla)

Comment 27

[John Nagle]

Greasemonkey is a cross-platform system for developing add-ons. See "http://www.greasespot.net/".  

The Greasemonkey compiler is a packaging system which takes Greasemonkey add-ons and turns them into Firefox add-ons.  It's developed separately from Greasemonkey and has worked well for years without changes.  Greasemonkey add-ons tend to be very stable, and don't require constant updating for Mozilla changes like Jetpack add-ons. Greasemonkey is what Jetpack wants to be when it grows up.

Reported this problem to the Greasemonkey compiler developers as their Issue #2.
https://github.com/ginatrapani/greasemonkey-multi-script-compiler/issues/2
I don't expect much to happen. 

AMO's add-on verifier should be modified to detect add-ons which are broken by this change to Mozilla.  Someone with
appropriate access should search all existing add-ons for "script-compiler.js".  All those are now probably broken.

Comment 28

[Bobby Holley (:bholley)]

Sounds good. Jorge, can you have a look?

Comment 29

[Jorge Villalobos [:jorgev] (he/him)]

I filed bug 938223 to update the validator and bug 938679 to notify affected add-on developers.

Comment 30

[Andy McKay]

With WebExtensions being the only valid way of doing extensions in Firefox 57, I don't think this bug is still relevant.