#9080 closed Bug (fixed)
List structure broken when deleting a list entry
Reported by: | Teresa Monahan | Owned by: | Garry Yao |
---|---|---|---|
Priority: | Normal | Milestone: | CKEditor 3.6.4 |
Component: | Core : Lists | Version: | 3.6.3 |
Keywords: | IBM | Cc: | Damian, Satya Minnekanti |
Description (last modified by )
To Reproduce:
- Copy the following into source view:
<ol> <li>A</li> <li>B</li> <li>C</li> </ol>
- Return to wysiwyg and place the cursor after 'B'
- Press backspace twice to delete the 2nd entry entirely.
Expected Result: The cursor should be placed after 'A'.
Actual result: A new paragraph is created and the list is broken:
<ol> <li>A</li> </ol> <p> </p> <ol> <li>C</li> </ol>
Even if the user hits backspace again to try to re-create the original list structure, 'A' and 'C' are now treated as separate lists e.g. the numbering is restarted, if the user tries to indent the list, only 'A' is indented.
This was introduced with rev [7392] and also occurs for bulleted lists.
Attachments (3)
Change History (27)
comment:1 Changed 12 years ago by
Description: | modified (diff) |
---|
comment:2 Changed 12 years ago by
Milestone: | → CKEditor 3.6.4 |
---|---|
Status: | new → confirmed |
comment:3 Changed 12 years ago by
Changed 12 years ago by
Attachment: | 9080.patch added |
---|
comment:4 Changed 12 years ago by
Owner: | set to Garry Yao |
---|---|
Status: | confirmed → review |
comment:6 follow-up: 7 Changed 12 years ago by
MS Word does the outdent, however it also maintains the integrity of the list. This is where the behavior in CKEditor differs.
Another important usecase to satisfy after this fix is to maintain the list structure for sub-lists e.g. Copy the following into source mode:
<ol> <li>AAA <ol> <li>BBB</li> <li>CCC</li> <li>DDD <ol> <li>EEE</li> <li>FFF</li> </ol> </li> </ol> </li> </ol>
- Place the cursor after CCC and hit backspace enough times to delete the entry altogether.
Expected Result: BBB and DDD should still be present within the same sub-list.
This example works as expected in MS Word.
comment:7 Changed 12 years ago by
Status: | review → review_failed |
---|
Replying to tmonahan:
Well... this is a very good justification for it.
In fact, we'll not always be able to mimic the MS Word behavior, because HTML has different requirements and limitations.
In this specific case, Word can handle lists starting at deeper levels without problems. We're not able to do so, because HTML lists must always start at level 1.
This means that we made a bad decision in the past and we need to change our position now, providing a simpler solution for it, which simple merge list items on backspace, as expected by the reporter.
comment:8 Changed 12 years ago by
Status: | review_failed → review |
---|
The patch contains 3 parts:
- Reverted the main change of [7392] to not have indentation affected by backspace key;
- Re-fixed #8249 by following the behavior of having backspace join the current list item with previous;
- Last side change related to undo - introduced editor::selectionChange to check immediately the selection, to avoid resulting in wrong undo snapshots.
comment:9 Changed 12 years ago by
Status: | review → assigned |
---|
Changed 12 years ago by
Attachment: | 9080_2.patch added |
---|
comment:10 Changed 12 years ago by
Status: | assigned → review |
---|
comment:12 Changed 12 years ago by
This test is failing: http://ckeditor.t/tt/8249/1.html
Additionally, the following case has a weird effect:
- Load this:
<ol> <li> </li> <li> B</li> <li> C</li> </ol>
- Put the caret at the first item.
- Hit BACKSPACE.
The list item is deleted and a new paragraph will be quickly created before the list, with the selection on it. This paragraph almost immediately disappears, having the selection moved back the to the new top of the list ("B").
In this case, nothing should happen, just like when we have that list item filled with text.
comment:13 Changed 12 years ago by
Status: | review → review_failed |
---|
comment:14 Changed 12 years ago by
Status: | review_failed → review |
---|
The issue in comment:12 can be reproduced without the patch, but it can be handled by the patch as well.
Changed 12 years ago by
Attachment: | 9080_3.patch added |
---|
comment:15 Changed 12 years ago by
Ok, that tc is now ok, but this test is still failing :(
http://ckeditor.t/tt/8249/1.html
comment:16 Changed 12 years ago by
Status: | review → review_failed |
---|
comment:17 Changed 12 years ago by
Status: | review_failed → review |
---|
My fault...I've failed to push some TCs.
comment:18 Changed 12 years ago by
Status: | review → review_passed |
---|
comment:19 Changed 12 years ago by
Resolution: | → fixed |
---|---|
Status: | review_passed → closed |
Fixed with [7540].
comment:20 Changed 12 years ago by
Resolution: | fixed |
---|---|
Status: | closed → reopened |
The issue in comment:12 is still present in Opera 12 - after hitting BACKSPACE whole list is deleted.
In IE8 something different happens:
- After BACKSPACE hit - first list item is deleted, the rest of the list gets proper numbers, new paragraph is created with content from first list item:
<p> </p> <ol> <li> B</li> <li> C</li> </ol>
- Now, when caret is in the paragraph hit DELETE. In visual the list is lost, we can see B and C letters one under another (with empty line before them):
B C
- Go back to source mode. The code is almost the same as in point 1 (after BACKSPACE hit) but we get
<ul>
elements instead of<ol>
. Switching back to visual mode recreates the list with bullets - not numbers.
Steps 2-3 can be repeated indefinitely.
comment:21 follow-up: 22 Changed 12 years ago by
Component: | General → Core : Lists |
---|---|
Resolution: | → fixed |
Status: | reopened → closed |
For the Opera issue it's not a regression, I've opened #9129 for it.
I cannot confirm the above issue on IE8 though.
comment:22 Changed 12 years ago by
Replying to garry.yao:
For the Opera issue it's not a regression, I've opened #9129 for it.
I cannot confirm the above issue on IE8 though.
I can also reproduce this issue on IE8 using the steps and markup specified in comment:12 - the problem does not occur in IE9.
comment:23 Changed 12 years ago by
The steps to reproduce the problem I am seeing are:
- Load this:
<ol> <li> </li> <li> B</li> <li> C</li> </ol>
- Place the cursor at the first list item.
- Press BACKSPACE. The first list item is removed and a paragraph is created before the list.
- Press DELETE. Problem: The paragraph is deleted and the list items no longer appear as a list (even though li is still displayed on the elements path bar).
- Go to Source view and then back to wysiwyg mode. Problem: The list now appears but it has bullets instead of numbers.
I can reproduce this on standalone IE8 on WinXP. It does not happen on an IE9 install running in IE8 mode.
comment:24 Changed 12 years ago by
The above TC is valid though not related to the ticket, it concerns delete key behavior outside of a list structure, which is not handled by our list keystrokes logic, plus it's limited affecting range, opened a new ticket instead #9152.
Please note that, as decided earlier, we'll follow the MS Word behavior on this.
This ticket will exclusively fix the merging behavior of BACKSPACE.