itemType[index] must be undefined for out-of-range index

[Arko Saha]

Fail- itemType[index] must be undefined for out-of-range index assert_equals: expected (undefined) undefined but got (object) null

Sample test:
test(function () {
    assert_equals( makeEl('div',{itemtype:' '}).itemType[0], window.undefined );
}, 'itemType[index] must be undefined for out-of-range index');


Expected result: itemTpye[0] should return undefined.
Actual: Returns null.

makeEl method creates an element <div> with empty itemtype(0 tokens).

According to the spec itemtpye attribute is a space-separated list. http://www.whatwg.org/specs/web-apps/current-work/#attr-itemtype
We have defined itemtype attribute as :
readonly attribute [Conditional=MICRODATA] DOMSettableTokenList itemType;

itemType[index] should return undefined for out-of-range index instead of null.

IndexedGetter spec: http://dev.w3.org/2006/webapi/WebIDL/#idl-indexed-properties

Same issue can also be observed with following attributes:
itemref,
itemprop

[Arko Saha]

Created attachment 156326 [details]
Patch

[WebKit Review Bot]

Comment on attachment 156326 [details]
Patch

Attachment 156326 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13425636

New failing tests:
storage/indexeddb/objectstore-basics-workers.html
storage/indexeddb/database-basics.html
storage/indexeddb/objectstore-basics.html

[WebKit Review Bot]

Created attachment 156336 [details]
Archive of layout-test-results from gce-cr-linux-04

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-04  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid

[Arko Saha]

Created attachment 156358 [details]
Patch for review

[Kentaro Hara]

Comment on attachment 156358 [details]
Patch for review

View in context: https://bugs.webkit.org/attachment.cgi?id=156358&action=review

You change is conformed to the spec, but this will break backward compatibility of WebKit.

How are Firefox, Opera, IE behave? (Please add the explanation to ChangeLog.)

> Source/WebCore/ChangeLog:9
> +        Made changes in IndexedGetter property so that it returns undefined for out-of-range
> +        index.

Please add the spec link (http://www.w3.org/TR/WebIDL/#idl-indexed-properties) to ChangeLog.

[Arko Saha]

Created attachment 156388 [details]
Updated patch

[Arko Saha]

(In reply to comment #5)
> (From update of attachment 156358 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=156358&action=review
> 
> You change is conformed to the spec, but this will break backward compatibility of WebKit.
> 
> How are Firefox, Opera, IE behave? (Please add the explanation to ChangeLog.)

Firefox and Opera's behavior is consistent with the spec. Both returns undefined for out-of-range index.
Tested on:
Firefox: 11.0
Opera: 12.01

> > Source/WebCore/ChangeLog:9
> > +        Made changes in IndexedGetter property so that it returns undefined for out-of-range
> > +        index.
> 
> Please add the spec link (http://www.w3.org/TR/WebIDL/#idl-indexed-properties) to ChangeLog.

Done.

[Kentaro Hara]

Comment on attachment 156388 [details]
Updated patch

Thank you very much for the clarification. Sounds reasonable to make this change.

But let's wait to cq+ it for one more day because someone might have concern about breaking the backward compatibility.

[Alexey Proskuryakov]

> > How are Firefox, Opera, IE behave? (Please add the explanation to ChangeLog.)
> 
> Firefox and Opera's behavior is consistent with the spec. Both returns undefined for out-of-range index.
> Tested on:
> Firefox: 11.0
> Opera: 12.01

Did you provide an answer about IE?

[Arko Saha]

(In reply to comment #9)
> Did you provide an answer about IE?

Observed the same behavior in IE. It returns undefined for out-of-range index.
Tested on IE9.

[Ryosuke Niwa]

Has this patch been landed?

[Arko Saha]

(In reply to comment #11)
> Has this patch been landed?

I haven't landed the patch as I was waiting to check if anyone has anymore concern. Looks like its ok to land the patch now.

[Kentaro Hara]

Comment on attachment 156388 [details]
Updated patch

Yes

[WebKit Review Bot]

Comment on attachment 156388 [details]
Updated patch

Rejecting attachment 156388 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1

ERROR: /mnt/git/webkit-commit-queue/LayoutTests/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: http://queues.webkit.org/results/13441921

[Kentaro Hara]

Comment on attachment 156388 [details]
Updated patch

View in context: https://bugs.webkit.org/attachment.cgi?id=156388&action=review

> LayoutTests/ChangeLog:5
> +

The "Reviewed by" line is needed here. Would you upload the patch again?

[Arko Saha]

Created attachment 156859 [details]
Patch for landing

[Arko Saha]

Comment on attachment 156859 [details]
Patch for landing

Patch for fast/dom/MicroData/domsettabletokenlist-attributes-out-of-range-index.html is incorrect.

[Arko Saha]

Created attachment 156865 [details]
Patch_for_landing_1

[WebKit Review Bot]

Comment on attachment 156865 [details]
Patch_for_landing_1

Clearing flags on attachment: 156865

Committed r124859: <http://trac.webkit.org/changeset/124859>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.