Lists not deleted properly using backspace

[James Cunningham]

Steps to reproduce the defect:

1. Open the Ajax sample.

2. Paste the following code into WYSIWYG mode

<ol>
	<li>
		item 1</li>
	<li>
		item 2
		<ol>
			<li>
				item 3</li>
			<li>
				item 4</li>
			<li>
				item 5</li>
		</ol>
	</li>
</ol>

3. Switch back to source mode.

4. Select all of the list items with the mouse & click backspace.

Expected: Nothing remains in the editor.

Actual: The list is not fully deleted (see screenshot) & the following code still remains in the editor:

<ol>
	<li>
		<ol>
			<li>
				&nbsp;</li>
		</ol>
	</li>
</ol>

[Garry Yao]

It's not IE specific and the backspace key behavior over range selection is quite browser dependent, we suggest not to rely on it.

[James Cunningham]

Why has this ticket been closed as invalid? Highlighting a list with the mouse and using backspace to delete it is common practice for deleting a list.

[Jakub Ś]

This has been true since CKEditor version 3.0. Behavior is browser dependable.

For IE6, IE7 & IE9:
Selecting list with a mouse and pressing del or backspace button always leaves code shown above by @jamescun.
Selecting list with CRTL+A and pressing del or backspace button removes everything.
Using the following combination: CRTL+A; DEL button; CRTL+Z; backspace button; leaves the code shown above by @jamescun.

IE8 behaves almost the same. The only difference is the combination CRTL+A; DEL button; CRTL+Z; backspace button; which leaves the following code:

<ol>
	<li>
		&nbsp;</li>
	<li>
		&nbsp;</li>
	<li>
		&nbsp;</li>
</ol>

Webkit and Opera are more consistent here. No matter whether you use mouse or CRTL+A, you are not able to select the whole list, thus after pressing backspace or del button you are always left with:

<ol>
	<li>
		&nbsp;</li>
</ol>

Pressing backspace or del one more time removes the leftovers.


In FireFox it works.

[Sa'ar Zac Elias]

Patch introduces a new IE9-friendly version of fixListRange that was proposed in #6432.

[Garry Yao]

Nice try Saar, while the "fixListRange" method wasn't of good quality (my fault) ;/ it breaks on the following cases:

<ol>
	<li>
		<input name="name" type="checkbox" />[item1</li>
	<li>
		item2]</li>
</ol>

<ol>
	<li>
		[item1
		<ol>
			<li>
				item2]</li>
		</ol>
	</li>
</ol>

Also because it's violation on one principle that in all dom::range methods should have no impact on doc selection.

It looks like range::enlarge is the best place for introducing it, as it's all about expanding the range boundaries on a particular situation, should fit well in the semantics.

[Sa'ar Zac Elias]

At least with IE8 (didn't test IE9 yet):

• Add a table. Select it as a control element. Hit BACKSPACE. JS error is thrown.
• The 2nd case you've specified failes (the first item should remain).

[Garry Yao]

The 2nd case you've specified failes (the first item should remain).

I vote instead the opposite, where the removes the entire list, as the selection is fully spanning the list, referencing the MS-Word behavior.

Instead of applying this fix to just IE, I'm now proposing it as a generic fix instead, which might give other browsers better result upon it.

[Sa'ar Zac Elias]

1. With the following:

   <ol>
   	<li>
   		<input name="name" type="checkbox" />[item1</li>
   	<li>
   		item2]</li>
   </ol>
   

Click on numbered list icon.
IE<9 : 2nd list item, though truncated, is not removed.
FF, Webkit : Cursor disappears. (FF: br is displayed in elements path).

---

2. IE: Empty the editor. Add a table. Select it from the elements patch. Hit BACKSPACE. JS error is triggered:

   Message: 'is' is null or not an object
   Line: 773
   Char: 4
   Code: 0
   URI: /core/dom/range.js
   

---

3. In Webkit (Chrome at least), following TC fails:

   <ol>
   	<li>
   		[item1
   		<ol>
   			<li>
   				item2]</li>
   		</ol>
   	</li>
   </ol>
   

[Garry Yao]

Replying to Saare:

1. With the following:

I don't understand how this's related....

2. IE: Empty the editor. Add a table. Select it from the elements patch. Hit BACKSPACE. JS error is triggered:

This one is addressed in the new patch.

3. In Webkit (Chrome at least), following TC fails:

The patch has intended to address the issue for all browsers, while as we saw here it's not working in this case for webkit, while I dont think we should be blocked by it considering the ticket is of an IE issue and we don't brought a regression for webkit.

[Frederico Caldeira Knabben]

​http://ckeditor.t/tt/7645/1.html ​http://ckeditor.t/tt/7645/2.html ​http://ckeditor.t/tt/7645/3.html

[Sa'ar Zac Elias]

Replying to garry.yao:

Replying to Saare: The patch has intended to address the issue for all browsers, while as we saw here it's not working in this case for webkit, while I dont think we should be blocked by it considering the ticket is of an IE issue and we don't brought a regression for webkit.

Since the same issue exsists in WebKit as well, it must be fixed there too.

• Ther're some debugger leftovers in the patch.
• ​http://ckeditor.t/dt/plugins/styles/styles.html fails in WebKit.

[Garry Yao]

I've altered the implementation a bit to have webkit counted into the fix, note that changes to the other plugins (other than wysiwyg and selection) are necessarily caused by the update of CKEDITOR.tools.tryThese.

​http://ckeditor.t/dt/plugins/styles/styles.html failures are not brought by the patch.

[Sa'ar Zac Elias]

R+ for 7645_6, to avoid redudant changes in 7645_5.

[Garry Yao]

Fixed with [7243].