Bug 1389922 - Align HTMLAllCollection.item()/legacy caller with spec; r?smaug draft
authorAryeh Gregor <ayg@aryeh.name>
Sun, 13 Aug 2017 15:40:33 +0300
changeset 646566 f3a6d9769c233a0e126e7b31c0733089049acbe3
parent 645586 77bbeaac0cbd8c1c956ab3c0aa2eebf292f0b38b
child 646567 16a658986ad3d01cb27dae3b63446bd1f8291595
push id74161
push userbmo:ayg@aryeh.name
push dateTue, 15 Aug 2017 12:07:31 +0000
reviewerssmaug
bugs1389922
milestone57.0a1
Bug 1389922 - Align HTMLAllCollection.item()/legacy caller with spec; r?smaug This appears to match Chrome's behavior as well, except that Chrome sometimes returns undefined instead of null. MozReview-Commit-ID: D7RdU9a8XGs
dom/html/HTMLAllCollection.cpp
testing/web-platform/meta/html/infrastructure/common-dom-interfaces/collections/htmlallcollection.html.ini
--- a/dom/html/HTMLAllCollection.cpp
+++ b/dom/html/HTMLAllCollection.cpp
@@ -51,24 +51,62 @@ HTMLAllCollection::Length()
 
 Element*
 HTMLAllCollection::Item(uint32_t aIndex)
 {
   nsCOMPtr<nsIContent> content = Collection()->Item(aIndex);
   return content ? content->AsElement() : nullptr;
 }
 
+/**
+ * Helper for the next method.  If the string is equal to a simple serialization
+ * of an integer in the range [0, UINT32_MAX) (excluding the endpoint!), return
+ * it.  Otherwise return UINT32_MAX, which indicates it is not an array index
+ * property name.
+ */
+static uint32_t GetAsArrayIndexPropertyName(const nsAString& aStr) {
+  uint32_t val = 0;
+  for (uint32_t i = 0; i < aStr.Length(); i++) {
+    if (aStr[i] < '0' || aStr[i] > '9') {
+      return UINT32_MAX;
+    }
+    if (aStr[i] == '0' && (i != 0 || aStr.Length() != 1)) {
+      // Leading 0, invalid
+      return UINT32_MAX;
+    }
+    uint32_t origVal = val;
+    val = val * 10 + aStr[i] - '0';
+    if (val < origVal) {
+      // Overflow, invalid array index
+      return UINT32_MAX;
+    }
+  }
+  // UINT32_MAX is an invalid array index too, but we don't have to check for
+  // it specially, because it's also our error return value.
+  return val;
+}
+
 void
 HTMLAllCollection::Item(const Optional<nsAString>& aName,
                         Nullable<OwningHTMLCollectionOrElement>& aResult)
 {
   if (!aName.WasPassed()) {
     aResult.SetNull();
     return;
   }
+  uint32_t val = GetAsArrayIndexPropertyName(aName.Value());
+  if (val != UINT32_MAX) {
+    Element* elem = Item(val);
+    if (elem) {
+      aResult.SetValue().SetAsElement() = elem;
+    } else {
+      aResult.SetNull();
+    }
+    return;
+  }
   NamedItem(aName.Value(), aResult);
 }
 
 nsContentList*
 HTMLAllCollection::Collection()
 {
   if (!mCollection) {
     nsIDocument* document = mDocument;
--- a/testing/web-platform/meta/html/infrastructure/common-dom-interfaces/collections/htmlallcollection.html.ini
+++ b/testing/web-platform/meta/html/infrastructure/common-dom-interfaces/collections/htmlallcollection.html.ini
@@ -1,23 +1,5 @@
 [htmlallcollection.html]
   type: testharness
-  [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
-
-  [item method with "array index property name"]
-    expected: FAIL
-
-  [item method with "array index property name" as number]
-    expected: FAIL
-
-  [item method with invalid "array index property name"]
-    expected: FAIL
-
   [collections are new live HTMLCollection instances]
     expected: FAIL