412150 - apple.com - someNodeList[-1] assumes return of last time (tabs on page don't work)

Status: RESOLVED INCOMPLETE

(Tech Evangelism Graveyard :: English US, defect, P2)

Description

[Samuel Sidler (old account; do not CC)]

The tabs on the page on the URL don't work on trunk but do on branch.

STR:
  1. Go to http://www.apple.com/macpro/performance.html
  2. Click on any unselected tab under the Video heading.

AR:
Nothing happens, though hover effects do exist.

ER:
The content under the tab should change like it does in Firefox 2.

Not sure what's going on here, but I'm requesting blocking anyway since this is a regression from the branch and Apple is a top100 site on Alexa.

Comment 1

[Mats Palmgren (inactive)]

WFM, Firefox 2008011204 on both MacOSX (10.5.1) and Linux.

Comment 2

[Samuel Sidler (old account; do not CC)]

So, this actually works for the "Video" heading now, though just the tabs. The sub-nav "Select your system" does not work and tabs under other sections do not work.

Comment 3

[Mats Palmgren (inactive)]

I can reproduce the problem with the modified STR in comment 2.
The regression window is: 2007-02-01-04 -- 2007-02-02-04
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2007-02-01+03%3A00&maxdate=2007-02-02+05%3A00&cvsroot=%2Fcvsroot
Nothing in that range jumps out at me though.
A testcase would be nice...

Comment 4

[Mats Palmgren (inactive)]

http://www.apple.com/global/scripts/content_swap.js

I'm guessing the problem occurs because we implemented
getElementsByClassName() in the range above: bug 357450

Evang?

Comment 5

[Mike Schroepfer]

(In reply to comment #4)
> http://www.apple.com/global/scripts/content_swap.js
> 
> I'm guessing the problem occurs because we implemented
> getElementsByClassName() in the range above: bug 357450
> 
> Evang?
> 

Why does that break their site?

Comment 6

[Mats Palmgren (inactive)]

The error is in the 'setMouseover' function in
http://www.apple.com/global/scripts/content_swap.js
the loop will run until 'i' is -1 and 'this.selectorList[i]' will then throw:

Error: uncaught exception: [Exception... "Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsIWebNavigation.loadURI]"  nsresult: "0x80070057 (NS_ERROR_ILLEGAL_VALUE)"  location: "JS frame :: chrome://global/content/viewSource.js :: viewSource :: line 152"  data: no]

which aborts the 'load' handler here:
http://www.apple.com/macpro/scripts/performance.js
and this is why only the first "tab object" works.

Changing "selector = this.selectorList[i]" to
"i >= 0 && (selector = this.selectorList[i])" fixes it.

I tested trunk builds before bug 357450, and also Firefox 2.0.0.11, and it
appears "document.getElementsByClassName" is implemented -- the difference
seems to be that we got 'null' rather than throwing when indexing the
result object from getElementsByClassName() with an out of range index...

So, maybe this is a regression after all?  Robert?

Comment 7

[Mats Palmgren (inactive)]

Sorry, I pasted wrong exception above, this is the correct one:

[Exception... "Index or size is negative or greater than the allowed amount"  code: "1" nsresult: "0x80530001 (NS_ERROR_DOM_INDEX_SIZE_ERR)"  location: "file:///usr/local/test/412150_files/content_swap.js Line: 40"]

Comment 8

[Robert Sayre]

ug, so possible dupe of bug 390411.

Comment 9

[Boris Zbarsky [:bzbarsky]]

> ug, so possible dupe of bug 390411.

It's not.  See bug 390411 comment 20.

Comment 10

[Arjan]

Attachment: Testcase

In firefox2, it says in the textfields:
Text1
Text2
Empty

In firefox3, it says:
Text1
Text2
boom pats exception thrown

Comment 11

[Daniel Holbert [:dholbert]]

(In reply to comment #10)
Some clarification on the behavior of this testcase, at least on my machine:
 - In the textboxes, I actually get "Text2" before "Text1".
 - In FF3, third textbox *is* empty; in FF2, it contains the string "empty"
 - In FF3, a JS exception (see comment 7) appears in the error console

Comment 12

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

blocking, assigning to jst for resolution

Johnny, should we evangelize here, or fix the way we've implemented this?

Comment 13

[Johnny Stenback (:jst)]

I think we should evangelize here. document.getElementByClassName() returns a NodeList, and their [] operator simply shortcuts to the .itemAt() method which takes an unsigned value. Nowhere else have I ever seen anyone assuming someNodeList[-1] would return the last item.

Anyone else have other thoughts?

Comment 14

[Johnny Stenback (:jst)]

Over to tech evang, nothing we're going to do about this in the code.

Comment 15

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Apparently this affects all iWeb-designed sites, for whatever that's worth.

https://bugzilla.mozilla.org/show_bug.cgi?id=425752

Comment 16

[Adam Guthrie]

So even though the spec. says that NodeList's item() method shouldn't throw any exceptions if the index invalid, our behavior is OK? http://www.w3.org/TR/DOM-Level-2-Core/core.html#ID-536297177

Comment 17

[Chris Lawson (gone)]

INCOMPLETE due to lack of activity since the end of 2009.

If someone is willing to investigate the issues raised in this bug to determine whether they still exist, *and* work with the site in question to fix any existing issues, please feel free to re-open and assign to yourself.

Sorry for the bugspam; filter on "NO MORE PRE-2010 TE BUGS" to remove.