Make HTMLAllCollection named property getter behave more according to spec

Source/core/html/HTMLAllCollection.cpp

 /*
  * Copyright (C) 2009, 2011, 2012 Apple Inc. All rights reserved.
+ * Copyright (C) 2014 Samsung Electronics. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
  *    documentation and/or other materials provided with the distribution.
  *
  * THIS SOFTWARE IS PROVIDED BY APPLE INC. ``AS IS'' AND ANY
  * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
  * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
  * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL APPLE INC. OR
  * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
  * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
  * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
  * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
  * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
 #include "config.h"
 #include "core/html/HTMLAllCollection.h"
 
+#include "HTMLNames.h"
 #include "core/dom/Element.h"
 #include "core/dom/NamedNodesCollection.h"
+#include "core/html/HTMLElement.h"
+#include "wtf/HashSet.h"
 
 namespace WebCore {
 
+using namespace HTMLNames;
+
 PassRefPtr<HTMLAllCollection> HTMLAllCollection::create(ContainerNode* node, CollectionType type)
 {
     return adoptRef(new HTMLAllCollection(node, type));
 }
 
 HTMLAllCollection::HTMLAllCollection(ContainerNode* node, CollectionType type)
     : HTMLCollection(node, type, DoesNotOverrideItemAfter)
 {
     ScriptWrappable::init(this);
 }
 
+bool HTMLAllCollection::elementNameShouldBeVisible(const HTMLElement& element)
+{
+    // The document.all collection returns only certain types of elements by name,
+    // although it returns any type of element by id.
+    return element.hasLocalName(aTag)
+        || element.hasLocalName(appletTag)
+        || element.hasLocalName(areaTag)
+        || element.hasLocalName(embedTag)
+        || element.hasLocalName(formTag)
+        || element.hasLocalName(frameTag)
+        || element.hasLocalName(framesetTag)
+        || element.hasLocalName(iframeTag)
+        || element.hasLocalName(imgTag)
+        || element.hasLocalName(objectTag)
+        || element.hasLocalName(inputTag) // FIXME: Not according to spec.

[arv (Not doing code reviews), 2014/02/11 20:48:42]

If IE11 behaves this way we should get the spec changed.

[Inactive, 2014/02/11 20:50:37]

Yes, based on my testing IE11 return the names of input and select elements.

[Inactive, 2014/02/11 20:55:10]

http://jsfiddle.net/jAx2t/3/

[Inactive, 2014/02/11 21:02:39]

I filed a bug against the spec:
https://www.w3.org/Bugs/Public/show_bug.cgi?id=24622

[Inactive, 2014/02/12 01:32:37]

Would you like me to remove those 2 FIXME comments as we behave the same way as
other browsers and probably don't want to "fix" this (but fix the spec instead).

+        || element.hasLocalName(selectTag); // FIXME: Not according to spec.
+}
+
+void HTMLAllCollection::supportedPropertyNames(Vector<String>& names)
+{
+    // As per the specification (http://www.whatwg.org/specs/web-apps/current-work/multipage/common-dom-interfaces.html#htmlallcollection-0):
+    // The supported property names consist of the non-empty values of all the id attributes of all the elements
+    // represented by the collection, and the non-empty values of all the name attributes of all the a, applet,
+    // area, embed, form, frame, frameset, iframe, img, and object elements represented by the collection, in
+    // tree order, ignoring later duplicates, with the id of an element preceding its name if it contributes
+    // both, they differ from each other, and neither is the duplicate of an earlier entry.
+    HashSet<AtomicString> existingNames;
+    ContainerNode& root = rootNode();
+    for (Element* element = traverseToFirstElement(root); element; element = traverseNextElement(*element, root)) {

[haraken, 2014/02/12 01:23:01]

We're duplicating this code in several files. It seems that the difference
between the duplicated code is just a condition in line 81 and 89. Probably we
can define a method for the condition instead of duplicating the entire loop.

You can do it in a follow-up if you agree with the idea.

[Inactive, 2014/02/12 01:31:05]

If it was just about the condition, I would have simply added it to the
HTMLCollection's implementation (this was my first attempt if you remember).
However, the order of id / name is also reversed. I am trying to get the DOM
spec fixed so that the id / name order is consistent between HTMLCollection and
its subclasses:
https://www.w3.org/Bugs/Public/show_bug.cgi?id=24624

If this goes through, I will refactor the code to avoid a lot of the
duplication. If you insist, I can also try and get HTMLCollection's
supportedPropertyNames() to work for HTMLAllCollection in this CL already.

[Inactive, 2014/02/12 16:28:41]

I am updating the id/name order in HTMLCollection with
https://codereview.chromium.org/157813013/ (the DOM spec was just updated). Once
that other CL lands, I will rebase this patch and get rid of the code
duplication.

+        const AtomicString& idAttribute = element->getIdAttribute();
+        if (!idAttribute.isEmpty()) {
+            HashSet<AtomicString>::AddResult addResult = existingNames.add(idAttribute);
+            if (addResult.isNewEntry)
+                names.append(idAttribute);
+        }
+        if (!element->isHTMLElement())
+            continue;
+        const AtomicString& nameAttribute = element->getNameAttribute();
+        if (!nameAttribute.isEmpty() && elementNameShouldBeVisible(toHTMLElement(*element))) {
+            HashSet<AtomicString>::AddResult addResult = existingNames.add(nameAttribute);
+            if (addResult.isNewEntry)
+                names.append(nameAttribute);
+        }
+    }
+}
+
 HTMLAllCollection::~HTMLAllCollection()
 {
 }
 
 Element* HTMLAllCollection::namedItemWithIndex(const AtomicString& name, unsigned index) const
 {
     updateNameCache();
 
     if (Vector<Element*>* cache = idCache(name)) {
         if (index < cache->size())
             return cache->at(index);
         index -= cache->size();
     }
 
     if (Vector<Element*>* cache = nameCache(name)) {
         if (index < cache->size())
             return cache->at(index);
     }
 
     return 0;
 }
 
-void HTMLAllCollection::anonymousNamedGetter(const AtomicString& name, bool& returnValue0Enabled, RefPtr<NodeList>& returnValue0, bool& returnValue1Enabled, RefPtr<Element>& returnValue1)
+void HTMLAllCollection::namedGetter(const AtomicString& name, bool& returnValue0Enabled, RefPtr<NodeList>& returnValue0, bool& returnValue1Enabled, RefPtr<Element>& returnValue1)
 {
     Vector<RefPtr<Element> > namedItems;
     this->namedItems(name, namedItems);
 
     if (!namedItems.size())
         return;
 
     if (namedItems.size() == 1) {
         returnValue1Enabled = true;
         returnValue1 = namedItems.at(0);
         return;
     }
 
     // FIXME: HTML5 specification says this should be a HTMLCollection.
     // http://www.whatwg.org/specs/web-apps/current-work/multipage/common-dom-interfaces.html#htmlallcollection
     returnValue0Enabled = true;
     returnValue0 = NamedNodesCollection::create(namedItems);
 }
 
 } // namespace WebCore