Bug 160682 - getElementsByTagName() should take a qualifiedName in parameter
Summary: getElementsByTagName() should take a qualifiedName in parameter
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: WebExposed
Depends on: 164705
Blocks:
  Show dependency treegraph
 
Reported: 2016-08-08 19:30 PDT by Chris Dumez
Modified: 2016-11-13 19:32 PST (History)
9 users (show)

See Also:


Attachments
WIP Patch (32.68 KB, patch)
2016-08-11 22:07 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
WIP Patch (33.82 KB, patch)
2016-08-11 22:29 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
WIP Patch (33.82 KB, patch)
2016-08-11 22:31 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (39.48 KB, patch)
2016-08-12 08:45 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (39.50 KB, patch)
2016-08-12 10:56 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (41.02 KB, patch)
2016-08-12 20:23 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 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
Comment 1 Chris Dumez 2016-08-11 22:07:22 PDT
Created attachment 285894 [details]
WIP Patch
Comment 2 Chris Dumez 2016-08-11 22:29:41 PDT
Created attachment 285896 [details]
WIP Patch
Comment 3 Chris Dumez 2016-08-11 22:31:26 PDT
Created attachment 285897 [details]
WIP Patch
Comment 4 Chris Dumez 2016-08-12 08:45:29 PDT
Created attachment 285913 [details]
Patch
Comment 5 Chris Dumez 2016-08-12 08:46:00 PDT
This passes all the tests. I'll evaluate performance on Speedometer and Dromaeo today.
Comment 6 Chris Dumez 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.
Comment 7 Chris Dumez 2016-08-12 10:56:40 PDT
Created attachment 285920 [details]
Patch
Comment 8 Chris Dumez 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.
Comment 9 Chris Dumez 2016-08-12 15:55:59 PDT
Relevant spec discussion:
https://github.com/whatwg/dom/issues/143
Comment 10 Ryosuke Niwa 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?
Comment 11 Chris Dumez 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?
Comment 12 Chris Dumez 2016-08-12 20:23:22 PDT
Created attachment 286000 [details]
Patch
Comment 13 WebKit Commit Bot 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>
Comment 14 WebKit Commit Bot 2016-08-12 21:22:08 PDT
All reviewed patches have been landed.  Closing bug.
Comment 15 Chris Dumez 2016-08-13 15:42:46 PDT
I monitored the perf bots and did not see any obvious regression from this change.