Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#6946 closed Bug (fixed)

[IE] copy and paste lists from own document leads into invalid lists

Reported by: Syslogic Owned by: Garry Yao
Priority: Normal Milestone: CKEditor 3.5.3
Component: Core : Parser Version: 3.5
Keywords: IE Cc:

Description

When copying and pasting lists in the CKEditor lists end up malformed in Internet Explorer (7 and 8).


Test setup:

  1. Add code below
    <p>
    	Paragraph before</p>
    <ul>
    	<li>
    		First item</li>
    	<li>
    		Second item</li>
    	<li>
    		Third item</li>
    	<li>
    		Fourth item</li>
    </ul>
    <p>
    	Paragraph after</p>
    
  1. copy the second and the third item and paste it below the last paragraph.

Now the bullets are gone. Visually it looks like one paragraph with between the two items a <br />

  1. Press the Code button. The code definitely doesn't look like what was expected: The items are not in an <ul> tag.
  1. Press the Code button again. The document is changed compared to the contents which was in the CKEditor before toggling the Code view.
  1. Press the Code button again. The code now represents the end result of the paste action.

When pasting a subset of a list, the new pasted data should be wrapped in a <ul> tag in order to create valid code.

Attachments (10)

01 start-situation.png (48.2 KB) - added by Syslogic 10 years ago.
Start situation
02 copy-paste-list-items.png (49.1 KB) - added by Syslogic 10 years ago.
Copy and paste list items
03 generated-code.png (52.8 KB) - added by Syslogic 10 years ago.
The generated code after pasting
04 corrected-after-code-view.png (48.8 KB) - added by Syslogic 10 years ago.
The corrected view after toggling the Code button
05 code-corrected.png (52.0 KB) - added by Syslogic 10 years ago.
The code representing the corrected view
6946.patch (1.3 KB) - added by Garry Yao 10 years ago.
6946_2.patch (3.0 KB) - added by Garry Yao 10 years ago.
6946_3.patch (9.4 KB) - added by Garry Yao 10 years ago.
6946_4.patch (9.5 KB) - added by Garry Yao 10 years ago.
6946_5.patch (9.6 KB) - added by Garry Yao 10 years ago.

Download all attachments as: .zip

Change History (24)

Changed 10 years ago by Syslogic

Attachment: 01 start-situation.png added

Start situation

Changed 10 years ago by Syslogic

Copy and paste list items

Changed 10 years ago by Syslogic

Attachment: 03 generated-code.png added

The generated code after pasting

Changed 10 years ago by Syslogic

The corrected view after toggling the Code button

Changed 10 years ago by Syslogic

Attachment: 05 code-corrected.png added

The code representing the corrected view

comment:1 Changed 10 years ago by Garry Yao

Keywords: IE added; IE7 IE8 lists removed
Status: newconfirmed

This's actually an unfixed case of #5626, ticket tc updated with [6299].

Changed 10 years ago by Garry Yao

Attachment: 6946.patch added

comment:2 Changed 10 years ago by Garry Yao

Component: Core : PastingCore : Parser
Owner: set to Garry Yao
Status: confirmedreview

Changed 10 years ago by Garry Yao

Attachment: 6946_2.patch added

comment:3 Changed 10 years ago by Garry Yao

Another patch addressing TCs from #6972.

comment:4 Changed 10 years ago by Alfonso Martínez de Lizarrondo

We should check if this also fixes correctly #6912

comment:5 Changed 10 years ago by Wiktor Walc

Milestone: CKEditor 3.5.2

comment:6 Changed 10 years ago by Sa'ar Zac Elias

Status: reviewreview_failed

Put in source:

<dl><dd>a</dd><dt>b</dt></dl>

Toggle to wysiwyg and back to source.
Expected:

<dl>
	<dd>
		a</dd>
	<dt>
		b</dt>
</dl>

Result:

<dl>
	<dd>
		a
		<dl>
			<dt>
				b</dt>
		</dl>
	</dd>
</dl>

Changed 10 years ago by Garry Yao

Attachment: 6946_3.patch added

comment:7 Changed 10 years ago by Garry Yao

Status: review_failedreview

Beyond addressing this particular issue, the patch is a simplification rewrite of the parse's DOM constructing logic, all current TCs regard parsing have to be verified.

comment:8 Changed 10 years ago by Sa'ar Zac Elias

Status: reviewreview_failed

Insert:

<ul><ol></ul>

Expected result:

<ul>
	<li>
		<ol>
		</ol>
	</li>
</ul>

Actual result:

<ul>
	<li>
		<ol>
		</ol>
	</li>
	<li>
		<ol>
		</ol>
	</li>
</ul>

Changed 10 years ago by Garry Yao

Attachment: 6946_4.patch added

comment:9 Changed 10 years ago by Garry Yao

Status: review_failedreview

Nice review, not sure why #3828 TC are not enrolled into the parser dt, I'm moving it now with [6546].

comment:10 Changed 10 years ago by Sa'ar Zac Elias

Status: reviewreview_failed

Insert:

<li>1</li>
<dt>2</dt>
<dd>3</dd>

Expected:

<ul>
	<li>
		1</li>
</ul>
<dl>
	<dt>
		2</dt>
	<dd>
		3</dd>
</dl>

Actual:

<dl>
	<dt>
		2</dt>
	<dd>
		3</dd>
</dl>

(List item is left out)

Changed 10 years ago by Garry Yao

Attachment: 6946_5.patch added

comment:11 Changed 10 years ago by Garry Yao

Status: review_failedreview

It's quite a tricky case, but we should be able to handle it : two times of invalid fixing.

While the correct expected result is a bit counterintuitive perhaps, but it should be instead, as the dl is invalid inside ul thus are lifted above it.

<dl><dt>2</dt></dl><ul><li>1</li></ul>

comment:12 Changed 10 years ago by Sa'ar Zac Elias

Status: reviewreview_passed

comment:13 Changed 10 years ago by Garry Yao

Resolution: fixed
Status: review_passedclosed

Fixed with [6561], thanks Saar for hq review.

comment:14 Changed 10 years ago by Frederico Caldeira Knabben

I've added a few TCs for the cases found here with [6641].

One of the tests is not passing because the expectation explained on comment:11 is wrong. The one in comment:10 is definitely what we must have.

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