Bug 1499571 - HTMLOptionsCollection::Remove shouldn't remove the first element of the collection if out of bounds. r=bzbarsky
authorEmilio Cobos Álvarez <emilio@crisal.io>
Wed, 17 Oct 2018 01:35:21 +0000
changeset 489950 d7a0a0e0173e23074808c47db4c9178e8f50708a
parent 489949 f7d1fd195963da3c729622e97599debeb425c335
child 489951 96a6e1ceb6977a32bc6a29f46f5a47622bfc4d50
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
reviewersbzbarsky
bugs1499571
milestone64.0a1
Bug 1499571 - HTMLOptionsCollection::Remove shouldn't remove the first element of the collection if out of bounds. r=bzbarsky Make it just forward to HTMLSelectElement::Remove like other browsers. Differential Revision: https://phabricator.services.mozilla.com/D8934
dom/html/HTMLOptionsCollection.cpp
dom/html/HTMLSelectElement.cpp
dom/html/test/test_bug405242.html
testing/web-platform/meta/html/semantics/forms/the-select-element/select-remove.html.ini
--- a/dom/html/HTMLOptionsCollection.cpp
+++ b/dom/html/HTMLOptionsCollection.cpp
@@ -280,18 +280,13 @@ HTMLOptionsCollection::Add(const HTMLOpt
 
 void
 HTMLOptionsCollection::Remove(int32_t aIndex, ErrorResult& aError)
 {
   if (!mSelect) {
     aError.Throw(NS_ERROR_UNEXPECTED);
     return;
   }
-
-  uint32_t len = mSelect->Length();
-  if (aIndex < 0 || (uint32_t)aIndex >= len)
-    aIndex = 0;
-
   mSelect->Remove(aIndex);
 }
 
 } // namespace dom
 } // namespace mozilla
--- a/dom/html/HTMLSelectElement.cpp
+++ b/dom/html/HTMLSelectElement.cpp
@@ -595,16 +595,20 @@ HTMLSelectElement::Add(nsGenericHTMLElem
   // insertBefore method on the parent of before.
   nsCOMPtr<nsINode> refNode = aBefore;
   parent->InsertBefore(aElement, refNode, aError);
 }
 
 void
 HTMLSelectElement::Remove(int32_t aIndex)
 {
+  if (aIndex < 0) {
+    return;
+  }
+
   nsCOMPtr<nsINode> option = Item(static_cast<uint32_t>(aIndex));
   if (!option) {
     return;
   }
 
   option->Remove();
 }
 
--- a/dom/html/test/test_bug405242.html
+++ b/dom/html/test/test_bug405242.html
@@ -22,14 +22,14 @@ sel.appendChild(new Option());
 sel.appendChild(new Option());
 sel.appendChild(new Option());
 opt = new Option();
 opt.value = 10;
 sel.appendChild(opt);
 sel.options.remove(0);
 sel.options.remove(1000);
 sel.options.remove(-1);
-is(sel.length, 1, "Unexpected option collection length");
-is(sel[0].value, "10", "Unexpected remained option");
+is(sel.length, 3, "Unexpected option collection length");
+is(sel[2].value, "10", "Unexpected remained option");
 </script>
 </pre>
 </body>
 </html>
deleted file mode 100644
--- a/testing/web-platform/meta/html/semantics/forms/the-select-element/select-remove.html.ini
+++ /dev/null
@@ -1,4 +0,0 @@
-[select-remove.html]
-  [select.options.remove(n) should work]
-    expected: FAIL
-