Opened 6 years ago

Closed 6 years ago

Last modified 3 years ago

#7131 closed Bug (fixed)

Copy/Paste Word List should preserve list properties

Reported by: Lynne Kues Owned by: Garry Yao
Priority: Normal Milestone: CKEditor 3.5.3
Component: Plugin : Paste from Word Version: 3.1
Keywords: IBM Cc: satya_minnekanti@…, jamcunni@…

Description (last modified by Krzysztof Studnik)

Copy a list formatted as follows from Word. Paste into CKEditor.

A.	Test
B.	Test
C.	Test
        a.	111
        b.	111
        c.	1111
F.	test

The list properties (i.e., number type, start number) are not preserved. The <liststyle> plugin supports these properties and should preserve these on paste.

Attachments (10)

7131.patch (4.4 KB) - added by Garry Yao 6 years ago.
7131_2.patch (13.7 KB) - added by Garry Yao 6 years ago.
7131.docx (14.7 KB) - added by Garry Yao 6 years ago.
Test case
TC_7131_2.docx (10.7 KB) - added by Frederico Caldeira Knabben 6 years ago.
Word file for test.
7131_3.patch (12.8 KB) - added by Garry Yao 6 years ago.
7131_3.docx (14.1 KB) - added by Sa'ar Zac Elias 6 years ago.
7131_4.patch (16.6 KB) - added by Garry Yao 6 years ago.
7131_5.patch (17.2 KB) - added by Garry Yao 6 years ago.
7131_6.patch (19.3 KB) - added by Garry Yao 6 years ago.
7131_7.patch (20.3 KB) - added by Sa'ar Zac Elias 6 years ago.

Download all attachments as: .zip

Change History (41)

comment:1 Changed 6 years ago by Lynne Kues

Keywords: IBM added

comment:2 Changed 6 years ago by Lynne Kues

Version: 3.5.1

comment:3 Changed 6 years ago by Krzysztof Studnik

Component: GeneralCore : Pasting
Description: modified (diff)
Status: newconfirmed
Version: 3.5.13.1

Pasting under IE - result in WYSIWYG

1.A Test
2.B Test
3.C Test
   1.111
   2.111
   3.1111
4.F test

comment:4 Changed 6 years ago by Satya Minnekanti

Cc: satya_minnekanti@… added

Changed 6 years ago by Garry Yao

Attachment: 7131.patch added

comment:5 Changed 6 years ago by Garry Yao

Owner: set to Garry Yao
Status: confirmedreview

Expecting the following result simply because in-consequent numbering is not well supported in CSS:

A.dddd
B.dddd
C.dddd 
 a.dddd
 b.dd
 c.dd
D.ddddd

comment:6 Changed 6 years ago by Garry Yao

New patch addresses most of the current pains from list pasting:

  1. Using "value" attribute (on list item) to present inconsequent list numbering, though it's currently deprecated on XHTML1.0 strict.
  2. "list-style-type" pulled up to list root node when possible.

Changed 6 years ago by Garry Yao

Attachment: 7131_2.patch added

Changed 6 years ago by Garry Yao

Attachment: 7131.docx added

Test case

comment:7 Changed 6 years ago by Wiktor Walc

The "value" attribute is copied to the next list item when adding a new entry to the list. In the attached test case, the last item "F" is duplicated when creating more list items:

A.	Test
B.	Test
C.	Test
        a.	111
        b.	111
        c.	1111
F.	test
F.	test
F.	test

comment:8 in reply to:  7 Changed 6 years ago by Garry Yao

Replying to wwalc:

The "value" attribute is copie...

As a enter key issue is encountered here, let's leave this a separate ticket.

comment:9 Changed 6 years ago by Wiktor Walc

Milestone: CKEditor 3.5.3

Separate ticket created: #7330

comment:10 Changed 6 years ago by Frederico Caldeira Knabben

Status: reviewreview_failed

With FF, the patch makes no difference and the list is not pasted as a list.

With IE, it's much better, but still not perfect. I'll add a new TC file that reflects the reporter TC. Empty D and E items are also added.

Changed 6 years ago by Frederico Caldeira Knabben

Attachment: TC_7131_2.docx added

Word file for test.

comment:12 in reply to:  10 ; Changed 6 years ago by Garry Yao

Status: review_failedreview

Replying to fredck:

With FF, the patch makes no difference and the list is not pasted as a list.

#6662 has to be checked.

With IE, it's much better, but still not perfect. I'll add a new TC file that reflects the reporter TC. Empty D and E items are also added.

It's interesting how you created that list but you've just cheated IE, other browsers works just well, so, an IE bug.

Replying to Saare: WFM in both IE and FF4?

comment:13 in reply to:  12 Changed 6 years ago by Frederico Caldeira Knabben

Status: reviewreview_failed

Replying to garry.yao:

#6662 has to be checked.

At this point #6662 needs to be fully fixed first, otherwise we're not able to check this ticket with FF.

It's interesting how you created that list but you've just cheated IE, other browsers works just well, so, an IE bug.

I've just used the only feature I've found in Word to continue the list from a different counting number: Right-Click the list item > Set Numbering Value... > Check Continue from previous List > Check Advance value (skip numbers ) > Set value to: F.

It looks like MS Word sends the skipped items as well. Is there any chance for us to identify and ignore them?

comment:14 Changed 6 years ago by Garry Yao

It looks like there's no way to tell the difference with plain empty list bullet, see if others get some idea.

comment:15 Changed 6 years ago by Garry Yao

Status: review_failedreview

Ask for another review as #6662 is now fixed.

I've just used the only feature I've found in Word to continue the list from a different counting number...

If you check the ticket TC doc file, it doesn't have this issue, so it looks like there's something to do with the way of editing the list, anyway I don't think we can find a solution here.

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

Can't we use the "mso-hide: all" style?

comment:17 in reply to:  16 Changed 6 years ago by Frederico Caldeira Knabben

Status: reviewreview_failed

Replying to Saare:

Can't we use the "mso-hide: all" style?

In fact this looks like the way for it.

comment:18 Changed 6 years ago by James Cunningham

Cc: jamcunni@… added

Changed 6 years ago by Garry Yao

Attachment: 7131_3.patch added

comment:19 Changed 6 years ago by Garry Yao

Status: review_failedreview

@Saare, You're awesome, I neglected that!

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

Status: reviewreview_failed
  • Seems like FF also adds the D,E bullets, are we able to fix it there?
  • Seems like this file does not work in FF.
  • Attaching another document that fails parsing.

Changed 6 years ago by Sa'ar Zac Elias

Attachment: 7131_3.docx added

Changed 6 years ago by Garry Yao

Attachment: 7131_4.patch added

comment:21 in reply to:  20 Changed 6 years ago by Garry Yao

Status: review_failedreview

Replying to Saare:

  • Seems like FF also adds the D,E bullets, are we able to fix it there?

I figured it out, yes it affects all browsers actually, that the empty list bullets issue only persist if you check "Continue from previous List" but not "Start new list" in the list numbering dialog;

It's fixed now.

  • Attaching another document that fails parsing.

This's another story, where we have two lists that are too close (without any break to denote they're two ;/ ), I've added another routine to deal with it.

Changed 6 years ago by Garry Yao

Attachment: 7131_5.patch added

comment:22 Changed 6 years ago by Garry Yao

Caught by Wiktor:

  1. List bullet pattern like (2) are not well handled;
  2. Fixing redundant list item numbering.

comment:23 Changed 6 years ago by Wiktor Walc

Status: reviewreview_failed

Something might be still wrong here:

 'cke:listsymbol' is null or not an object

when pasting documents from #7209

comment:24 Changed 6 years ago by Garry Yao

Status: review_failedreview

comment:25 Changed 6 years ago by Wiktor Walc

Status: reviewreview_failed

IE8 now shows an internal error when using the following example: http://dev.ckeditor.com/attachment/ticket/6570/ordered_list.docx (it did not happen in 3.5.2, the list properties were not recognized properly in 3.5.2, but the error didn't occur)

The JavaScript error is:

 '0' is null or not an object, default.js line 220

Changed 6 years ago by Garry Yao

Attachment: 7131_6.patch added

comment:26 Changed 6 years ago by Garry Yao

Status: review_failedreview

comment:27 Changed 6 years ago by Wiktor Walc

Status: reviewreview_failed

With the attached patch IE8 now throws an alert message with the following document: list-test.doc from #1457

  'undefined' is null or not an object  default.js, line 643 character 8

comment:28 Changed 6 years ago by Wiktor Walc

(Just a note that I could not find any other issues while checking this patch)

Changed 6 years ago by Sa'ar Zac Elias

Attachment: 7131_7.patch added

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

Status: review_failedreview_passed

New patch fixes the error. FF 3.6 will still produce incorrect result:

<ol>
	<li>
		one</li>
	<li>
		two
		<ol style="list-style-type: lower-alpha;">
			<li>
				AAA</li>
			<li>
				BBB</li>
		</ol>
	</li>
</ol>
<p style="margin-left: 108pt;">
	&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; i.&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; III</p>
<p style="margin-left: 108pt;">
	&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; ii.&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; IIII</p>
<ol style="list-style-type: lower-alpha;">
	<li value="3">
		CCC</li>
	<li>
		three</li>
</ol>

But this is because we have no clear indication of list items there:

<p class="MsoNormal" style="margin-left: 108pt; text-indent: -108pt;"><span style=""><span style=""><span style="font: 7pt &quot;Times New Roman&quot;;">&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;
</span>i.<span style="font: 7pt &quot;Times New Roman&quot;;">&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;
</span></span></span><span dir="LTR"></span>III</p>

<p class="MsoNormal" style="margin-left: 108pt; text-indent: -108pt;"><span style=""><span style=""><span style="font: 7pt &quot;Times New Roman&quot;;">&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;
</span>ii.<span style="font: 7pt &quot;Times New Roman&quot;;">&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;
</span></span></span><span dir="LTR"></span>IIII</p>

FF4 works, and at least we got no error. R+ for me as for other cases, please give the latest patch your review.
@Garry, this is a wonderful enhancement, congratulations.

comment:30 Changed 6 years ago by Garry Yao

Resolution: fixed
Status: review_passedclosed

Fixed with [6616], thanks for the throughly review.

comment:31 Changed 3 years ago by Frederico Caldeira Knabben

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