Opened 6 years ago

Closed 6 years ago

#10646 closed Bug (fixed)

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

Reported by: Zoltan Koszegi Owned by: Olek Nowodziński
Priority: Normal Milestone: CKEditor 4.2.1
Component: General Version: 4.0
Keywords: Oracle Cc:

Description

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.

Change History (11)

comment:1 Changed 6 years ago by Zoltan Koszegi

Version: 4.1.14.1.2

comment:2 Changed 6 years ago by Frederico Caldeira Knabben

Cc: Frederico Caldeira Knabben added

comment:3 Changed 6 years ago by Jakub Ś

Cc: Frederico Caldeira Knabben removed
Status: newconfirmed
Version: 4.1.24.0

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?

comment:4 Changed 6 years ago by Frederico Caldeira Knabben

Summary: TRYING TO REMOVE A SUB-LIST FROM A LIST ENTRY DELETES THE ENTIRE LISTTrying to remove a sub-list from a list entry deletes the entire list

Please don't scream! :)

comment:5 Changed 6 years ago by Frederico Caldeira Knabben

Keywords: Oracle added; oracle removed
Milestone: CKEditor 4.2.1

comment:6 Changed 6 years ago by Olek Nowodziński

Owner: set to Olek Nowodziński
Status: confirmedassigned

A first bad commit git:a930e533e5e04ac7

Last edited 6 years ago by Olek Nowodziński (previous) (diff)

comment:7 Changed 6 years ago by Olek Nowodziński

Status: assignedreview
  • 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.

comment:8 Changed 6 years ago by Piotrek Koszuliński

Status: reviewreview_failed

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.

comment:9 Changed 6 years ago by Olek Nowodziński

Status: review_failedreview
  • 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).

comment:10 Changed 6 years ago by Piotr Jasiun

Status: reviewreview_passed

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".

comment:11 Changed 6 years ago by Olek Nowodziński

Resolution: fixed
Status: review_passedclosed

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

Note: See TracTickets for help on using tickets.
© 2003 – 2019 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy