Closed Bug 607352 Opened 9 years ago Closed 7 years ago

Can’t access the "onclick" property of XPCNativeWrapped objects in chrome context

Categories

(Core :: XPConnect, defect)

x86
All
defect
Not set

Tracking

()

RESOLVED FIXED
Tracking Status
blocking2.0 --- .x+

People

(Reporter: sloonz, Assigned: mrbkap)

Details

Attachments

(1 file, 1 obsolete file)

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.2.11) Gecko/20101012 Firefox/3.6.11
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.2.11) Gecko/20101012 

I’m trying to develop a userChromeJS script which involve checking the existence of an "unload" attributes in some DOM elements (I check myElement.onclick == undefined). However, I get this error :

Error: Component is not available = NS_ERROR_NOT_AVAILABLE
Source file: chrome://userchromejs/content/userChromeJS.js -> file:///Users/sloonz/Library/Application%20Support/Firefox/Profiles/atn1tz86.dev/chrome/test.js
Line: 6

Checking for the existence of another attribute seems perfectly valid (myElement.aruniste == undefined returns true). The exception is raised in nsEventReceiverSH::RegisterCompileHandler (dom/base/nsClassInfo.cpp) :

  if (ObjectIsNativeWrapper(cx, obj)) {
    return NS_ERROR_NOT_AVAILABLE;
  }

If I return NS_OK instead of NS_ERROR_NOT_AVAILABLE, or if I drop this test, I have no exception, but the result is always "undefined".

Reproducible: Always

Steps to Reproduce:
1. Install userChromeJS extension (http://userchromejs.mozdev.org/)
2. Install test.js in (profile)/chrome and add userChrome.import("test.js", "UChrm"); into (profile)/chrome/userChrome.js
3. Go surf in the url in test.js
4. Observer the results in the console
Actual Results:  
undefined
Error: Component is not available = NS_ERROR_NOT_AVAILABLE
Source file: chrome://userchromejs/content/userChromeJS.js -> file:///Users/sloonz/Library/Application%20Support/Firefox/Profiles/atn1tz86.dev/chrome/test.js
Line: 6


Expected Results:  
undefined
undefined (or the content of the onclick attribute if there is one defined)
Attached file test case
You can work around this by examining myElement.wrappedJSObject.onclick, I assume.

Blake, what's the right behavior here?  Compiling on access to the wrapper seems really fishy to me; throwing is not great either, though.  We could make the case for just not claiming to have the property (since conceptually XPCNativeWrapper isn't supposed to have things we can't get via xpconnect), but we bend this for other classinfo stuff in some cases...
Even ccing blake!
Component: DOM → XPConnect
QA Contact: general → xpconnect
Hi - we found this bug while looking for open issues with XPCNativeWrapper.

We are trying to run jQuery 1.4.2 and use XPCNativeWrapper. We are failed when jQuery examines wrapped DOM element, with NS_ERROR_NOT_AVAILABLE.  See http://code.jquery.com/jquery-1.4.2.js where it does:

var eventSupported = function( eventName ) { 
    var el = document.createElement("div"); 
    eventName = "on" + eventName; 

    var isSupported = (eventName in el); 

Here, 'document' is an XPCNativeWrapped object, and 'eventName in el' triggers NS_ERROR_NOT_AVAILABLE.

For us, this is indicative of a larger problem.  JavaScript code written for client-side environments (jQuery, jQuery UI, plug-ins etc. etc.) just does not appear to run easily when window is an XPCNativeWrapper.

The consequence is that people are required to wrappedJSObject for all but the most trivial tasks, which in essence renders XPCNativeWrapper useless. We certainly cannot afford to waste days and days debugging XPCNativeWrapper related problems - we'll just turn it off.
I have no idea who "we" is in comment 4 or why anyone should care about this "we", but if you're trying to use jquery against XPCNativeWrapper, you will almost certainly lose.  The point of XPCNativeWrapper is to expose a safe view of the DOM to the app; one that is completely unaffacted by any JS that might have run against that DOM and is unaffected by DOM quirks that override DOM APIs.  So for example, given a form f, XPCNativeWrapper(f).elements.length is the length of the elements list of f, which is NOT necessarily the case for normal DOM access.

Now jquery is very clever and tries to use all sorts of corner cases of the DOM, many of which are very explicitly and purposefully blocked by XPCNativeWrapper, because exposing them would violate the guarantees XPCNativeWrapper is supposed to provide.
"We" is anybody who does serious content script creation: GreaseMonkey, the userchrome project who submitted this bug report, and probably many others.  We specifically are LibX (libx.org) - our goal is to create an end-user programming framework for librarians to create and combine user scripts that integrate pointers to library resources into web pages.

I'm a bit concerned by your attitude towards this - do you expect content script developers to use a style of JavaScript programming that was common in 2003, before the advent of jQuery and libraries like it?  In other words, we are not supposed to take advantage of libraries such as jQuery UI and the many jQuery plug-ins? I note that even Mozilla's now defunct ubiquity project integrated jQuery - presumably an older or a tweaked version. A quick Google search reveals that Jetpack is facing similar problems - with jQuery not working out of the box, requiring tweaks and special versions; currently, integrating jQuery in Jetpack is a work in progress.  And that's a Mozilla project.

I also do not understand why 'el in dom' should be considered a corner case. It works in regular JavaScript.  You could implement XPCNativeWrapper's functionality just the same, hiding any the changes the page may have made.

Oh, and "we" do not have these problems in LibX for Google Chrome's isolated worlds.  jQuery there works just fine, and we are protected from any changes the page may have made. It vastly increases our productivity when writing user scripts.
We're getting really far afield from this bug here; I suggest bringing up your concerns in the .platform newsgroup or dev-platform mailing list.

But the short answer is I don't "expect" you do anything.  I'm just saying that I know jquery depends on certain DOM operations that are inherently unsafe in the sense of "safe" XPCNativeWrapper is supposed to provide, so you may not be able to use them together.  If that means you need to turn off XPCNativeWrapper, then that's clearly your decision.  If you think that some other sense of "safe" should be being provided by some other wrapper object, then we should talk about that in the newsgroup/maillist, not in this bug which is about XPCNativeWrapper.
Oh, and to answer the technical question: XPCNativeWrapper is supposed to provide access to IDL-declared DOM APIs.  on* properties are not, at this time, IDL-declared DOM APIs, though HTML5 may be changing some of that.  They're just regular "expando" properties on the page's JSObjects, which the page can overwrite or redefine in various ways.

Note that unlike in webkit your "eventName in el" code above will return false in Gecko if you run it in a webpage.  Try it: open Firefox and Chrome/Safari and run 

  javascript:alert("onclick" in document.createElement("div"))

in the url bar.  XPCNativeWrapper is just reflecting that behavior difference across the chrome boundary.  Now it should be returning false, not throwing.  That's what this bug is about, and why it's still open; see comment 2.  But the behavior difference will remain, of course.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Thanks for your reply. You're right, it is a platform concern.  I view XPCNativeWrapper as the basis for providing an isolated world that resembles, and is mostly compatible with, the environment presented to regular, client-side JavaScript. Certainly, blog posts such as this one: http://blog.mozilla.com/mrbkap/2010/02/11/xpcnativewrappersno-going-away/ seemed to encourage the use of XPCNativeWrapper to represent the window object in sandboxes created via Components.utils.Sandbox. Perhaps I am mistaken and their only purpose is to provide access to IDL-declared DOM APIs, and not to be mostly compatible with the window object seen by client-side JavaScript.

Let me create a cross reference in this bug report to another bug report that deals with expando properties in XPCNativeWrappers; see Bug 478529. (This has to do with this bug only inasmuch it provides another example of XPCNativeWrapper's unexpected behavior and lack of documentation.)
Yeah, we should talk in .platform.  XPCNativeWrapper is not meant to provide what you want, though its .wrappedJSObject more or less is.
Unless I'm misunderstanding what you want.  Again, that's not a discussion for this bug.  Let's take it to the maillist/newsgroup.
Duplicate of this bug: 614757
There are actually three issues here:

1. accessing onevent attributes throws an exception;

2. wrappers don't provide the kind of jQuery-compatible "isolated world" that addon developers (and the developers of addon development platforms, such as Jetpack, userChromeJS, and LibX) want in order to expose content to chrome and addons more safely;

3. calling .getElementById() on an unwrapped document object returns an xray-wrapped object instead of an unwrapped object.

This last issue hasn't been mentioned in this bug yet but is the reason I filed duplicate bug 614757, which probably isn't a duplicate after all, although I overreduced its testcase until it appeared to turn into one (since I mistakenly thought the issues were the same).

In Firefox 4.0b6, where document objects are wrapped with XPCNativeWrappers, calling .getElementById() on an unwrapped document object in a chrome context returns an unwrapped object.

In Firefox 4.0b7 (and tip), on the other hand, where document objects are wrapped with XrayWrappers, calling .getElementById() on an unwrapped document object in a chrome context returns an xray-wrapped object.

So even if one loads jQuery in a chrome context that has access to an *unwrapped* document object (and window, etc.), jQuery is going to end up accessing wrapped objects and then trip over issue #1 with them.

I've updated the testcase I originally attached to bug 614757 to show this problem, and I am attaching it here.  It actually tests both issues #1 and #3.  It fails the test for issue #1 on 4.0b6, 4.0b7, and the tip.  It only fails the test for issue #3 on 4.0b7 and the tip.

Steps to Reproduce:

1. clone the Add-on SDK (hg clone https://hg.mozilla.org/labs/jetpack-sdk/);
2. enter the SDK directory and apply the testcase patch (cd jetpack-sdk; hg import --no-commit /path/to/test-wrappers.diff);
3. activate the SDK (source bin/activate);
4. enter the jetpack-core package directory and run the tests (cd packages/jetpack-core/; cfx test -f wrappers -b /path/to/firefox/binary).

Expected Results: all tests pass in the "test getting DOM object from unwrapped document" test function.

Actual Results: the "document element retrieved from unwrapped document via .getElementByID() is an [object HTMLHtmlElement]]" test fails.


Boris: would you prefer that I reopen bug 614757 and focus it on issue #3, leaving this bug to focus on issue #1 (with issue #2 to be addressed via discussion in .platform)?
> Boris: would you prefer that I reopen bug 614757 and focus it on issue #3,

Yes!  That sounds like a bad regression in wrapper behavior; we need to at least figure out whether it was intended.
(In reply to comment #14)
> > Boris: would you prefer that I reopen bug 614757 and focus it on issue #3,
> 
> Yes!  That sounds like a bad regression in wrapper behavior; we need to at
> least figure out whether it was intended.

Done!
No longer blocks: 604641
Whats the status of this bug?
If this is still an issue, we need a fix.
blocking2.0: --- → ?
From my point of view, I'm still unaware of a solution in Firefox that provides extensions with an environment that's comparable to Chrome's "isolated worlds" in which content scripts run.

Such a solution must protect extensions from ill-behaved web pages that may try to compromise users running a particular extension, but it must also be compatible with a regular client-side JavaScript environment to the degree that libraries designed for those environments (e.g., jQuery) can run unchanged.
Godmar, simply working with safe js object wrappers (not with xray wrappers) will give you such an environment.  Though note that the web page will be able to mess with DOM access... but allowing that is a prerequisite if you expect jQuery to work, due to the way jQuery operates.
Could you elaborate what you mean by "safe js object wrappers?" - a pointer to documentation may be helpful.
If you're touching an "unwrapped" object from chrome (e.g. after doing .wrappedJSObject), you're working with a safe js object wrapper.  They guarantee nothing about the values you'll get back from the DOM node, but they do guarantee that any scripts involved (getters, setters, etc) will run with the page's permissions and will not be able to execute code with system privileges.
Blake will investigate this and based on that we'll determine whether this is a blocker or not.
Assignee: nobody → mrbkap
Ok - I'm slow. We are now using .wrappedJSObject, but don't understand where the safety in it lies.  Safe compared to what? I thought it was unsafe because it exposes extension code directly any changes, such as expando properties, getters/setters etc. that may have been made by client-side code loaded from the web site the user is visiting.

So, I think the need for a safe environment - like Google's isolated worlds persists, and this should perhaps be made a request for enhancements.
> Safe compared to what?

Safe compared to direct access to the untrusted objects.

> I thought it was unsafe because it exposes extension code directly any changes,
> such as expando properties, getters/setters etc.

"onclick" is an expando property.... so if you want access to it, you need to expose expando properties.
Assignee: mrbkap → gal
Speculatively blocking, through this might be a WONTFIX (check-raise!)
Assignee: gal → mrbkap
blocking2.0: ? → final+
Whiteboard: [softblocker][needs investigation]
Myk seems to be able to reproduce this. I will work with him.
I think the fact that the access _throws_ should be fixed.  What it should _return_ is a separate issue.
bz, what kind of wrapper is throwing here?
As originally filed, against 3.6, XPCNativeWrapper.

In the new world, presumably XRayWrapper.
So this is not a regression? This is a long standing bug?
As far as I can tell, yes.
Comment on attachment 493436 [details] [diff] [review]
testcase that shows regression in 4.0b7

Marking this obsolete, as the bug shown here was fixed in bug 614757.
Attachment #493436 - Attachment is obsolete: true
The actual bug here was fixed (bug 614757). The remainder is a long-standing bug and not a regression. Unblocking.
blocking2.0: final+ → .x
Whiteboard: [softblocker][needs investigation]
Summary: Can’t access to "onclick" property of XPCNativeWrapped objects in chrome context → Can’t access the "onclick" property of XPCNativeWrapped objects in chrome context
This was fixed by bug 659350.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.