Bug 173148 – Not possible to remove the 'li' element inside the table cell

• Status: RESOLVED
• Resolution: FIXED
• Component: HTML Editing

[Javier Fernandez]

Created attachment 312421 [details]
Test case to reproduce the issue

What steps will reproduce the problem?
(1) Load the attached test case
(2) Place the cursor at any of the li items
(3) hit backspace

What is the expected result?

Both list items can be deleted.

What happens instead?

None of these list items can be deleted by hitting backspace.

[Javier Fernandez]

This looks like a clear interoperability issue, as both Firefox and IE/Edge allow deleting the list item element inside the table cel

[Javier Fernandez]

Created attachment 312695 [details]
Patch

[Alex Christensen]

Comment on attachment 312695 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=312695&action=review

It would be nice if there were a web platform test for this and a link to the relevant specification.
This seems like it might be good, but I'm not the right reviewer for this code.

> LayoutTests/editing/assert_selection.js:3
> +// found in the LICENSE file.

The LICENSE file isn't included here.

[Javier Fernandez]

(In reply to Alex Christensen from comment #3)
> Comment on attachment 312695 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=312695&action=review
> 
> It would be nice if there were a web platform test for this and a link to
> the relevant specification.

Oh, there is, indeed. I added such test as part of my patch for Blink. The problem is that WebKit doesn't import the WPT tests yet. I was tempted to do it but I thought there were lots of failures and wasn't sure it was a good idea. 


> > LayoutTests/editing/assert_selection.js:3
> > +// found in the LICENSE file.
> 
> The LICENSE file isn't included here.

Indeed, Ill add it. Thanks.

[Darin Adler]

(In reply to Javier Fernandez from comment #4)
> (In reply to Alex Christensen from comment #3)
> > It would be nice if there were a web platform test for this and a link to
> > the relevant specification.
> 
> Oh, there is, indeed. I added such test as part of my patch for Blink. The
> problem is that WebKit doesn't import the WPT tests yet. I was tempted to do
> it but I thought there were lots of failures and wasn't sure it was a good
> idea. 

I think it’s a good idea.

[Javier Fernandez]

@rniwa, any comment on this issue ? Thanks.

[Ryosuke Niwa]

Comment on attachment 312695 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=312695&action=review

Code change looks okay but the test needs to be placed elsewhere or be rewritten (preferred).

> LayoutTests/editing/assert_selection.js:1
> +// Copyright 2016 The Chromium Authors. All rights reserved.

This code is just incomprehensible.
Either we should add this under LayoutTests/imported/ or not add it and write our own test instead.

> LayoutTests/editing/deleting/delete-list-items-in-table-cell-expected.txt:6
> +
> +PASS Delete - When deleting the second ordered list items in a table cell its content is merged with the first list item. 
> +PASS Delete - When deleting the second unordered list items in a table cell its content is merged with the first list item. 
> +PASS Delete - When deleting the last ordered list item in a table cell we must delete the whole ordered list. 
> +PASS Delete - When deleting the last unrdered list item in a table cell we must delete the whole unordered list. 
> +

This output doesn't tell us what kind of markup is being tested at all.
Please use dump-as-markup.js to dump the content as the content is edited.
See other tests.

[Ryosuke Niwa]

Also, please add a test case where the table cell is the root editable element, and other case where the entire editable content is inside a (non-editable) table cell.

[Javier Fernandez]

Created attachment 314194 [details]
Patch

[Javier Fernandez]

@rniwa, could you please take another look ?

[Ryosuke Niwa]

Comment on attachment 314194 [details]
Patch

r=me. The patch looks great now.

[WebKit Commit Bot]

Comment on attachment 314194 [details]
Patch

Clearing flags on attachment: 314194

Committed r220398: <http://trac.webkit.org/changeset/220398>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/33773900>