Last Comment Bug 310351 - Cannot convert NodeList to JS Array with Array.prototype.slice any more
: Cannot convert NodeList to JS Array with Array.prototype.slice any more
: fixed1.8, regression, testcase
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: x86 Windows XP
-- normal (vote)
: ---
Assigned To: Blake Kaplan (:mrbkap)
: Jason Orendorff [:jorendorff]
Depends on: 379248 311090
  Show dependency treegraph
Reported: 2005-09-28 13:10 PDT by Aiko
Modified: 2007-04-30 04:45 PDT (History)
4 users (show)
bob: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

testcase (442 bytes, text/html)
2005-09-28 13:11 PDT, Aiko
no flags Details
Why is this needed? (896 bytes, patch)
2005-10-03 15:45 PDT, Blake Kaplan (:mrbkap)
no flags Details | Diff | Splinter Review
Because of this (3.81 KB, patch)
2005-10-03 18:27 PDT, Blake Kaplan (:mrbkap)
jst: review+
brendan: superreview+
brendan: approval1.8b5+
Details | Diff | Splinter Review

Description User image Aiko 2005-09-28 13:10:20 PDT
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050908 Firefox/1.4
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050908 Firefox/1.4

Until recently it was possible to convert a NodeList, e.g. the result of
document.getElementsByTagName to a JS array using code like

array =, 0)

This is broken in current trunk builds and Firefox Beta 1. It was still working
in Deer Park Alpha 2. I'm not sure whether other generic JS array methods are
affected, at least join() still works. 

Maybe this was caused by the splitwindows patch, so hopefully placing this in
DOM: Core isn't too far off.

Reproducible: Always

Steps to Reproduce:
1. Run the attached testcase
Actual Results:  
[object HTMLBRElement],[object HTMLBRElement],[object HTMLBRElement]

Expected Results:  
[object HTMLBRElement],[object HTMLBRElement],[object HTMLBRElement]
[object HTMLBRElement],[object HTMLBRElement],[object HTMLBRElement]
Comment 1 User image Aiko 2005-09-28 13:11:17 PDT
Created attachment 197746 [details]
Comment 2 User image Aiko 2005-10-03 14:14:15 PDT
I had some more time to browse through

Works: 1.8b3 20050713
Fails: 1.8b3 20050714

In that time frame, bug 299738 added a PropertyExists check to array_slice, this
looks like a probable cause.
Comment 3 User image Blake Kaplan (:mrbkap) 2005-10-03 15:45:26 PDT
Created attachment 198370 [details] [diff] [review]
Why is this needed?

It appears that OBJ_LOOKUP_PROPERTY is failing us here. For some reason that
I'm not really sure of, it's returning a null property, even though that
property actually does exist. The OBJ_GET_PROPERTY is returning the correct
value, causing us to not think that the collection is empty.
Comment 4 User image Brendan Eich [:brendan] 2005-10-03 16:56:32 PDT
It's always possible for some naughty host object to answer "no" to lookups (by
not resolving) but still give a useful value back, or even reify a property
lazily on get.  Is that the case here?  We should probably back away from the
change to slice, but similar changes or existing patterns occur elsewhere in

Comment 5 User image Blake Kaplan (:mrbkap) 2005-10-03 18:27:35 PDT
Created attachment 198392 [details] [diff] [review]
Because of this

This seems to fix this. I feel like more testing is necessary, but don't have
time. :-(
Comment 6 User image Johnny Stenback (:jst, 2005-10-03 18:30:46 PDT
Comment on attachment 198392 [details] [diff] [review]
Because of this

Comment 7 User image Brendan Eich [:brendan] 2005-10-03 21:39:06 PDT
Comment on attachment 198392 [details] [diff] [review]
Because of this

Great, thanks!

Comment 8 User image Blake Kaplan (:mrbkap) 2005-10-03 22:53:01 PDT
Fix checked into MOZILLA_1_8_BRANCH and trunk.
Comment 9 User image Aiko 2005-10-04 09:47:15 PDT
Verified fixed with today's trunk build, thanks.
Comment 10 User image :Gavin Sharp [email:] 2005-10-04 13:15:18 PDT
Looks as though this may have caused bug 311104.
Comment 11 User image Boris Zbarsky [:bz] (still a bit busy) 2005-10-06 12:23:52 PDT
It looks like this affected Tdhtml some (about 1% or so).  Those testcase do
quite a lot of access to various DOM nodelists, and this patch added some extra
work per each access.  :(
Comment 12 User image 2005-10-08 17:29:34 PDT
(In reply to comment #0)
>array =, 0)
That's neat. Now if only [].concat(nodeList) worked like that ;-)
Comment 13 User image Bob Clary [:bc:] 2005-10-09 19:44:50 PDT
Checking in regress-310351.js;
/cvsroot/mozilla/js/tests/js1_5/Array/regress-310351.js,v  <--  regress-310351.js
initial revision: 1.1
Comment 14 User image Bob Clary [:bc:] 2006-09-14 22:48:14 PDT
The nodeList is live and if the dom changes the test will falsely fail. This saves the nodeList length prior to reportCompare to prevent that.

Checking in regress-310351.js;
/cvsroot/mozilla/js/tests/js1_5/Array/regress-310351.js,v  <--  regress-310351.js
new revision: 1.2; previous revision: 1.1

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