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 500032 d7a0a0e0173e23074808c47db4c9178e8f50708a
parent 500031 f7d1fd195963da3c729622e97599debeb425c335
child 500033 96a6e1ceb6977a32bc6a29f46f5a47622bfc4d50
push id1864
push userffxbld-merge
push dateMon, 03 Dec 2018 15:51:40 +0000
treeherdermozilla-release@f040763d99ad [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbzbarsky
bugs1499571
milestone64.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
-