Opened 13 years ago
Last modified 13 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 )
Steps to reproduce:
- Open CKEDITOR.
- Create a table.
- Highlight the table with "Select All" button and click "Copy" icon.
- Click "New Page" icon.
- Click "Paste" icon.
- Click "Source" -> HTML source is displayed, there is "<p> </p>".
- Click "Source" -> HTML page is displayed, line break is inserted before the table.
Bug is present since 3.4.2 i believe.
Attachments (4)
Change History (21)
comment:1 Changed 13 years ago by
Description: | modified (diff) |
---|---|
Keywords: | IE added |
Status: | new → confirmed |
Version: | 3.6.2 → 3.0 |
comment:3 Changed 13 years ago by
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 13 years ago by
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 13 years ago by
Component: | General → Core : Pasting |
---|---|
Summary: | AFTER COPYING A TABLE AND CLICKING SOURCE , A LINE BREAK IS INSERTED → IE: 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 follow-up: 8 Changed 13 years ago by
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
comment:7 Changed 13 years ago by
Cc: | mail@… added |
---|
comment:8 Changed 13 years ago by
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 13 years ago by
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 13 years ago by
Attachment: | Fix_working_copy_Updated.patch added |
---|
Workaround 3_Working copy of Fix_Updated.
comment:10 Changed 13 years ago by
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 13 years ago by
Keywords: | Oracle HasPatch added |
---|
comment:12 Changed 13 years ago by
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 13 years ago by
Attachment: | workaround.patch added |
---|
comment:13 Changed 13 years ago by
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 13 years ago by
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.
comment:15 Changed 13 years ago by
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 13 years ago by
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 13 years ago by
We're reworking the clipboard features for CKEditor 4 and there is a good chance that this one gonna get fixed with it.
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.