Change deleteCell behavior when there are no cells

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}

[rwlbuis, 2016-10-11 19:54:04]

Description was changed from

==========
deleteCell WIP

BUG=
==========

to

==========
deleteCell WIP

[1] https://html.spec.whatwg.org/multipage/tables.html#dom-tr-deletecell
==========

[rwlbuis, 2016-10-11 19:57:22]

Description was changed from

==========
deleteCell WIP

[1] https://html.spec.whatwg.org/multipage/tables.html#dom-tr-deletecell
==========

to

==========
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
==========

[rwlbuis, 2016-10-11 19:58:07]

rob.buis@samsung.com changed reviewers:
+ foolip@chromium.org

[rwlbuis, 2016-10-11 19:58:07]

PTAL.

[foolip, 2016-10-12 13:07:11]

lgtm % optional nit

https://codereview.chromium.org/2406423004/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/html/HTMLTableRowElement.cpp (right):

https://codereview.chromium.org/2406423004/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/HTMLTableRowElement.cpp:123: IndexSizeError,
"The value provided (" + String::number(index) +
Optional nit: could you reorder this so that it obviously matches the spec
exactly, step-by-step with spec comments?

[rwlbuis, 2016-10-12 13:48:38]

https://codereview.chromium.org/2406423004/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/html/HTMLTableRowElement.cpp (right):

https://codereview.chromium.org/2406423004/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/HTMLTableRowElement.cpp:123: IndexSizeError,
"The value provided (" + String::number(index) +
On 2016/10/12 13:07:11, foolip wrote:
> Optional nit: could you reorder this so that it obviously matches the spec
> exactly, step-by-step with spec comments?

Good idea, done.

[foolip, 2016-10-12 14:19:01]

https://codereview.chromium.org/2406423004/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/HTMLTableRowElement.cpp (right):

https://codereview.chromium.org/2406423004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLTableRowElement.cpp:118: // elements in
the cells collection, then throw "IndexSizeError".
Oh look, the spec has a bug here, doesn't it? I couldn't find you on GitHub, if
you have an account can you review https://github.com/whatwg/html/pull/1890?

This comment needs to be updated to match, too.

[rwlbuis, 2016-10-12 15:29:39]

https://codereview.chromium.org/2406423004/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/HTMLTableRowElement.cpp (right):

https://codereview.chromium.org/2406423004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLTableRowElement.cpp:118: // elements in
the cells collection, then throw "IndexSizeError".
On 2016/10/12 14:19:01, foolip wrote:
> Oh look, the spec has a bug here, doesn't it? I couldn't find you on GitHub,
if
> you have an account can you review https://github.com/whatwg/html/pull/1890

I added myself and okayed the change.

> This comment needs to be updated to match, too.

Done :)

[foolip, 2016-10-12 15:38:15]

lgtm

[rwlbuis, 2016-10-12 20:13:31]

The CQ bit was checked by rob.buis@samsung.com

[commit-bot: I haz the power, 2016-10-12 20:13:55]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-12 21:38:06]

Description was changed from

==========
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
==========

to

==========
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
==========

[commit-bot: I haz the power, 2016-10-12 21:38:09]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-10-12 21:39:34]

Description was changed from

==========
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
==========

to

==========
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}
==========

[commit-bot: I haz the power, 2016-10-12 21:39:35]

Patchset 3 (id:??) landed as
https://crrev.com/549c1d11032cc509f1438de8f1e979f9793d53c8
Cr-Commit-Position: refs/heads/master@{#424869}