Bug 1398354 part 2. Update document.all item() and legacycaller to new spec semantics. r=mccr8

dom/html/HTMLAllCollection.cpp

@@ -49,10 +49,37 @@ HTMLAllCollection::Length()
   return Collection()->Length(true);
 }
 
-nsIContent*
+Element*
 HTMLAllCollection::Item(uint32_t aIndex)
 {
-  return Collection()->Item(aIndex);
+  nsIContent* item = Collection()->Item(aIndex);
+  return item ? item->AsElement() : nullptr;
+}
+
+void
+HTMLAllCollection::Item(const Optional<nsAString>& aNameOrIndex,
+                        Nullable<OwningHTMLCollectionOrElement>& aResult)
+{
+  if (!aNameOrIndex.WasPassed()) {
+    aResult.SetNull();
+    return;
+  }
+
+  const nsAString& nameOrIndex = aNameOrIndex.Value();
+  uint32_t indexVal;
+  if (js::StringIsArrayIndex(nameOrIndex.BeginReading(),
+                             nameOrIndex.Length(),
+                             &indexVal)) {
+    Element* element = Item(indexVal);
+    if (element) {
+      aResult.SetValue().SetAsElement() = element;
+    } else {
+      aResult.SetNull();
+    }
+    return;
+  }
+
+  NamedItem(nameOrIndex, aResult);
 }
 
 nsContentList*
@@ -121,7 +148,7 @@ HTMLAllCollection::GetDocumentAllList(const nsAString& aID)
 void
 HTMLAllCollection::NamedGetter(const nsAString& aID,
                                bool& aFound,
-                               Nullable<OwningNodeOrHTMLCollection>& aResult)
+                               Nullable<OwningHTMLCollectionOrElement>& aResult)
 {
   if (aID.IsEmpty()) {
     aFound = false;
@@ -148,7 +175,7 @@ HTMLAllCollection::NamedGetter(const nsAString& aID,
   // There's only 0 or 1 items. Return the first one or null.
   if (nsIContent* node = docAllList->Item(0, true)) {
     aFound = true;
-    aResult.SetValue().SetAsNode() = node;
+    aResult.SetValue().SetAsElement() = node->AsElement();
     return;
   }
 

dom/html/HTMLAllCollection.h

@@ -16,14 +16,15 @@
 
 class nsContentList;
 class nsHTMLDocument;
-class nsIContent;
 class nsINode;
 
 namespace mozilla {
 namespace dom {
 
-class OwningNodeOrHTMLCollection;
+class Element;
+class OwningHTMLCollectionOrElement;
 template<typename> struct Nullable;
+template<typename> class Optional;
 
 class HTMLAllCollection final : public nsISupports
                               , public nsWrapperCache
@@ -40,32 +41,32 @@ public:
   nsINode* GetParentObject() const;
 
   uint32_t Length();
-  nsIContent* Item(uint32_t aIndex);
-  void Item(const nsAString& aName, Nullable<OwningNodeOrHTMLCollection>& aResult)
+  Element* IndexedGetter(uint32_t aIndex, bool& aFound)
   {
-    NamedItem(aName, aResult);
-  }
-  nsIContent* IndexedGetter(uint32_t aIndex, bool& aFound)
-  {
-    nsIContent* result = Item(aIndex);
+    Element* result = Item(aIndex);
     aFound = !!result;
     return result;
   }
 
   void NamedItem(const nsAString& aName,
-                 Nullable<OwningNodeOrHTMLCollection>& aResult)
+                 Nullable<OwningHTMLCollectionOrElement>& aResult)
   {
     bool found = false;
     NamedGetter(aName, found, aResult);
   }
   void NamedGetter(const nsAString& aName,
                    bool& aFound,
-                   Nullable<OwningNodeOrHTMLCollection>& aResult);
+                   Nullable<OwningHTMLCollectionOrElement>& aResult);
   void GetSupportedNames(nsTArray<nsString>& aNames);
-  void LegacyCall(JS::Handle<JS::Value>, const nsAString& aName,
-                  Nullable<OwningNodeOrHTMLCollection>& aResult)
+
+  void Item(const Optional<nsAString>& aNameOrIndex,
+            Nullable<OwningHTMLCollectionOrElement>& aResult);
+
+  void LegacyCall(JS::Handle<JS::Value>,
+                  const Optional<nsAString>& aNameOrIndex,
+                  Nullable<OwningHTMLCollectionOrElement>& aResult)
   {
-    NamedItem(aName, aResult);
+    Item(aNameOrIndex, aResult);
   }
 
 private:
@@ -76,6 +77,11 @@ private:
    */
   nsContentList* GetDocumentAllList(const nsAString& aID);
 
+  /**
+   * Helper for indexed getter and spec Item() method.
+   */
+  Element* Item(uint32_t aIndex);
+
   RefPtr<nsHTMLDocument> mDocument;
   RefPtr<nsContentList> mCollection;
   nsRefPtrHashtable<nsStringHashKey, nsContentList> mNamedMap;

dom/webidl/HTMLAllCollection.webidl

@@ -6,9 +6,8 @@
 [LegacyUnenumerableNamedProperties]
 interface HTMLAllCollection {
   readonly attribute unsigned long length;
-  getter Node? (unsigned long index);
-  Node? item(unsigned long index);
-  (Node or HTMLCollection)? item(DOMString name);
-  legacycaller (Node or HTMLCollection)? (DOMString name);
-  getter (Node or HTMLCollection)? namedItem(DOMString name);
+  getter Element (unsigned long index);
+  getter (HTMLCollection or Element)? namedItem(DOMString name);
+  (HTMLCollection or Element)? item(optional DOMString nameOrIndex);
+  legacycaller (HTMLCollection or Element)? (optional DOMString nameOrIndex);
 };

testing/web-platform/meta/html/dom/interfaces.https.html.ini

@@ -358,9 +358,6 @@
 
 
 [interfaces.https.html?include=HTML.*]
-  [HTMLAllCollection interface: operation item(DOMString)]
-    expected: FAIL
-
   [HTMLAllCollection must be primary interface of document.all]
     expected: FAIL
 

testing/web-platform/meta/html/infrastructure/common-dom-interfaces/collections/htmlallcollection.html.ini

@@ -1,31 +1,3 @@
 [htmlallcollection.html]
-  [legacy caller with "array index property name"]
-    expected: FAIL
-
-  [legacy caller with "array index property name" as number]
-    expected: FAIL
-
-  [legacy caller with invalid "array index property name"]
-    expected: FAIL
-
-  [legacy caller with no argument]
-    expected: FAIL
-
-  [item method with "array index property name"]
-    expected: FAIL
-
-  [item method with invalid "array index property name"]
-    expected: FAIL
-
-  [item method with no argument]
-    expected: FAIL
-
   [collections are new live HTMLCollection instances]
     expected: FAIL
-
-  [legacy caller with undefined]
-    expected: FAIL
-
-  [item method with undefined]
-    expected: FAIL
-