Opened 8 years ago

Last modified 7 years ago

#8386 confirmed Bug

IE: When copying and pasting a table, an empty <p> is appended on output

Reported by: naresh.sivaraman Owned by:
Priority: Normal Milestone:
Component: Core : Pasting Version: 3.0
Keywords: IE Oracle HasPatch Cc: mail@…

Description (last modified by Jakub Ś)

Steps to reproduce:

  1. Open CKEDITOR.
  2. Create a table.
  3. Highlight the table with "Select All" button and click "Copy" icon.
  4. Click "New Page" icon.
  5. Click "Paste" icon.
  6. Click "Source" -> HTML source is displayed, there is "<p> &nbsp;</p>".
  7. Click "Source" -> HTML page is displayed, line break is inserted before the table.

Bug is present since 3.4.2 i believe.

Attachments (4)

Workaround.patch (740 bytes) - added by Yaswanth 7 years ago.
Workaround 2.
Fix_working_copy.patch (2.2 KB) - added by Yaswanth 7 years ago.
Workaround 3_Working copy of Fix.
Fix_working_copy_Updated.patch (2.5 KB) - added by Yaswanth 7 years ago.
Workaround 3_Working copy of Fix_Updated.
workaround.patch (1.0 KB) - added by Yaswanth 7 years ago.

Download all attachments as: .zip

Change History (21)

comment:1 Changed 8 years ago by Jakub Ś

Description: modified (diff)
Keywords: IE added
Status: newconfirmed
Version: 3.6.23.0

Issue has been reproducible in IE6-8 from CKEditor 3.0. IE9 seems to be working fine.

NOTE: In CKE 3.0 table is created with empty paragraph above so you have to go to source, remove it manually and than continue with TC steps.

comment:2 Changed 8 years ago by naresh.sivaraman

Do you have any updates on this ticket?

comment:3 Changed 8 years ago by Tetsuro Wakashima

It would be really helpful if you can give us an update on this. My customer has been waiting for the fix for months. Would you please at least let us know a tentative time by which this will be fixed.

comment:4 Changed 7 years ago by Tetsuro Wakashima

I need to inform my customer if there would be still possibility of fixing this issue in the future or not.

I would appreciate if you could update that it would be feasible to fix this before too long. Thanks.

comment:5 Changed 7 years ago by Frederico Caldeira Knabben

Component: GeneralCore : Pasting
Summary: AFTER COPYING A TABLE AND CLICKING SOURCE , A LINE BREAK IS INSERTEDIE: When copying and pasting a table, an empty <p> is appended on output

This is a bug with IE8 and previous versions of it. It makes it possible to paste a table inside the <p> element, which is not acceptable. This has been fixed with IE9.

I doubt something can be done to workaround this issue, especially considering that this doesn't impact IE9.

I'm leaving this ticket opened because it is still a valid issue that may be reconsidered in the future, if we'll be refactoring things related to clipboard operations.

comment:6 Changed 7 years ago by Yaswanth

I've suggested the below workarounds \ fix to the Customer:

1)Configuring config.enterMode to either CKEDITOR.ENTER_BR or CKEDITOR.ENTER_DIV so that no <p> are created.

2)In onSelectionChangeFixBody function, suppressing the creation of new paragraph. This is just a hack to avoid the creation of new paragraphs on selection. User would still be able to create a new paragraph on pressing enter. Issue may not be completely fixed with this. Patch Attached.

3)In onInsertHtml function, If we are in an empty block then replacing it with an inserting block. I see that this is how it has been handled in onInsertElement (For example while creating a table in an empty block from dialog, empty block is replaced with table instead of placing table inside the empty block). I moved the bits of code from onInsertElement method which seems to be doing this and used the same in onInsertHTML and it seems to be fixing the problem (at least to the level I have tested). I think this would be the ideal fix for this and i am still exploring this fix. It would be helpful and speedy if you can help us out at this. Patch Attached.

Please let me know your thoughts on the above approaches. Thanks

Changed 7 years ago by Yaswanth

Attachment: Workaround.patch added

Workaround 2.

Changed 7 years ago by Yaswanth

Attachment: Fix_working_copy.patch added

Workaround 3_Working copy of Fix.

comment:7 Changed 7 years ago by Yaswanth

Cc: mail@… added

comment:8 in reply to:  6 Changed 7 years ago by Frederico Caldeira Knabben

Replying to yaswanth.ravella:

I can confirm you that your option 3 is the way to go if we want to find any kind of workaround for it.

A bigger analysis would be needed to come with a final solution (which could be even different from the above). So, to speed up thing at your side, I would recommend you going ahead with your patch, well testing it on different kinds of situations.

As the way you have implemented, I don't see much problems if you're dealing with a single element to be paste. Please be sure to test well the behavior of it when passing more complex HTML to it, containing several elements at the same level (like <p>a</p><p>b</p>).

comment:9 Changed 7 years ago by Yaswanth

Hi Fred,

Thanks for your response. We have tested this fix further and it doesn't seem to be working when using Copy and Paste buttons from the UI. It is working fine with Ctrl+C and Ctrl+V though.

I've tried a different approach to fix the original issue by creating an Element from the HTML data from insertHTML and firing the insertElement. It seems to be working fine. I've tested this with Paste, Smiley & Insert_Special_Charecter actions and it doesn't seem to be causing any issues.

I am uploading the patch here for your review. Can you please let me know if this patch would cause any regressions.

Changed 7 years ago by Yaswanth

Workaround 3_Working copy of Fix_Updated.

comment:10 Changed 7 years ago by Frederico Caldeira Knabben

The proposed patch will not work in these two cases:

  • The data to be inserted is plain text.
  • The data to be inserted is more complex than a single HTML element, like <p>A</p><p>B</p>.

comment:11 Changed 7 years ago by Frederico Caldeira Knabben

Keywords: Oracle HasPatch added

comment:12 Changed 7 years ago by Yaswanth

Hi Fred,

Thanks for pointing out the issues with the previous patch. I have tested the scenarios you have mentioned and I was able to reproduce them.

How about firing insertElement only when the paste data is not a Text Node and is a Block else do the normal execution. Also paste data is included inside a div before invoking insertElement. I have tested this approach and it seems to me solving the original issue and also not causing the issues faced with previous 2 patches. This patch is w.r.t the original CKEditor code.

I am uploading the patch here for your review. Can you please let us know if this fix might cause any regressions ?

Changed 7 years ago by Yaswanth

Attachment: workaround.patch added

comment:13 Changed 7 years ago by Frederico Caldeira Knabben

The problem with the latest patch is this call:

tempData = CKEDITOR.dom.element.createFromHtml(tempData)

Just like said in comment:10, this will not work if you have two block elements being inserted (e.g <p>A</p><p>B</p>). In that case tempData will be only (<p>A</p>).

So, you need to find out a way to check if there is just one element being passed at the root of the data.

comment:14 Changed 7 years ago by Yaswanth

Hi Fred,

Thanks for the response. Yes, tempData will only be (<p>A</p>) but when insertElement is fired we will be passing the div with complete html (evt.data) inserted into it.

this.fire( 'insertElement', CKEDITOR.dom.element.createFromHtml('<div>'+evt.data+'</div>') );  

So insertElement will be fired with element (<div><p>A</p><p>B</p></div>) and both the paragraphs will appear in the editor. Please correct if I am wrong or missing something.

Last edited 7 years ago by Yaswanth (previous) (diff)

comment:15 Changed 7 years ago by Frederico Caldeira Knabben

Oh, that's true... that would be ok as long as you're ok to leave with these extra divs being inserted around every time a block needs to be inserted into the document. In that case, your patch sounds reasonable.

comment:16 Changed 7 years ago by Yaswanth

Thanks Fred. I think we will go ahead with this patch for now and will pickup the patch again if CKEditor team comes up with a final solution.

comment:17 Changed 7 years ago by Frederico Caldeira Knabben

We're reworking the clipboard features for CKEditor 4 and there is a good chance that this one gonna get fixed with it.

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