Opened 6 years ago

Closed 6 years ago

Last modified 6 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 6 years ago.
Start situation
02 copy-paste-list-items.png (49.1 KB) - added by Syslogic 6 years ago.
Copy and paste list items
03 generated-code.png (52.8 KB) - added by Syslogic 6 years ago.
The generated code after pasting
04 corrected-after-code-view.png (48.8 KB) - added by Syslogic 6 years ago.
The corrected view after toggling the Code button
05 code-corrected.png (52.0 KB) - added by Syslogic 6 years ago.
The code representing the corrected view
6946.patch (1.3 KB) - added by garry.yao 6 years ago.
6946_2.patch (3.0 KB) - added by garry.yao 6 years ago.
6946_3.patch (9.4 KB) - added by garry.yao 6 years ago.
6946_4.patch (9.5 KB) - added by garry.yao 6 years ago.
6946_5.patch (9.6 KB) - added by garry.yao 6 years ago.

Download all attachments as: .zip

Change History (24)

Changed 6 years ago by Syslogic

Start situation

Changed 6 years ago by Syslogic

Copy and paste list items

Changed 6 years ago by Syslogic

The generated code after pasting

Changed 6 years ago by Syslogic

The corrected view after toggling the Code button

Changed 6 years ago by Syslogic

The code representing the corrected view

comment:1 Changed 6 years ago by garry.yao

  • Keywords IE added; IE7 IE8 lists removed
  • Status changed from new to confirmed

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

Changed 6 years ago by garry.yao

comment:2 Changed 6 years ago by garry.yao

  • Component changed from Core : Pasting to Core : Parser
  • Owner set to garry.yao
  • Status changed from confirmed to review

Changed 6 years ago by garry.yao

comment:3 Changed 6 years ago by garry.yao

Another patch addressing TCs from #6972.

comment:4 Changed 6 years ago by alfonsoml

We should check if this also fixes correctly #6912

comment:5 Changed 6 years ago by wwalc

  • Milestone set to CKEditor 3.5.2

comment:6 Changed 6 years ago by Saare

  • Status changed from review to review_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 6 years ago by garry.yao

comment:7 Changed 6 years ago by garry.yao

  • Status changed from review_failed to review

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 6 years ago by Saare

  • Status changed from review to review_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 6 years ago by garry.yao

comment:9 Changed 6 years ago by garry.yao

  • Status changed from review_failed to review

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

comment:10 Changed 6 years ago by Saare

  • Status changed from review to review_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 6 years ago by garry.yao

comment:11 Changed 6 years ago by garry.yao

  • Status changed from review_failed to review

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 6 years ago by Saare

  • Status changed from review to review_passed

comment:13 Changed 6 years ago by garry.yao

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

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

comment:14 Changed 6 years ago by fredck

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 – 2016 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy