Opened 8 years ago

Closed 8 years ago

#3165 closed Bug (fixed)

[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 (5)

3165.patch (2.8 KB) - added by Garry Yao 8 years ago.
ie8_space_bug.png (8.8 KB) - added by Frederico Caldeira Knabben 8 years ago.
IE8 issue after 3165.patch
3165_2.patch (5.7 KB) - added by Garry Yao 8 years ago.
3165_3.patch (8.1 KB) - added by Garry Yao 8 years ago.
3165_4.patch (7.1 KB) - added by Garry Yao 8 years ago.

Download all attachments as: .zip

Change History (19)

comment:1 Changed 8 years ago by Frederico Caldeira Knabben

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 ; Changed 8 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 8 years ago by Frederico Caldeira Knabben

Keywords: Confirmed added; Pending removed
Milestone: CKEditor 3.0CKEditor 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 8 years ago by Garry Yao

Component: GeneralCore : Lists
Keywords: IE removed

#4009 has been marked as DUP.

Changed 8 years ago by Garry Yao

Attachment: 3165.patch added

comment:5 Changed 8 years ago by Garry Yao

Keywords: Review? added
Owner: set to Garry Yao
Status: newassigned
Version: SVN (CKEditor)

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

comment:6 Changed 8 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 8 years ago by Frederico Caldeira Knabben

Attachment: ie8_space_bug.png added

IE8 issue after 3165.patch

comment:7 Changed 8 years ago by Frederico Caldeira Knabben

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 8 years ago by Garry Yao

Attachment: 3165_2.patch added

comment:8 Changed 8 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 8 years ago by Frederico Caldeira Knabben

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 8 years ago by Garry Yao

Attachment: 3165_3.patch added

comment:10 Changed 8 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 8 years ago by Frederico Caldeira Knabben

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 8 years ago by Garry Yao

Attachment: 3165_4.patch added

comment:12 Changed 8 years ago by Garry Yao

Keywords: Review? added; Review- removed

comment:13 Changed 8 years ago by Frederico Caldeira Knabben

Keywords: Review+ added; Review? removed

comment:14 Changed 8 years ago by Garry Yao

Resolution: fixed
Status: assignedclosed

Fixed with [4546].

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