Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(205)

Issue 2406423004: Change deleteCell behavior when there are no cells (Closed)

Created:
4 years, 2 months ago by rwlbuis
Modified:
4 years, 2 months ago
Reviewers:
foolip
CC:
chromium-reviews, blink-reviews, tfarina, blink-reviews-w3ctests_chromium.org, blink-reviews-html_chromium.org, dglazkov+blink
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Change deleteCell behavior when there are no cells Change deleteCell behavior when there are no cells, according to the spec [1] when the index is -1 and there are no cells we should not do anything. Behavior matches Firefox, Safari and Edge. BUG=651762 [1] https://html.spec.whatwg.org/multipage/tables.html#dom-tr-deletecell Committed: https://crrev.com/549c1d11032cc509f1438de8f1e979f9793d53c8 Cr-Commit-Position: refs/heads/master@{#424869}

Patch Set 1 #

Total comments: 2

Patch Set 2 : Reorder and add comments #

Total comments: 2

Patch Set 3 : Fix comment for step 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+14 lines, -14 lines) Patch
D third_party/WebKit/LayoutTests/imported/wpt/html/semantics/tabular-data/the-tr-element/deleteCell-expected.txt View 1 chunk +0 lines, -8 lines 0 comments Download
M third_party/WebKit/Source/core/html/HTMLTableRowElement.cpp View 1 2 1 chunk +14 lines, -6 lines 0 comments Download

Messages

Total messages: 15 (6 generated)
rwlbuis
PTAL.
4 years, 2 months ago (2016-10-11 19:58:07 UTC) #4
foolip
lgtm % optional nit https://codereview.chromium.org/2406423004/diff/1/third_party/WebKit/Source/core/html/HTMLTableRowElement.cpp File third_party/WebKit/Source/core/html/HTMLTableRowElement.cpp (right): https://codereview.chromium.org/2406423004/diff/1/third_party/WebKit/Source/core/html/HTMLTableRowElement.cpp#newcode123 third_party/WebKit/Source/core/html/HTMLTableRowElement.cpp:123: IndexSizeError, "The value provided (" ...
4 years, 2 months ago (2016-10-12 13:07:11 UTC) #5
rwlbuis
https://codereview.chromium.org/2406423004/diff/1/third_party/WebKit/Source/core/html/HTMLTableRowElement.cpp File third_party/WebKit/Source/core/html/HTMLTableRowElement.cpp (right): https://codereview.chromium.org/2406423004/diff/1/third_party/WebKit/Source/core/html/HTMLTableRowElement.cpp#newcode123 third_party/WebKit/Source/core/html/HTMLTableRowElement.cpp:123: IndexSizeError, "The value provided (" + String::number(index) + On ...
4 years, 2 months ago (2016-10-12 13:48:38 UTC) #6
foolip
https://codereview.chromium.org/2406423004/diff/20001/third_party/WebKit/Source/core/html/HTMLTableRowElement.cpp File third_party/WebKit/Source/core/html/HTMLTableRowElement.cpp (right): https://codereview.chromium.org/2406423004/diff/20001/third_party/WebKit/Source/core/html/HTMLTableRowElement.cpp#newcode118 third_party/WebKit/Source/core/html/HTMLTableRowElement.cpp:118: // elements in the cells collection, then throw "IndexSizeError". ...
4 years, 2 months ago (2016-10-12 14:19:01 UTC) #7
rwlbuis
https://codereview.chromium.org/2406423004/diff/20001/third_party/WebKit/Source/core/html/HTMLTableRowElement.cpp File third_party/WebKit/Source/core/html/HTMLTableRowElement.cpp (right): https://codereview.chromium.org/2406423004/diff/20001/third_party/WebKit/Source/core/html/HTMLTableRowElement.cpp#newcode118 third_party/WebKit/Source/core/html/HTMLTableRowElement.cpp:118: // elements in the cells collection, then throw "IndexSizeError". ...
4 years, 2 months ago (2016-10-12 15:29:39 UTC) #8
foolip
lgtm
4 years, 2 months ago (2016-10-12 15:38:15 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2406423004/40001
4 years, 2 months ago (2016-10-12 20:13:55 UTC) #11
commit-bot: I haz the power
Committed patchset #3 (id:40001)
4 years, 2 months ago (2016-10-12 21:38:09 UTC) #13
commit-bot: I haz the power
4 years, 2 months ago (2016-10-12 21:39:35 UTC) #15
Message was sent while issue was closed.
Patchset 3 (id:??) landed as
https://crrev.com/549c1d11032cc509f1438de8f1e979f9793d53c8
Cr-Commit-Position: refs/heads/master@{#424869}

Powered by Google App Engine
This is Rietveld 408576698