Opened 8 years ago

Closed 8 years ago

#5144 closed Bug (fixed)

Pasting in Chrome can leave <div id="cke_pastebin"> elements

Reported by: Alfonso Martínez de Lizarrondo Owned by: Garry Yao
Priority: Normal Milestone: CKEditor 3.3
Component: Core : Pasting Version: 3.1
Keywords: Confirmed WebKit Review+ Cc:

Description

Reported in http://cksource.com/forums/viewtopic.php?f=11&t=17447

Seems to work fine with Chrome 4.0.249.89 (38071) in Windows XP with the patches for #4988 and #5095. But previously I've seen it fail, so this case must be reviewed

Attachments (2)

5144.patch (1.5 KB) - added by Alfonso Martínez de Lizarrondo 8 years ago.
Proposed patch
5144_2.patch (774 bytes) - added by Garry Yao 8 years ago.

Download all attachments as: .zip

Change History (18)

comment:1 Changed 8 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 3.2

comment:2 Changed 8 years ago by Frederico Caldeira Knabben

Component: GeneralCore : Pasting

comment:3 Changed 8 years ago by Alfonso Martínez de Lizarrondo

Resolution: fixed
Status: newclosed

Fixed with [5118]

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

Resolution: fixed
Status: closedreopened

I was wrong, those patches didn't fix anything and I've been able to reproduce the problem also in Safari (tested only windows for the moment)

The key seems to be to paste plain text with several lines. Copy the text to notepad (or a textarea) and copy from there before trying to paste so it doesn't use html:

line1
line2

If the bug isn't fixed for 3.2 then the entry in changes.html must be removed.

Changed 8 years ago by Alfonso Martínez de Lizarrondo

Attachment: 5144.patch added

Proposed patch

comment:5 Changed 8 years ago by Alfonso Martínez de Lizarrondo

Keywords: WebKit Review? added; Chrome removed

As the problem seems to be that it's cloning the container for each new line, I've inserted a div without any attribute, and then we start reading its contents. That way there won't be any id="cke_pastebin" elements inserted.

comment:6 Changed 8 years ago by Frederico Caldeira Knabben

Keywords: Review- added; Review? removed
Milestone: CKEditor 3.2CKEditor 3.3

The problem here is that we should have no DIVs at all in this case. It's webkit here automating things too much, instead of taking the easy way.

We need to rework the pasted data in this case, stripping the extra divs, creating the correct output.

I've removed the changelog entry with [5127].

comment:7 Changed 8 years ago by David Maggard

Same Error in Air

comment:8 Changed 8 years ago by Bart

I ran into this bug today (using CKE on a Drupal install) Off course you can work around this using the allowed input types and just not allowing DIV's to be parsed on "save", but would be nice to not have these when copying from, say Notepad to Chrome ...

Changed 8 years ago by Garry Yao

Attachment: 5144_2.patch added

comment:9 Changed 8 years ago by Garry Yao

Keywords: Review? added; Review- removed
Owner: changed from Alfonso Martínez de Lizarrondo to Garry Yao
Status: reopenednew

Using 'body' element as a trick to void pasting in webkit be context-sensitive.

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

Keywords: Review- added; Review? removed

The result of this patch is similar to the previous one. Safari now generates this code:

<p>
	&nbsp;</p>
<div>
	line1</div>
<div>
	line2</div>

But IE and Firefox generate this one:

<p>
	line1<br />
	line2<br />
	&nbsp;</p>

comment:11 Changed 8 years ago by Frederico Caldeira Knabben

This ticket is about having <div id="cke_pastebin">. If in WebKit we have those other divs for each line, it may be related to the way the browser handles the pasting data. If that's the case, we must have a different ticket for it, leaving this one to fix the cke_pastebin issue only.

comment:12 Changed 8 years ago by Alfonso Martínez de Lizarrondo

But the output now it's the same that the one generated with the first patch, and I thought that the reason that it was rejected it was due to having those extra <div>.

comment:13 Changed 8 years ago by Frederico Caldeira Knabben

Ah, sorry... I've just made a comment for the developer evaluation without looking at the ticket history. In fact, having the DIVs is plain wrong based on the previous review comments.

comment:14 Changed 8 years ago by Garry Yao

Keywords: Review? added; Review- removed

I don't understand your points here, we just want to eliminate the cloning of the the pasting container, while using what element to present a line is just Webkit's preference, we don't have much to do here.

comment:15 Changed 8 years ago by Frederico Caldeira Knabben

Keywords: Review+ added; Review? removed

Ok, we don't need to fix the way WebKit handles the clipboard data for now. It's out of the ticket scope.

comment:16 Changed 8 years ago by Garry Yao

Resolution: fixed
Status: newclosed

Fixed with [5385].

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