Bug 118561 – NamedFlowCollection getters should follow the same pattern as HTMLCollection

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Radu Stavila]

When there's no named flow in the collection:

collection[42] === undefined
collection['fooBar'] === undefined
collection.item(42) === null
collection.namedItem('fooBar') === null


https://www.w3.org/Bugs/Public/show_bug.cgi?id=22617

[Radu Stavila]

Created attachment 206456 [details]
Patch

[Chris Dumez]

Maybe we should wait until we get resolution on the W3C bug, just in case?

[Radu Stavila]

The discussion was on the www-style list, Alan already decided to roll back the modification. I already emailed him to post a comment on the w3 bug so it's clear to everybody, he will probably do it when he wakes up.

[Chris Dumez]

Comment on attachment 206456 [details]
Patch

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

> LayoutTests/fast/regions/webkit-named-flow-collection-empty-getters.html:4
> +

nit: extra line

> LayoutTests/fast/regions/webkit-named-flow-collection-empty-getters.html:7
> +            if (window.testRunner)

nit: We probably don't need these 2 lines, I believe js-test-pre.js takes care of it.

> LayoutTests/fast/regions/webkit-named-flow-collection-empty-getters.html:18
> +    <script src="../../fast/js/resources/js-test-post.js"></script> 

nit: weird identation

> Source/WebCore/dom/DOMNamedFlowCollection.idl:-39
> -    [TreatReturnedNullObjectAs=Undefined] getter WebKitNamedFlow namedItem(DOMString name);

We should drop support for this extended attribute as it is no longer used and was introduced just for this special case.

[Alexandru Chiculita]

Comment on attachment 206456 [details]
Patch

r=me

[Alexandru Chiculita]

(In reply to comment #4)
> (From update of attachment 206456 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=206456&action=review
> 
Didn't see your review. I've set the cq-.

Do you want to drop that in this patch? I would put it in a different one, so that we can revert it when needed in the future.

[Chris Dumez]

(In reply to comment #6)
> (In reply to comment #4)
> > (From update of attachment 206456 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=206456&action=review
> > 
> Didn't see your review. I've set the cq-.
> 
> Do you want to drop that in this patch? I would put it in a different one, so that we can revert it when needed in the future.

well, this is basically a revert of <http://trac.webkit.org/changeset/152466> so I think it makes sense. Alternatively, we can simply revert the previous patch and merely add the test.

[Alexandru Chiculita]

Comment on attachment 206456 [details]
Patch

Let's remove the code in the generator too.

[Radu Stavila]

Created attachment 206458 [details]
Patch which also removes generator code

[Chris Dumez]

Comment on attachment 206458 [details]
Patch which also removes generator code

Thanks. Looks good. r=me.

[WebKit Commit Bot]

Comment on attachment 206458 [details]
Patch which also removes generator code

Clearing flags on attachment: 206458

Committed r152569: <http://trac.webkit.org/changeset/152569>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.