Last Comment Bug 425139 - XPCSafeJSObjectWrapper provides incorrect type information
: XPCSafeJSObjectWrapper provides incorrect type information
Status: RESOLVED FIXED
[firebug-p1]
:
Product: Core
Classification: Components
Component: XPConnect (show other bugs)
: Trunk
: All All
: P2 normal (vote)
: mozilla1.9
Assigned To: Mike Shaver (:shaver -- probably not reading bugmail closely)
:
Mentors:
Depends on: 441714
Blocks: 423947
  Show dependency treegraph
 
Reported: 2008-03-25 22:40 PDT by John J. Barton
Modified: 2010-09-18 01:17 PDT (History)
9 users (show)
dsicore: blocking1.9+
gavin.sharp: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Test extension (3.76 KB, application/octet-stream)
2008-03-26 15:15 PDT, John J. Barton
no flags Details
testExtension@mozilla.org in a zip file (5.44 KB, application/octet-stream)
2008-03-30 22:32 PDT, John J. Barton
no flags Details
unwrap for typeof (930 bytes, patch)
2008-04-10 12:38 PDT, Mike Shaver (:shaver -- probably not reading bugmail closely)
mrbkap: review+
mbeltzner: approval1.9+
Details | Diff | Review
Mochitest (1.12 KB, patch)
2008-04-10 18:25 PDT, Blake Kaplan (:mrbkap) (please use needinfo!)
shaver: review+
Details | Diff | Review

Description John J. Barton 2008-03-25 22:40:44 PDT
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.
Comment 1 Mike Shaver (:shaver -- probably not reading bugmail closely) 2008-03-26 09:59:00 PDT
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

Comment 2 John J. Barton 2008-03-26 13:21:51 PDT
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

Comment 3 John J. Barton 2008-03-26 15:15:15 PDT
Created attachment 311882 [details]
Test extension

This should have been a test for the bug, if it worked.
Comment 4 John J. Barton 2008-03-26 15:18:56 PDT
I created a test extension, but I cannot find the window objects. This wrapper stuff makes working in extensions way harder. 
Comment 5 John J. Barton 2008-03-30 09:22:36 PDT
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.
Comment 7 John J. Barton 2008-03-30 22:35:03 PDT
(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;


Comment 8 Damon Sicore (:damons) 2008-04-01 14:44:28 PDT
+'ing this so we can determine what needs to be done.  bkap, thoughts?
Comment 9 Mike Shaver (:shaver -- probably not reading bugmail closely) 2008-04-07 14:47:38 PDT
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. :/
Comment 10 Jonas Sicking (:sicking) PTO Until July 5th 2008-04-07 18:14:53 PDT
Shaver says he'll apply his l33t skillz to this bug
Comment 11 Damon Sicore (:damons) 2008-04-08 12:44:43 PDT
Fixing the priority.  P2.
Comment 12 Mike Shaver (:shaver -- probably not reading bugmail closely) 2008-04-10 12:38:50 PDT
Created attachment 314930 [details] [diff] [review]
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?  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.
Comment 13 Mike Shaver (:shaver -- probably not reading bugmail closely) 2008-04-10 13:08:38 PDT
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. :)
Comment 14 John J. Barton 2008-04-10 13:25:44 PDT
(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 ;-)
Comment 15 Mike Shaver (:shaver -- probably not reading bugmail closely) 2008-04-10 13:44:32 PDT
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! :)
Comment 16 Mike Shaver (:shaver -- probably not reading bugmail closely) 2008-04-10 18:12:37 PDT
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.
Comment 17 Blake Kaplan (:mrbkap) (please use needinfo!) 2008-04-10 18:25:19 PDT
Created attachment 315013 [details] [diff] [review]
Mochitest

Here's a mochitest for this bug.
Comment 18 Mike Shaver (:shaver -- probably not reading bugmail closely) 2008-04-10 18:40:20 PDT
Comment on attachment 315013 [details] [diff] [review]
Mochitest

r=shaver, want to add one for XML too?
Comment 19 Mike Beltzner [:beltzner, not reading bugmail] 2008-04-10 20:56:43 PDT
Comment on attachment 314930 [details] [diff] [review]
unwrap for typeof

a=beltzner, and for the mochitest that goes with it!
Comment 20 Mike Shaver (:shaver -- probably not reading bugmail closely) 2008-04-10 20:58:03 PDT
Thanks, flagging for commit.  (I won't be able to watch the tree to cover it tomorrow until rather late.)
Comment 21 John J. Barton 2008-04-11 14:09:45 PDT
(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! 
Comment 22 :Gavin Sharp [email: gavin@gavinsharp.com] 2008-04-13 18:18:59 PDT
mozilla/js/src/jsapi.c 	3.442
mozilla/js/src/xpconnect/tests/mochitest/test_wrappers.html 	1.5

Note You need to log in before you can comment on or make changes to this bug.