Summary: | NamedFlowCollection getters should follow the same pattern as HTMLCollection | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Radu Stavila <stavila> | ||||||
Component: | CSS | Assignee: | Radu Stavila <stavila> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | achicu, arv, cdumez, commit-queue, esprehn+autocc, kangil.han, rniwa, WebkitBugTracker | ||||||
Priority: | P2 | Keywords: | WebExposed | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 57312 | ||||||||
Attachments: |
|
Description
Radu Stavila
2013-07-11 05:39:48 PDT
Created attachment 206456 [details]
Patch
Maybe we should wait until we get resolution on the W3C bug, just in case? 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. 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. Comment on attachment 206456 [details]
Patch
r=me
(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. (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. Comment on attachment 206456 [details]
Patch
Let's remove the code in the generator too.
Created attachment 206458 [details]
Patch which also removes generator code
Comment on attachment 206458 [details]
Patch which also removes generator code
Thanks. Looks good. r=me.
Comment on attachment 206458 [details] Patch which also removes generator code Clearing flags on attachment: 206458 Committed r152569: <http://trac.webkit.org/changeset/152569> All reviewed patches have been landed. Closing bug. |