Closed Bug 425139 Opened 16 years ago Closed 16 years ago

XPCSafeJSObjectWrapper provides incorrect type information

Categories

(Core :: XPConnect, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla1.9

People

(Reporter: johnjbarton, Assigned: shaver)

References

Details

(Whiteboard: [firebug-p1])

Attachments

(3 files, 1 obsolete file)

In chrome code running under xpcnativewrappers, and with an object 'win' referring to a content window, the defined by the content window expression
  this.userObjects = [];
in the object 'console' can be accessed like this by the extension:
 var userObjects = win.wrappedJSObject.console.userObjects;
Unfortunately the resulting object is not an array.  Other types are wrong also.

I'll try to come up with a test case, but this will prevent Firebug from running.
Whiteboard: [firebug-p1]
Do you mean that it reports false for "instanceof Array", or do you mean that you can't call Array methods on it?  The former isn't unlikely, due to the way instanceof is specified (ECMA 262-3 is pretty much hell-bent on there only being one global object), but might be fixable with some hackery on SJOWs.  The latter would be more surprising.

If the issue is instanceof lying, do these work?

userObjects instanceof win.Array
userObjects instanceof win.wrappedJSObject.Array

For your first expression I get 
invalid 'instanceof' operand win.Array;

One form of the problem is that "apply" does not take the object:
 subHandler.apply(context.firebugConsoleHandler, userObjects);
so even if your expression worked, I don't think it would help me: I need userObjects instanceof Array == true

Attached file Test extension (obsolete) —
This should have been a test for the bug, if it worked.
I created a test extension, but I cannot find the window objects. This wrapper stuff makes working in extensions way harder. 
Component: General → XPConnect
Product: Firefox → Core
QA Contact: general → xpconnect
Even though this report is still lame, I am requesting blocking for FF3. Firebug needs either the type information to be fixed or a workaround. This is the only non-crash FF3 problem I can't workaround.
Blocks: 423947
No longer blocks: 423796
Flags: blocking1.9?
Attachment #311882 - Attachment is obsolete: true
(In reply to comment #6)
> Created an attachment (id=312668) 
Ran on 
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008033005 Minefield/3.0pre

Demonstrates that 
typeof(unwrapped.aFunction) 
returns "object" for 
var unwrapped = win.window.wrappedJSObject;


+'ing this so we can determine what needs to be done.  bkap, thoughts?
Assignee: nobody → mrbkap
Flags: blocking1.9? → blocking1.9+
In the absence of a typeof override as requested in Bug 427623, this probably involves forking the SJOW's class so that call+construct are present on one set (used to wrap callables), and not on the other (used to wrap non-callables).  Joy. :/
Shaver says he'll apply his l33t skillz to this bug
Assignee: mrbkap → shaver
Fixing the priority.  P2.
Priority: -- → P2
With this patch I...might pass the test?  The content area says:

  From test.xul we have typeof(aFunction):object

but the console output says

  From test.xul we have typeof(aFunction):function

John: what's the difference supposed to be between those?  I'll pop into the debugger and see if I can figure out what's going on, too.

I would love if someone could turn the test extension into a mochitest, since I'll want that when I land, whether this patch is it or not.
(In reply to comment #12)
> Created an attachment (id=314930) [details]
> unwrap for typeof
> 
> With this patch I...might pass the test?  The content area says:
> 
>   From test.xul we have typeof(aFunction):object
> 
> but the console output says
> 
>   From test.xul we have typeof(aFunction):function
> 
> John: what's the difference supposed to be between those? 

The content area shows the BAD result from current FF3. The console shows your result.  I'm hoping they are different ;-)
A test that always shows a fixed failure message makes me hurt inside.  But I'm glad that the patch seems to work, at least! :)
Attachment #314930 - Flags: review?(mrbkap) → review+
Comment on attachment 314930 [details] [diff] [review]
unwrap for typeof

Blocker with simple fix to make working with wrappers a little bit less painful; requesting approval.
Attachment #314930 - Flags: approval1.9?
Attached patch MochitestSplinter Review
Here's a mochitest for this bug.
Attachment #315013 - Flags: review?(shaver)
Comment on attachment 315013 [details] [diff] [review]
Mochitest

r=shaver, want to add one for XML too?
Attachment #315013 - Flags: review?(shaver) → review+
Comment on attachment 314930 [details] [diff] [review]
unwrap for typeof

a=beltzner, and for the mochitest that goes with it!
Attachment #314930 - Flags: approval1.9? → approval1.9+
Thanks, flagging for commit.  (I won't be able to watch the tree to cover it tomorrow until rather late.)
Keywords: checkin-needed
(In reply to comment #13)
> https://build.mozilla.org/tryserver-builds/2008-04-10_11:47-shaver@mozilla.com-typeofunwrap/
> has try-server builds coming in, if people want to test along from home. :)
> 

That build makes the Firebug DOM panel look normal, thanks! 
mozilla/js/src/jsapi.c 	3.442
mozilla/js/src/xpconnect/tests/mochitest/test_wrappers.html 	1.5
Status: NEW → RESOLVED
Closed: 16 years ago
Flags: in-testsuite+
Keywords: checkin-needed
OS: Windows XP → All
Hardware: PC → All
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9
No longer blocks: 411814
Depends on: 441714
You need to log in before you can comment on or make changes to this bug.