Bug 160682 – getElementsByTagName() should take a qualifiedName in parameter

• Component: DOM
• Status: RESOLVED FIXED

[Chris Dumez]

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

[Chris Dumez]

Created attachment 285894 [details]
WIP Patch

[Chris Dumez]

Created attachment 285896 [details]
WIP Patch

[Chris Dumez]

Created attachment 285897 [details]
WIP Patch

[Chris Dumez]

Created attachment 285913 [details]
Patch

[Chris Dumez]

This passes all the tests. I'll evaluate performance on Speedometer and Dromaeo today.

[Chris Dumez]

(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]

Created attachment 285920 [details]
Patch

[Chris Dumez]

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]

Relevant spec discussion:
https://github.com/whatwg/dom/issues/143

[Ryosuke Niwa]

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 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]

Created attachment 286000 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 286000 [details]
Patch

Clearing flags on attachment: 286000

Committed r204441: <http://trac.webkit.org/changeset/204441>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Chris Dumez]

I monitored the perf bots and did not see any obvious regression from this change.