Ticket #3165 (closed Bug: fixed)

Opened 6 years ago

Last modified 5 years ago

[IE] enterKey incorrect with list item

Reported by: garry.yao Owned by: garry.yao
Priority: Normal Milestone: CKEditor 3.1
Component: Core : Lists Version: SVN (CKEditor) - OLD
Keywords: Confirmed Review+ Cc:

Description

Reproducing Procedures

  1. Open the replace by code example page;
  2. Make document content and selection as following:
    <ol>
    	<li>
    		level1^
    		<ol>
    			<li>
    				level2</li>
    		</ol>
    	</li>
    </ol>
    
  3. Press Enter key
  • Actual Result :
    <ol>
    	<li>
    	</li>
    	<li>
    		level1
    		<ol>
    			<li>
    				level2</li>
    		</ol>
    	</li>
    </ol>
    
  • Expected Result :
    <ol>
    	<li>
    		level1
    		<ol>
    			<li>
    				level2</li>
    		</ol>
    	</li>
    	<li>
    	</li>
    </ol>
    

Attachments

3165.patch (2.8 KB) - added by garry.yao 5 years ago.
ie8_space_bug.png (8.8 KB) - added by fredck 5 years ago.
IE8 issue after 3165.patch
3165_2.patch (5.7 KB) - added by garry.yao 5 years ago.
3165_3.patch (8.1 KB) - added by garry.yao 5 years ago.
3165_4.patch (7.1 KB) - added by garry.yao 5 years ago.

Change History

comment:1 follow-up: ↓ 2 Changed 6 years ago by fredck

  • Keywords Confirmed added

Confirmed with IE.

I'm having the expected result specified in the ticket with FF, but the correct result should be instead:

<ol>
	<li>
		level1</li>
	<li>
		^
		<ol>
			<li>
				level2</li>
		</ol>
	</li>
</ol>

comment:2 in reply to: ↑ 1 ; follow-up: ↓ 3 Changed 6 years ago by garry.yao

  • Keywords Pending added; Confirmed removed

Replying to fredck: I've go the result you expected with current trunk in FF, but when comparing to MS-Word, it would give result as below, which seems to be more reasonable.

<ol>
	<li>
		level1
	</li>
	<li>^
	</li>
	<li>
		<ol>
			<li>
				level2</li>
		</ol>
	</li>	
</ol>

comment:3 in reply to: ↑ 2 Changed 5 years ago by fredck

  • Keywords Confirmed added; Pending removed
  • Milestone changed from CKEditor 3.0 to CKEditor 3.1

Replying to garry.yao:

which seems to be more reasonable.

No, it's not. A text empty <li> can easily become a selection pain for us.

comment:4 Changed 5 years ago by garry.yao

  • Keywords IE removed
  • Component changed from General to Core : Lists

#4009 has been marked as DUP.

Changed 5 years ago by garry.yao

comment:5 Changed 5 years ago by garry.yao

  • Status changed from new to assigned
  • Owner set to garry.yao
  • Version set to SVN (CKEditor)
  • Keywords Review? added

Ticket Test added at :
http://ckeditor.t/tt/3165/1.html.

comment:6 Changed 5 years ago by garry.yao

The way that v2 used to work around this situation is wrong (#1420) where a NBSP is used as line holder, for the following reasons:

  1. Break keyboard navigation;
  2. Break reversibility: if user press 'Backspace' after he pressed 'Enter', the result is not guaranteed and since we don't have managed backspace keystroke (at least for now), it sucks.

Changed 5 years ago by fredck

IE8 issue after 3165.patch

comment:7 Changed 5 years ago by fredck

  • Keywords Review- added; Review? removed

Things look better after the patch, but the <br> trick has its drawback:

  • In FF, the addition <br> remains there. It may not be an issue with the enter code, but it should be cleaned up on output.
  • In IE, the most important issue. Check out the attached screenshot. The <br> generates some unwanted space in the list. Even worst, if you continue hitting ENTER and adding text, the number of <br>s increase and the space gets bigger.

So, much probably, the solution used to fill paragraphs must be the same to be used here.

Changed 5 years ago by garry.yao

comment:8 Changed 5 years ago by garry.yao

  • Keywords Review? added; Review- removed

As discussed with Fred today:

In FF, the addition <br> remains there. ...it should be cleaned up on output.

FF works well with <br> as filler in wysiwyg, while filler node in output should always be &nbsp;

In IE, ...The <br> generates some unwanted space in the list.

<br> is quite broken in IE8, though works in IE6/7, we would keep using the NBSP for IE only, even though the backspace logic is still buggy.

comment:9 Changed 5 years ago by fredck

  • Keywords Review- added; Review? removed
  • In Firefox, the <br> is still there if we type some text after hitting enter:
<ol>
	<li>
		level1</li>
	<li>
		Some text<br />
		<ol>
			<li>
				level2</li>
		</ol>
	</li>
</ol>
  • We have the same issue with IE, but instead of a <br> we have a &nbsp;:
<ol>
	<li>
		level1</li>
	<li>
		Some text&nbsp;
		<ol>
			<li>
				level2</li>
		</ol>
	</li>
</ol>

Even worst, if you continually hit ENTER and type text for each new item, the last one will accumulate lots of &nbsp;, one for each ENTER.

In all browsers we should have the &nbsp; only if the list remains empty, like the following:

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

Changed 5 years ago by garry.yao

comment:10 Changed 5 years ago by garry.yao

  • Keywords Review? added; Review- removed

The new patch is targeting the above mentioned issue.
Noted that the changes within the selection plugin is to make sure that only one NBSP is to be introduced at this point.

comment:11 Changed 5 years ago by fredck

  • Keywords Review- added; Review? removed

Ok for the changes, but is it possible to have the filtering thing in the list plugin? The data processor is really becoming a mess.

Changed 5 years ago by garry.yao

comment:12 Changed 5 years ago by garry.yao

  • Keywords Review? added; Review- removed

comment:13 Changed 5 years ago by fredck

  • Keywords Review+ added; Review? removed

comment:14 Changed 5 years ago by garry.yao

  • Status changed from assigned to closed
  • Resolution set to fixed

Fixed with [4546].

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