http://www.whatwg.org/specs/web-apps/current-work/multipage/urls.html#dom-tokenlist-item The following test fails document.body.className = 'a b'; is(document.body.classList[2], null, 'Wrong value for out of bounds index'); is(document.body.classList[-1], undefined, 'Wrong value for non index access');
Actually, in this case document.body.classList[2] should return undefined (but document.body.classList.item(2) should return null) according to the Web IDL spec (if that's still true in the latest version, see bug 501257 comment 6 for the explanation).
I think the first cause of action should be to bring this up to the public-webapps@w3.org and/or public-script-coord@w3.org lists and get clarification on what the defined behavior should be. For example should classList[-1] behave the same as classList.item(-1)? And if so, should that throw or be the same as classList.item(-1 >>> 0) == classList.item(2^32 - 1)? And what about classList.item[1.5]?
(In reply to comment #2) > For example should classList[-1] behave the same as classList.item(-1)? And if > so, should that throw or be the same as classList.item(-1 >>> 0) == > classList.item(2^32 - 1)? > > And what about classList.item[1.5]? I don't know if this was brought up on the list, but: classList[-1] should return undefined, because there is no property named "-1" classList.item(-1) should return null, because ToUint32(-1) is equal to or greater than the number of tokens, which HTML5 says means null is returned. classList.item(-1 >>> 0) returns null, since it's the same as the previous case. classList.item[1.5] returns undefined, because there is no property named "1.5" on the Function object for "item". :) classList.item(1.5) returns whatever classList.item(1) returns. There was some recent discussion on how exactly out-of-range numbers are coerced into unsigned longs. The spec currently says to do a ToUint32 on it. But perhaps that should change: http://krijnhoetmer.nl/irc-logs/whatwg/20090714#l-102 http://www.w3.org/mid/Pine.LNX.4.62.0907140132530.9397@hixie.dreamhostps.com
Created attachment 534102 [details] [diff] [review] Patch v1 (code changes)
Created attachment 534103 [details] [diff] [review] Patch v1 (tests)
Comment on attachment 534102 [details] [diff] [review] Patch v1 (code changes) What about other users of nsStringArraySH? Is it specified for those cases that undefined can be returned? Would it make sense to add a new success code which nsDOMTokenList::Item could return in some case; NS_SUCCESS_UNDEFINED. Then this code could check that, and return undefined, or if the string is null, return null, otherwise the string.
Looking at the other things this patch affects: * nsHistorySH (History): no idea what this is supposed to be, it isn't in the spec and I've only got SECURITY_ERRs out of it * nsStringListSH (DOMStringList): needs this fix * nsMediaListSH (MediaList) and nsCSSStyleDeclSH (CSSStyleDeclaration): don't have the actual definitions WebIDL wants, but would need the fix * nsOfflineResourceListSH: proprietary, but should match DOMStringList, IMO So I'd rather change these cases (and add tests) than leave them returning the empty string. Is that okay with you?
Yes, changing all the cases and adding tests sounds ok.
Created attachment 535936 [details] [diff] [review] Patch v2, with tests
Comment on attachment 535936 [details] [diff] [review] Patch v2, with tests Is CSSStyleDeclaration [] behavior defined anywhere? Should we open a w3c bug to fix it to work like other interfaces?
Comment on attachment 535936 [details] [diff] [review] Patch v2, with tests Actually, the patch does something else than what the summary says. undefined might be returned even when inside the bounds.
Are there any cases when Item() might return null when inside the bounds?
I'm pretty sure that can't happen.
Comment on attachment 535936 [details] [diff] [review] Patch v2, with tests OK. Could we add some #ifdef DEBUG code to each GetStringAt implementation to check that null is returned only when index out of bounds.
http://hg.mozilla.org/mozilla-central/rev/2cd5af3aca9a
Documentation updated by Trevor.
dom/tests/mochitest/bugs/test_bug529328.html was run and it passed on: Mozilla/5.0 (Windows NT 6.1; rv:7.0) Gecko/20100101 Firefox/7.0 Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:7.0) Gecko/20100101 Firefox/7.0 Mozilla/5.0 (X11; Linux i686; rv:7.0) Gecko/20100101 Firefox/7.0 Mozilla/5.0 (Windows NT 5.1; rv:7.0) Gecko/20100101 Firefox/7.0 + Aurora (Fx 8.0a2) & Central (Fx 9.0a1)
or
