Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make table.deleteRow(-1) a no-op when there are no rows #1924

Merged
merged 1 commit into from
Oct 20, 2016

Conversation

foolip
Copy link
Member

@foolip foolip commented Oct 18, 2016

This copies the wording of tbody.deleteRow, with "its parent" instead
of "this element". The previous wording would throw for this case.

Tests: web-platform-tests/wpt#4001

@foolip
Copy link
Member Author

foolip commented Oct 18, 2016

Please do not merge before #1890 as the commit message claims to copy wording that that commit changes.

This copies the wording of tbody.deleteRow, with "its parent" instead
of "this element". The previous wording would throw for this case.

Tests: web-platform-tests/wpt#4001
@foolip
Copy link
Member Author

foolip commented Oct 18, 2016

@rwlbuis, care to review this and the test?

@foolip
Copy link
Member Author

foolip commented Oct 18, 2016

@domenic found #1519 where this was previously discussed.

@domenic domenic added normative change compat Standard is not web compatible or proprietary feature needs standardizing labels Oct 18, 2016
@cdumez
Copy link

cdumez commented Oct 19, 2016

lgtm. It is best to be consistent.
I'll fix WebKit if this lands.

@foolip
Copy link
Member Author

foolip commented Oct 20, 2016

Thanks all, now merging this and web-platform-tests/wpt#4001

foolip added a commit to web-platform-tests/wpt that referenced this pull request Oct 20, 2016
@foolip foolip merged commit ad778cb into master Oct 20, 2016
@foolip foolip deleted the deleteRow-minus-one branch October 20, 2016 11:18
@foolip
Copy link
Member Author

foolip commented Oct 20, 2016

Chromium change is https://codereview.chromium.org/2427963004/ (no bug, if one is created it'll be linked from there)

@cdumez, can you link to WebKit change when it lands?

Filed Edge issue at https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/9440419/

@cdumez
Copy link

cdumez commented Oct 20, 2016

kisg pushed a commit to paul99/webkit-mips that referenced this pull request Oct 20, 2016
https://bugs.webkit.org/show_bug.cgi?id=163746

Reviewed by Alex Christensen.

LayoutTests/imported/w3c:

Import test coverage from:
- web-platform-tests/wpt#4001

* web-platform-tests/html/semantics/tabular-data/the-table-element/remove-row-expected.txt:
* web-platform-tests/html/semantics/tabular-data/the-table-element/remove-row.html:
* web-platform-tests/html/semantics/tabular-data/the-tbody-element/deleteRow-expected.txt:
* web-platform-tests/html/semantics/tabular-data/the-tbody-element/deleteRow.html:
* web-platform-tests/html/semantics/tabular-data/the-tr-element/deleteCell-expected.txt:
* web-platform-tests/html/semantics/tabular-data/the-tr-element/deleteCell.html:

Source/WebCore:

Make table.deleteRow(-1) a no-op when there are no rows, instead of throwing:
- whatwg/html#1924

This is more consistent with the behavior of tbody.deleteRow(-1) and
tr.deleteCell(-1). This is also consistent with Gecko. Blink is doing the
same change via:
- https://codereview.chromium.org/2427963004/

No new tests, updated existing tests.

* html/HTMLTableElement.cpp:
(WebCore::HTMLTableElement::deleteRow):


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@207640 268f45cc-cd09-0410-ab3c-d52691b4dbfc
ryanhaddad pushed a commit to WebKit/WebKit that referenced this pull request Dec 22, 2020
https://bugs.webkit.org/show_bug.cgi?id=163746

Reviewed by Alex Christensen.

LayoutTests/imported/w3c:

Import test coverage from:
- web-platform-tests/wpt#4001

* web-platform-tests/html/semantics/tabular-data/the-table-element/remove-row-expected.txt:
* web-platform-tests/html/semantics/tabular-data/the-table-element/remove-row.html:
* web-platform-tests/html/semantics/tabular-data/the-tbody-element/deleteRow-expected.txt:
* web-platform-tests/html/semantics/tabular-data/the-tbody-element/deleteRow.html:
* web-platform-tests/html/semantics/tabular-data/the-tr-element/deleteCell-expected.txt:
* web-platform-tests/html/semantics/tabular-data/the-tr-element/deleteCell.html:

Source/WebCore:

Make table.deleteRow(-1) a no-op when there are no rows, instead of throwing:
- whatwg/html#1924

This is more consistent with the behavior of tbody.deleteRow(-1) and
tr.deleteCell(-1). This is also consistent with Gecko. Blink is doing the
same change via:
- https://codereview.chromium.org/2427963004/

No new tests, updated existing tests.

* html/HTMLTableElement.cpp:
(WebCore::HTMLTableElement::deleteRow):


Canonical link: https://commits.webkit.org/181525@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@207640 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compat Standard is not web compatible or proprietary feature needs standardizing normative change
Development

Successfully merging this pull request may close these issues.

None yet

4 participants