getElementsByTagName() should take a qualifiedName in parameter

third_party/WebKit/Source/core/html/HTMLTagCollection.h

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2001 Dirk Mueller (mueller@kde.org)
  * Copyright (C) 2004, 2005, 2006, 2007, 2008 Apple Inc. All rights reserved.
  * Copyright (C) 2008 Nokia Corporation and/or its subsidiary(-ies)
  * Copyright (C) 2014 Samsung Electronics. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ skipping 18 matching lines @@
 #include "core/dom/TagCollection.h"
 
 namespace blink {
 
 // Collection that limits to a particular tag and whose rootNode is in an
 // HTMLDocument.
 class HTMLTagCollection final : public TagCollection {
  public:
   static HTMLTagCollection* Create(ContainerNode& root_node,
                                    CollectionType type,
-                                   const AtomicString& local_name) {
+                                   const AtomicString& qualified_name) {
     DCHECK_EQ(type, kHTMLTagCollectionType);
-    return new HTMLTagCollection(root_node, local_name);
+    return new HTMLTagCollection(root_node, qualified_name);
   }
 
   bool ElementMatches(const Element&) const;
 
  private:
-  HTMLTagCollection(ContainerNode& root_node, const AtomicString& local_name);
+  HTMLTagCollection(ContainerNode& root_node,
+                    const AtomicString& qualified_name);
 
-  AtomicString lowered_local_name_;
+  AtomicString lowered_qualified_name_;
 };
 
 DEFINE_TYPE_CASTS(HTMLTagCollection,
                   LiveNodeListBase,
                   collection,
                   collection->GetType() == kHTMLTagCollectionType,
                   collection.GetType() == kHTMLTagCollectionType);
 
 inline bool HTMLTagCollection::ElementMatches(
     const Element& test_element) const {
-  // Implements
-  // https://dom.spec.whatwg.org/#concept-getelementsbytagname
-  if (local_name_ != g_star_atom) {
-    const AtomicString& local_name =
-        test_element.IsHTMLElement() ? lowered_local_name_ : local_name_;
-    if (local_name != test_element.localName())
-      return false;
-  }
-  DCHECK_EQ(namespace_uri_, g_star_atom);
-  return true;
+  if (test_element.IsHTMLElement())

[tkent, 2017/05/09 00:28:01]

The specification says "if root’s node document is an HTML document".  Checking
IsHTMLElement() looks wrong.

https://dom.spec.whatwg.org/#concept-getelementsbytagname

[Shanmuga Pandi, 2017/05/10 06:35:45]

I have tried to replace the if condition with below options, but many
LayoutTests are failing on external/wpt/dom/*
1. if (test_element.GetDocument().IsHTMLDocument() ||
test_element.GetDocument().IsXHTMLDocument())
2. if (test_element.IsHTMLElement() &&
(test_element.GetDocument().IsHTMLDocument() ||
test_element.GetDocument().IsXHTMLDocument()))

Also
in ContainerNode::getElementsByTagName, we are already checking isHTMLDocument
or not like below.
  if (GetDocument().IsHTMLDocument()) {
    return EnsureCachedCollection<HTMLTagCollection>(kHTMLTagCollectionType,
                                                     qualified_name);
  }

[fs, 2017/05/10 08:34:15]

FWIW, I think this "IsHTMLElement" check corresponds to the "Whose namespace is
the HTML namespace..." part (and also the "is not the HTML namespace" part by
association.)

[tkent, 2017/05/11 04:14:02]

You're right.  "if root’s node document is an HTML document" is implemented in
ContainerNode::getElementsByTagName().
Sorry for the wrong comment!

+    return lowered_qualified_name_ == test_element.TagQName().ToString();
+  return qualified_name_ == test_element.TagQName().ToString();

[fs, 2017/05/08 14:20:36]

This creates a temporary string for each comparison. Compare the two components
(prefix, localName) of QName instead. (This also means that the storage for the
filter needs to be different.)

[Shanmuga Pandi, 2017/05/10 06:35:45]

I think initially webkit tried this approach and later due to regression they
comparing TagQName string instead.
Related patch:
https://trac.webkit.org/changeset/208674/webkit
Please correct me if I misunderstood any.

[Shanmuga Pandi, 2017/05/10 07:05:28]

Can we use Document::ParseQualifiedName() and store prefix and localName and
compare the two elements as your suggestion ?

[fs, 2017/05/10 08:34:15]

Fair enough. We could add a "CompareQualifiedName(const QualifiedName& qname,
const AtomicString& other_qname)" method that did a "component-wise" match. But
that is better as a separate CL in that case.

[Shanmuga Pandi, 2017/05/10 09:03:12]

Ok. I will keep the current comparision as it is and will make separate CL as
per your suggestion.

 }
 
 }  // namespace blink
 
 #endif  // HTMLTagCollection_h