XPCSafeJSObjectWrapper provides incorrect type information

RESOLVED FIXED in mozilla1.9

Status

()

Core
XPConnect
P2
normal
RESOLVED FIXED
10 years ago
7 years ago

People

(Reporter: John J. Barton, Assigned: shaver)

Tracking

Trunk
mozilla1.9
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [firebug-p1])

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

10 years ago
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.
(Reporter)

Updated

10 years ago
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

(Reporter)

Comment 2

10 years ago
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

(Reporter)

Comment 3

10 years ago
Created attachment 311882 [details]
Test extension

This should have been a test for the bug, if it worked.
(Reporter)

Comment 4

10 years ago
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
(Reporter)

Comment 5

10 years ago
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?
(Reporter)

Updated

10 years ago
Attachment #311882 - Attachment is obsolete: true
(Reporter)

Comment 6

10 years ago
Created attachment 312668 [details]
testExtension@mozilla.org in a zip file

https://fireclipse.svn.sourceforge.net/svnroot/fireclipse/trunk/FireclipseExtensions/chromebug/test/bugzilla-425139
(Reporter)

Comment 7

10 years ago
(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
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.
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. :)
(Reporter)

Comment 14

10 years ago
(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)

Updated

10 years ago
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?
Created attachment 315013 [details] [diff] [review]
Mochitest

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
(Reporter)

Comment 21

10 years ago
(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
Last Resolved: 10 years ago
Flags: in-testsuite+
Keywords: checkin-needed
OS: Windows XP → All
Hardware: PC → All
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9
(Reporter)

Updated

10 years ago
No longer blocks: 411814
Depends on: 441714

Updated

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