477431 - Inspector gets confused as to whether a node has children

Status: VERIFIED FIXED

(Core :: XPConnect, defect)

Description

[Alex Vincent [:WeirdAl]]

Attachment: testcase document

Steps to reproduce:
(1) Open the grid-navigable.xul attachment in DOM Inspector.
(2) In the DOM Nodes panel on the left, navigate to window > gridbox > stack > second anonymous grid > columns.
(3) Switch to JS Object Viewer in the right-hand panel.

Expected results:  DOM Nodes panel shows the columns element as having children, and is expandable.  JS Object Viewer panel shows the childNodes property as having children and is also expandable.

Actual results:  DOM Nodes panel shows the columns element as having no children.  JS Object Viewer panel shows the childNodes property as having children, but you cannot expand the child rows beneath it.

Comment 1

[Alex Vincent [:WeirdAl]]

Attachment: patch for JS Object Viewer

I don't have a patch for the DOM Nodes viewer yet.

Comment 2

[Shawn Wilsher :sdwilsh]

Comment on attachment 361106 [details] [diff] [review]
patch for JS Object Viewer

r=sdwilsh, if you update the javadoc comment above the method, and add a comment stating why you are forcing everything to a string.

Comment 3

[[e-mail]]

I don't think this is right. For some reason, the enumerator for an XPCSafeJSObjectWrapper for childNodes is returning the properties as numbers, not strings. For example, evaluate these in the JS console (opening the JS console from a blank browser tab for consistency):

>for (var prop in XPCSafeJSObjectWrapper(top.opener.content.document.childNodes)) dump(prop + "(" + (typeof prop) + ")\n")
0(number)
1(number)
length(string)
item(string)

>for (var prop in XPCNativeWrapper(top.opener.content.document.childNodes)) dump(prop + "(" + (typeof prop) + ")\n")
0(string)
1(string)

>for (var prop in (top.opener.content.document.childNodes)) dump(prop + "(" + (typeof prop) + ")\n")
0(string)
1(string)
length(string)
item(string)

Comment 4

[[e-mail]]

Comment on attachment 361106 [details] [diff] [review]
patch for JS Object Viewer

This is actually the wrong fix for two reasons. The underlying bug is that XPConnect is enumerating numeric properties, although ES3 requires strings. Even if numeric properties were expected, we would want to stringise them during enumeration and not once every comparison.

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

I got this one.

Comment 6

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Fix

Comment 7

[Blake Kaplan (:mrbkap) (inactive)]

This doesn't block, but it's a potential compatibility problem. I'll request approval for the 1.9 branch after I get this on trunk.

Comment 8

[Blake Kaplan (:mrbkap) (inactive)]

http://hg.mozilla.org/mozilla-central/rev/48f66b2b3c61

Comment 9

[Alex Vincent [:WeirdAl]]

I wonder what an automated test for this would look like.  The testcase in comment 0 is clearly too big, and requires DOM Inspector to actually test.

Comment 10

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Mochitest

This could be done with an xpcshell "simple" test, but since we already have the test_wrapper.html file, this will do.

Comment 11

[Blake Kaplan (:mrbkap) (inactive)]

http://hg.mozilla.org/mozilla-central/rev/2357b4fd2560

Comment 12

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 361874 [details] [diff] [review]
Fix

a191=beltzner, please make sure to also land the test

Comment 13

[Blake Kaplan (:mrbkap) (inactive)]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/e0d8d4951c82 and http://hg.mozilla.org/releases/mozilla-1.9.1/rev/f63833f5f97d

Comment 14

[Aakash Desai [:aakashd]]

verified FIXED on builds:

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090721
Minefield/3.6a1pre ID:20090721044139

and

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.1pre) Gecko/20090720
Shiretoko/3.5.1pre ID:20090720042942

Comment 15

[Alex Vincent [:WeirdAl]]

mrbkap: I think the Mochitest you landed for this bug (comment 10) is actually not testing the number correctly.

js> var x = {0: 3};
js> var y = "";
js> for (var prop in x) { y += (typeof prop) + "\n"; }
string

js> y
string

In other words, the property names are strings before you pass them into XPCSafeJSObjectWrapper(...).

Comment 16

[Blake Kaplan (:mrbkap) (inactive)]

Alex, your test doesn't work the way you think it does -- for-in string-izes for you. Internally, we keep the properties as their original types and only convert when they become user-visible.