Auto merge of #12594 - GuillaumeGomez:remove_row, r=Ms2ger

Add DeleteRow method

<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12594)
<!-- Reviewable:end -->

8 files changed, 106 insertions, 37 deletions

diff --git a/components/script/dom/htmlcollection.rs b/components/script/dom/htmlcollection.rs
index 798d0731b2c..95134556bc0 100644
--- a/components/script/dom/htmlcollection.rs
+++ b/components/script/dom/htmlcollection.rs
@@ -248,7 +248,7 @@ impl<'a> Iterator for HTMLCollectionElementsIter<'a> {
                       .filter_map(Root::downcast)
                       .filter(|element| filter.filter(&element, root))
                       .next()
-   }
+    }
 }
 
 impl HTMLCollectionMethods for HTMLCollection {
diff --git a/components/script/dom/htmltableelement.rs b/components/script/dom/htmltableelement.rs
index 4cff33b78a1..1d189f4a21f 100644
--- a/components/script/dom/htmltableelement.rs
+++ b/components/script/dom/htmltableelement.rs
@@ -34,6 +34,20 @@ pub struct HTMLTableElement {
     tbodies: MutNullableHeap<JS<HTMLCollection>>,
 }
 
+#[allow(unrooted_must_root)]
+#[derive(JSTraceable, HeapSizeOf)]
+struct TableRowFilter {
+    sections: Vec<JS<Node>>,
+}
+
+impl CollectionFilter for TableRowFilter {
+    fn filter(&self, elem: &Element, root: &Node) -> bool {
+        elem.is::<HTMLTableRowElement>() &&
+            (root.is_parent_of(elem.upcast())
+                || self.sections.iter().any(|ref section| section.is_parent_of(elem.upcast())))
+    }
+}
+
 impl HTMLTableElement {
     fn new_inherited(localName: Atom, prefix: Option<DOMString>, document: &Document)
                      -> HTMLTableElement {
@@ -120,32 +134,22 @@ impl HTMLTableElement {
             thead.upcast::<Node>().remove_self();
         }
     }
-}
-
-impl HTMLTableElementMethods for HTMLTableElement {
-    // https://html.spec.whatwg.org/multipage/#dom-table-rows
-    fn Rows(&self) -> Root<HTMLCollection> {
-        #[allow(unrooted_must_root)]
-        #[derive(JSTraceable, HeapSizeOf)]
-        struct TableRowFilter {
-            sections: Vec<JS<Node>>
-        }
-
-        impl CollectionFilter for TableRowFilter {
-            fn filter(&self, elem: &Element, root: &Node) -> bool {
-                elem.is::<HTMLTableRowElement>() &&
-                    (root.is_parent_of(elem.upcast())
-                        || self.sections.iter().any(|ref section| section.is_parent_of(elem.upcast())))
-            }
-        }
 
-        let filter = TableRowFilter {
+    fn get_rows(&self) -> TableRowFilter {
+        TableRowFilter {
             sections: self.upcast::<Node>()
                           .children()
                           .filter_map(|ref node|
                                 node.downcast::<HTMLTableSectionElement>().map(|_| JS::from_ref(&**node)))
                           .collect()
-        };
+        }
+    }
+}
+
+impl HTMLTableElementMethods for HTMLTableElement {
+    // https://html.spec.whatwg.org/multipage/#dom-table-rows
+    fn Rows(&self) -> Root<HTMLCollection> {
+        let filter = self.get_rows();
         HTMLCollection::new(window_from_node(self).r(), self.upcast(), box filter)
     }
 
@@ -338,6 +342,22 @@ impl HTMLTableElementMethods for HTMLTableElement {
         Ok(new_row)
     }
 
+    // https://html.spec.whatwg.org/multipage/#dom-table-deleterow
+    fn DeleteRow(&self, mut index: i32) -> Fallible<()> {
+        let rows = self.Rows();
+        // Step 1.
+        if index == -1 {
+            index = rows.Length() as i32 - 1;
+        }
+        // Step 2.
+        if index < 0 || index as u32 >= rows.Length() {
+            return Err(Error::IndexSize);
+        }
+        // Step 3.
+        Root::upcast::<Node>(rows.Item(index as u32).unwrap()).remove_self();
+        Ok(())
+    }
+
     // https://html.spec.whatwg.org/multipage/#dom-table-bgcolor
     make_getter!(BgColor, "bgcolor");
 
diff --git a/components/script/dom/webidls/HTMLTableElement.webidl b/components/script/dom/webidls/HTMLTableElement.webidl
index 596f5abd188..f0d8e19d0eb 100644
--- a/components/script/dom/webidls/HTMLTableElement.webidl
+++ b/components/script/dom/webidls/HTMLTableElement.webidl
@@ -19,7 +19,7 @@ interface HTMLTableElement : HTMLElement {
   HTMLTableSectionElement createTBody();
   readonly attribute HTMLCollection rows;
   [Throws] HTMLTableRowElement insertRow(optional long index = -1);
-  //void deleteRow(long index);
+  [Throws] void deleteRow(long index);
 
   // also has obsolete members
 };
diff --git a/tests/wpt/metadata/FileAPI/file/File-constructor.html.ini b/tests/wpt/metadata/FileAPI/file/File-constructor.html.ini
index 79167726fa2..1134394834f 100644
--- a/tests/wpt/metadata/FileAPI/file/File-constructor.html.ini
+++ b/tests/wpt/metadata/FileAPI/file/File-constructor.html.ini
@@ -11,3 +11,4 @@
   [Various fileBits]
     expected: FAIL
     bug: https://github.com/servo/servo/issues/10911
+
diff --git a/tests/wpt/metadata/MANIFEST.json b/tests/wpt/metadata/MANIFEST.json
index 765b201622d..5606390b636 100644
--- a/tests/wpt/metadata/MANIFEST.json
+++ b/tests/wpt/metadata/MANIFEST.json
@@ -15226,6 +15226,10 @@
         "url": "/dom/nodes/prepend-on-Document.html"
       },
       {
+        "path": "dom/nodes/remove-row.html",
+        "url": "/dom/nodes/remove-row.html"
+      },
+      {
         "path": "dom/nodes/remove-unscopable.html",
         "url": "/dom/nodes/remove-unscopable.html"
       },
@@ -37201,7 +37205,9 @@
     ]
   },
   "local_changes": {
-    "deleted": [],
+    "deleted": [
+      "dom/nodes/remove-row.html"
+    ],
     "deleted_reftests": {},
     "items": {
       "testharness": {
@@ -37216,6 +37222,12 @@
             "path": "html/semantics/scripting-1/the-script-element/script-charset-03.html",
             "url": "/html/semantics/scripting-1/the-script-element/script-charset-03.html"
           }
+        ],
+        "html/semantics/tabular-data/the-table-element/remove-row.html": [
+          {
+            "path": "html/semantics/tabular-data/the-table-element/remove-row.html",
+            "url": "/html/semantics/tabular-data/the-table-element/remove-row.html"
+          }
         ]
       }
     },
diff --git a/tests/wpt/metadata/dom/nodes/getElementsByClassName-21.htm.ini b/tests/wpt/metadata/dom/nodes/getElementsByClassName-21.htm.ini
deleted file mode 100644
index 143ad1185c4..00000000000
--- a/tests/wpt/metadata/dom/nodes/getElementsByClassName-21.htm.ini
+++ /dev/null
@@ -1,5 +0,0 @@
-[getElementsByClassName-21.htm]
-  type: testharness
-  [delete element from collection]
-    expected: FAIL
-
diff --git a/tests/wpt/metadata/html/dom/interfaces.html.ini b/tests/wpt/metadata/html/dom/interfaces.html.ini
index 89a8654cd7e..7696abf478f 100644
--- a/tests/wpt/metadata/html/dom/interfaces.html.ini
+++ b/tests/wpt/metadata/html/dom/interfaces.html.ini
@@ -3237,9 +3237,6 @@
   [HTMLAreaElement interface: document.createElement("area") must inherit property "noHref" with the proper type (10)]
     expected: FAIL
 
-  [HTMLTableElement interface: operation deleteRow(long)]
-    expected: FAIL
-
   [HTMLTableElement interface: attribute sortable]
     expected: FAIL
 
@@ -3267,12 +3264,6 @@
   [HTMLTableElement interface: attribute cellSpacing]
     expected: FAIL
 
-  [HTMLTableElement interface: document.createElement("table") must inherit property "deleteRow" with the proper type (13)]
-    expected: FAIL
-
-  [HTMLTableElement interface: calling deleteRow(long) on document.createElement("table") with too few arguments must throw TypeError]
-    expected: FAIL
-
   [HTMLTableElement interface: document.createElement("table") must inherit property "sortable" with the proper type (14)]
     expected: FAIL
 
diff --git a/tests/wpt/web-platform-tests/html/semantics/tabular-data/the-table-element/remove-row.html b/tests/wpt/web-platform-tests/html/semantics/tabular-data/the-table-element/remove-row.html
new file mode 100644
index 00000000000..b0e529f91ec
--- /dev/null
+++ b/tests/wpt/web-platform-tests/html/semantics/tabular-data/the-table-element/remove-row.html
@@ -0,0 +1,50 @@
+<!doctype html>
+<meta charset="utf-8">
+<title>Delete Row tests</title>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+
+<table id="element">
+  <thead>
+    <th>First column</th>
+    <th>Second column</th>
+  </thead>
+  <tbody>
+    <tr>
+      <td>1.1</td>
+      <td>1.2</td>
+    </tr>
+    <tr>
+      <td>2.1</td>
+      <td>2.2</td>
+    </tr>
+  </tbody>
+</table>
+
+<script>
+var el = document.getElementById('element');
+
+test(function() {
+  assert_throws("IndexSizeError", function() {
+    el.deleteRow(-2)
+  })
+}, 'deleteRow function invalid argument');
+test(function() {
+  assert_throws("IndexSizeError", function() {
+    el.deleteRow(el.rows.length)
+  })
+}, 'deleteRow function invalid argument bis');
+
+test(function() {
+  var old_length = el.rows.length;
+  el.insertRow(-1);
+  el.deleteRow(-1);
+  assert_equals(old_length, el.rows.length);
+}, "check normal deleteRow");
+test(function() {
+  while (el.rows.length > 1) {
+    el.deleteRow(-1);
+  }
+  assert_equals(1, el.rows.length);
+}, "check normal deleteRow bis");
+</script>