Trying to remove a sub-list from a list entry deletes the entire list

[Zoltan Koszegi]

Steps To Recreate: 1) Make a web page with paragraphs surrounding a leveled list (OL or UL) where the last item is a single child on the 2nd level, like so:

how much

1. wood would
2. a wood
   1. chuck
   2. chuck
3. if a woodchuck
   1. could chuck

wood

2) Attempt to remove the 'could chuck' line by selecting it and pressing delete or backspace.

RESULT 1 (of 2): The whole list is removed!

3) Undo the delete and try cutting the item, instead.

RESULT 2 (of 2): The final paragraph ('wood') becomes a 2nd line in the 3rd level-1 list item.

I expected both attempts to act like the previous release of ckEditor and simply remove that list item.

NOTE: If there are 2 or more items under "3", selecting all of the sub-list has the same result. Selecting the just one of a 2+ sub-list only removes the selected item.

[Jakub Ś]

To reproduce use below code:

<p>how much</p>
<ol>
	<li>wood would</li>
	<li>a wood 
	<ol>
		<li>chuck</li>
		<li>chuck</li>
	</ol>
	</li>
	<li>if a woodchuck
	<ol>
		<li>could chuck</li>
	</ol>
	</li>
</ol>
<p>wood</p>

I was able to reproduce first isse from CKEDitor 4.0 in all browsers (works fine in 4.0 beta and 3.x).

@zkoszegi I wasn't able to reproduce second issue. Cutting last list item just removed text in last list item and that is it. Is there anything you have forgotten to add?

[Frederico Caldeira Knabben]

Please don't scream! :)

[Olek Nowodziński]

A first bad commit ​git:a930e533e5e04ac7

[Olek Nowodziński]

• Created dev ​t/10646 branch with a fix and corresponding test branch with a manual test sample.
• Refactorized getSelectedTableList() method in editable, explained the algorithm.
• Still I wonder whether making getSelectedTableList() public and tested automatically would make much more sense that manual tests.

[Piotrek Koszuliński]

Please split code refactorisation from a fix to at least two different commits. It is hard to understand what's really changed.

Regarding testing - it should be possible to write automated test since backspace is handled by our custom handler in this case.

Of course more precise unit tests for getSelectedTableList would be nice, but let's not make them part of this ticket. Especially that we'll be refactoring that part soon.

[Olek Nowodziński]

• Split the fix into two commits.
• Found out that similar tests are in dt/core/editable/keystrokes.html
  • Added test cases specific for this ticket.
  • Removed MT (obsolete since introduced automated tests).

[Piotr Jasiun]

Review passed. I've rebased dev to master and added ticket id to tests. I wasn't also able to reproduce second issue here (problem with cut) so let's mark it as "worksforme".

[Olek Nowodziński]

Merged branch into master, dev (​​git:2998399), tests (0160e06).