RESOLVED FIXED 160682
getElementsByTagName() should take a qualifiedName in parameter
https://bugs.webkit.org/show_bug.cgi?id=160682
Summary getElementsByTagName() should take a qualifiedName in parameter
Chris Dumez
Reported 2016-08-08 19:30:11 PDT
getElementsByTagName() should take a qualifiedName in parameter, not a localName: - https://dom.spec.whatwg.org/#dom-document-getelementsbytagname - https://dom.spec.whatwg.org/#concept-getelementsbytagname
Attachments
WIP Patch (32.68 KB, patch)
2016-08-11 22:07 PDT, Chris Dumez
no flags
WIP Patch (33.82 KB, patch)
2016-08-11 22:29 PDT, Chris Dumez
no flags
WIP Patch (33.82 KB, patch)
2016-08-11 22:31 PDT, Chris Dumez
no flags
Patch (39.48 KB, patch)
2016-08-12 08:45 PDT, Chris Dumez
no flags
Patch (39.50 KB, patch)
2016-08-12 10:56 PDT, Chris Dumez
no flags
Patch (41.02 KB, patch)
2016-08-12 20:23 PDT, Chris Dumez
no flags
Chris Dumez
Comment 1 2016-08-11 22:07:22 PDT
Created attachment 285894 [details] WIP Patch
Chris Dumez
Comment 2 2016-08-11 22:29:41 PDT
Created attachment 285896 [details] WIP Patch
Chris Dumez
Comment 3 2016-08-11 22:31:26 PDT
Created attachment 285897 [details] WIP Patch
Chris Dumez
Comment 4 2016-08-12 08:45:29 PDT
Chris Dumez
Comment 5 2016-08-12 08:46:00 PDT
This passes all the tests. I'll evaluate performance on Speedometer and Dromaeo today.
Chris Dumez
Comment 6 2016-08-12 10:03:33 PDT
(In reply to comment #5) > This passes all the tests. I'll evaluate performance on Speedometer and > Dromaeo today. Seems to be perf-neutral on Dromaeo DOM core. However, it looks like this is a 1-3% regression on Speedometer. I'll see what I can do to avoid regressing Speedometer.
Chris Dumez
Comment 7 2016-08-12 10:56:40 PDT
Chris Dumez
Comment 8 2016-08-12 10:57:16 PDT
Ok, this now seems to be perf-neutral on Speedometer as well. I'll closely monitor the perf bots when it lands though.
Chris Dumez
Comment 9 2016-08-12 15:55:59 PDT
Ryosuke Niwa
Comment 10 2016-08-12 18:38:45 PDT
Comment on attachment 285920 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=285920&action=review > Source/WebCore/dom/AllDescendantsCollection.h:32 > +// Collection that matches all element descendants. I don't think this comment is necessary given the code below that says: CachedHTMLCollection<AllDescendantsCollection, > Source/WebCore/dom/AllDescendantsCollection.h:33 > +class AllDescendantsCollection final : public CachedHTMLCollection<AllDescendantsCollection, CollectionTypeTraits<AllDescendants>::traversalType> { Can we use this in HTMLAllCollection as well?
Chris Dumez
Comment 11 2016-08-12 19:21:11 PDT
Comment on attachment 285920 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=285920&action=review >> Source/WebCore/dom/AllDescendantsCollection.h:33 >> +class AllDescendantsCollection final : public CachedHTMLCollection<AllDescendantsCollection, CollectionTypeTraits<AllDescendants>::traversalType> { > > Can we use this in HTMLAllCollection as well? Do you mean have HTMLAllCollection subclass AllDescendantsCollection to avoid duplicating the elementMatches() implementation?
Chris Dumez
Comment 12 2016-08-12 20:23:22 PDT
WebKit Commit Bot
Comment 13 2016-08-12 21:22:02 PDT
Comment on attachment 286000 [details] Patch Clearing flags on attachment: 286000 Committed r204441: <http://trac.webkit.org/changeset/204441>
WebKit Commit Bot
Comment 14 2016-08-12 21:22:08 PDT
All reviewed patches have been landed. Closing bug.
Chris Dumez
Comment 15 2016-08-13 15:42:46 PDT
I monitored the perf bots and did not see any obvious regression from this change.
Note You need to log in before you can comment on or make changes to this bug.