From ddbec46e1fe6716e2cba5e073f62014c22539589 Mon Sep 17 00:00:00 2001 From: shanehandley <1322294+shanehandley@users.noreply.github.com> Date: Mon, 8 Apr 2024 01:09:22 +1000 Subject: [PATCH] fix: Handle table.deleteRow with no rows (#32009) * fix: Handle table.deleteRow with no rows * Respond to review, update legacy layout expectations --- components/script/dom/htmltableelement.rs | 29 ++++++++++++++----- .../the-table-element/remove-row.html.ini | 5 ---- .../the-table-element/remove-row.html.ini | 3 -- 3 files changed, 21 insertions(+), 16 deletions(-) delete mode 100644 tests/wpt/meta-legacy-layout/html/semantics/tabular-data/the-table-element/remove-row.html.ini delete mode 100644 tests/wpt/meta/html/semantics/tabular-data/the-table-element/remove-row.html.ini diff --git a/components/script/dom/htmltableelement.rs b/components/script/dom/htmltableelement.rs index bd2294611ed..9194578a96c 100644 --- a/components/script/dom/htmltableelement.rs +++ b/components/script/dom/htmltableelement.rs @@ -395,19 +395,32 @@ 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() { + let num_rows = rows.Length() as i32; + + // Step 1: If index is less than −1 or greater than or equal to the number of elements + // in the rows collection, then throw an "IndexSizeError". + if !(-1..num_rows).contains(&index) { return Err(Error::IndexSize); } - // Step 3. + + let num_rows = rows.Length() as i32; + + // Step 2: If index is −1, then remove the last element in the rows collection from its + // parent, or do nothing if the rows collection is empty. + if index == -1 { + index = num_rows - 1; + } + + if num_rows == 0 { + return Ok(()); + } + + // Step 3: Otherwise, remove the indexth element in the rows collection from its parent. DomRoot::upcast::(rows.Item(index as u32).unwrap()).remove_self(); + Ok(()) } diff --git a/tests/wpt/meta-legacy-layout/html/semantics/tabular-data/the-table-element/remove-row.html.ini b/tests/wpt/meta-legacy-layout/html/semantics/tabular-data/the-table-element/remove-row.html.ini deleted file mode 100644 index 4939cb2245e..00000000000 --- a/tests/wpt/meta-legacy-layout/html/semantics/tabular-data/the-table-element/remove-row.html.ini +++ /dev/null @@ -1,5 +0,0 @@ -[remove-row.html] - type: testharness - [deleteRow(-1) with no rows] - expected: FAIL - diff --git a/tests/wpt/meta/html/semantics/tabular-data/the-table-element/remove-row.html.ini b/tests/wpt/meta/html/semantics/tabular-data/the-table-element/remove-row.html.ini deleted file mode 100644 index 0359c82cb6b..00000000000 --- a/tests/wpt/meta/html/semantics/tabular-data/the-table-element/remove-row.html.ini +++ /dev/null @@ -1,3 +0,0 @@ -[remove-row.html] - [deleteRow(-1) with no rows] - expected: FAIL