Summary: | Not possible to remove the 'li' element inside the table cell | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Javier Fernandez <jfernandez> | ||||||||
Component: | HTML Editing | Assignee: | Javier Fernandez <jfernandez> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | achristensen, cdumez, commit-queue, darin, enrica, jfernandez, rniwa, sam, webkit-bug-importer, wenson_hsieh | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
See Also: | https://bugs.chromium.org/p/chromium/issues/detail?id=731621 | ||||||||||
Attachments: |
|
This looks like a clear interoperability issue, as both Firefox and IE/Edge allow deleting the list item element inside the table cel Created attachment 312695 [details]
Patch
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. (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. (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. @rniwa, any comment on this issue ? Thanks. 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. 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. Created attachment 314194 [details]
Patch
@rniwa, could you please take another look ? Comment on attachment 314194 [details]
Patch
r=me. The patch looks great now.
Comment on attachment 314194 [details] Patch Clearing flags on attachment: 314194 Committed r220398: <http://trac.webkit.org/changeset/220398> All reviewed patches have been landed. Closing bug. |
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.