Closed Bug 880070 Opened 12 years ago Closed 7 years ago

NS_ERROR_XPC_BAD_OP_ON_WN_PROTO in Addon Content Script

Categories

(Firefox :: Extension Compatibility, defect)

22 Branch
x86_64
macOS
defect
Not set
normal

Tracking

()

RESOLVED INVALID

People

(Reporter: dpskidmore, Unassigned)

References

Details

(Keywords: testcase)

Attachments

(1 file, 2 obsolete files)

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.
Could you attach a simple extension (testcase) reproducing the issue, please. Maybe there is a regression in Firefox or SDK.
Flags: needinfo?(dpskidmore)
Keywords: testcase-wanted
Attached file Simple test-case addon-on. (obsolete) (deleted) —
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.
Flags: needinfo?(dpskidmore)
Attachment #759251 - Attachment mime type: application/octet-stream → application/x-xpinstall
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: qawanted
QA Contact: jbecerra
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.
Blocks: 658909
Keywords: regression
Bisecting that bug is unlikely to be helpful. :-) Does the addon still break if you remove the background.js and logic.js stuff?
(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.
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?
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|.
Try evaluating all this stuff in a Sandbox with sandboxPrototype set to the content. We've got special magic to make sandboxPrototype work correctly. :-)
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?
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?
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.
(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.
(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!
Attached file Updated test addon to use a sandbox. (obsolete) (deleted) —
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.
Attachment #759251 - Attachment is obsolete: true
You need to use evalInSandbox, rather than loadSubscript. Do your callbacks get called?
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.
Attachment #760265 - Attachment is obsolete: true
(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.
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.
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.
Sounds good to me. Shouldn't we close this bug, then?
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.
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.
(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.
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.
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?
(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)
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.
Sounds good. Jorge, can you have a look?
Flags: needinfo?(jorge)
I filed bug 938223 to update the validator and bug 938679 to notify affected add-on developers.
Flags: needinfo?(jorge)
With WebExtensions being the only valid way of doing extensions in Firefox 57, I don't think this bug is still relevant.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: