Opened 13 years ago

Closed 12 years ago

#7366 closed Bug (fixed)

Pasting from browser toolbar menu bypasses paste filters

Reported by: Arne Owned by: Garry Yao
Priority: Normal Milestone: CKEditor 3.6.3
Component: Core : Pasting Version: 3.0
Keywords: Cc: satya_minnekanti@…

Description (last modified by Anna Tomanek)

  1. Go to demo page
  1. Paste some content (i.e. from MS Word) by choosing 'Edit' -> 'Paste' from the browser toolbar.
  1. Switch to source mode and observe the garbage HTML.

Tested in WinXP (FF 3.6.15, IE8)

Seems nothing is listening to the document paste event.

Attachments (6)

7366.patch (5.7 KB) - added by Piotrek Koszuliński 12 years ago.
Fix for Firefox&IE
7366_2.patch (2.1 KB) - added by Garry Yao 12 years ago.
7366_3.patch (5.8 KB) - added by Piotrek Koszuliński 12 years ago.
7366_4.patch (1.5 KB) - added by Garry Yao 12 years ago.
7366_5.patch (2.2 KB) - added by Garry Yao 12 years ago.
7366_6.patch (2.6 KB) - added by Garry Yao 12 years ago.

Download all attachments as: .zip

Change History (34)

comment:1 Changed 13 years ago by Anna Tomanek

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

comment:2 Changed 13 years ago by Satya Minnekanti

Cc: satya_minnekanti@… added

comment:3 Changed 13 years ago by Wiktor Walc

Looks similar to #7304

comment:4 Changed 13 years ago by Garry Yao

Status: confirmedpending

Can you verify the bug is fixed on trunk?

comment:5 Changed 13 years ago by Arne

Just pointing out that this relates to using the browser toolbar, not the editor toolbar, so in that it differs from #7304 it seems.

comment:6 Changed 13 years ago by Wiktor Walc

Status: pendingconfirmed
Version: 3.0

Indeed, using "Edit -> Paste" from the browser toolbar makes the difference here. Confirmed in IE8 / Win 7.

comment:7 Changed 13 years ago by Garry Yao

Link 2.x bug #756.

comment:8 Changed 12 years ago by Jakub Ś

Looks to me like quite a critical issue.

There are 3 ways to bypass filters.

  1. Using "Edit -> Paste"
  2. Using Native browser Menu - just use CRTL + Right-click.
  3. Using Drag and Drop from word directly to CKEditor.

All of those cases produce garbage HTML.

Issue has been reproducible in all browsers from CKEditor 3.0.

Changed 12 years ago by Piotrek Koszuliński

Attachment: 7366.patch added

Fix for Firefox&IE

comment:9 Changed 12 years ago by Piotrek Koszuliński

Owner: set to Piotrek Koszuliński
Status: confirmedreview

Changed 12 years ago by Garry Yao

Attachment: 7366_2.patch added

comment:10 Changed 12 years ago by Garry Yao

Owner: changed from Piotrek Koszuliński to Garry Yao

7366.patch fails the following case:

  1. Click "Paste" from browser toolbar menu;
  2. On clipboard security popup choose "Allow";
    • Actual: Paste dialog opened;
    • Expected: Pasted succeed without dialog;

Besides, the patch is over-complicated, considering all we need is to invoke the editor paste command, providing a simplified one.

comment:11 Changed 12 years ago by Piotrek Koszuliński

Patch 7366_2.patch doesn't work for me (tested on IE8 & Fx):

  1. For quickly repeated ctrl+v IE throws security alert.
  2. For native context menu and native menu bar it doesn't filter MSWord stuff.
  3. Additionally - line endings differ from these in original plugin.js, so I had a problem with applying patch on Linux.

The way I'm testing (for v4 we've got playground and there it's easily visible if pasted content was handled by us, but for v3 we don't have it):

  1. Set editor data to: <p>This is more <strong class="MsoNormal">sample text</strong>.</p>
  2. Copy this in wysiwyg mode.
  3. Paste into the editor.
  4. Check in source mode if MsoNormal class was removed by filter.
Last edited 12 years ago by Piotrek Koszuliński (previous) (diff)

Changed 12 years ago by Piotrek Koszuliński

Attachment: 7366_3.patch added

comment:12 Changed 12 years ago by Piotrek Koszuliński

7366.patch works for me, but it was missing one thing. For declined security alert opened by pasting from native context menu or menu bar it wasn't opening paste dialog. This is fixed by 7366_3.patch.

However I've checked now that this patch doesn't work with latest trunk version for pasting from menu bar. It throws some error related probably to dialog.

But with revision 7364 it works perfectly fine - all features are ok.

comment:13 Changed 12 years ago by Garry Yao

Status: reviewassigned

The trunk is right now unstable to review this ticket, #8843 and #8849 need to be reviewed before hand.

comment:14 Changed 12 years ago by Garry Yao

But anyway 7366_3 is still too drastic a change for v3, since it changes the paste event used for other browsers as well, my intention is to provide a dedicated patch only for the toolbar menu paste issue, so a simplified patch must be considered.

comment:15 Changed 12 years ago by Piotrek Koszuliński

I believe there's another solution for all clipboard's problem. But I'm not sure it will be any simpler. The problem is caused by the fact that all browsers behave differently when they fire (before)paste events.

Anyway, I'm keeping finger crossed for you :)

comment:16 Changed 12 years ago by Garry Yao

Status: assignedreview

Ok, I come with a latest patch that fixes the above tcs while it need to be reviewed along with changes proposed in #8843 and #8849 (in order to have correct cursor positon).

Changed 12 years ago by Garry Yao

Attachment: 7366_4.patch added

comment:17 Changed 12 years ago by Piotrek Koszuliński

I've checked 7366_4.patch and it seems to work correctly (I wasn't checking against #8843 and #8849). It stripped some features, but not critical ones. And it's definitely shorter :>. Gr8 job.

comment:18 Changed 12 years ago by Garry Yao

Milestone: CKEditor 3.6.3

Since it's almost reviewed, let's consider it for the release.

comment:19 Changed 12 years ago by spoutnik

7366_4 is working like a charm (ubuntu ff11, winxp ff11, winxp ie8, win7 ie9, win7 ff10)

comment:20 Changed 12 years ago by Garry Yao

Status: reviewreview_failed

The patch forces the paste dialog on Fx and Webkit which can be avoided.

Changed 12 years ago by Garry Yao

Attachment: 7366_5.patch added

comment:21 Changed 12 years ago by Garry Yao

Status: review_failedreview

New patch should have exactly the same code path for IE (so it doesn't have to be retested on IE ) while fix the above mentioned issue.

comment:22 Changed 12 years ago by Piotrek Koszuliński

I've found an issue on IE8.

  1. click edit->paste (in native menu bar)
  2. IE throws security alert
  3. accept it
  4. paste dialog will open
  5. click edit->paste again
  6. content will be pasted correctly

The same happens for ckeditor's context menu.

Maybe this is caused by ckeditor's commands which still fires the same native events than before, when now IE listens for different one. Note that in my patch I moved event name to the mainPasteEvent variable.

Last edited 12 years ago by Piotrek Koszuliński (previous) (diff)

comment:23 Changed 12 years ago by Jakub Ś

#8867 was marked as duplicate

comment:24 Changed 12 years ago by Frederico Caldeira Knabben

Other than the above, I've noticed that CTRL+RIGHT-CLICK is not opening the security alert on IE9, which seems to be wrong (considering that we're not able to grab the "paste" event in time with IE).

comment:26 Changed 12 years ago by Frederico Caldeira Knabben

Status: reviewreview_failed

Changed 12 years ago by Garry Yao

Attachment: 7366_6.patch added

comment:27 Changed 12 years ago by Garry Yao

Status: review_failedreview

I'm unable to reproduce the tc in comment 22 with a default IE security configuration.

comment:28 Changed 12 years ago by Frederico Caldeira Knabben

Status: reviewreview_passed

comment:29 Changed 12 years ago by Garry Yao

Resolution: fixed
Status: review_passedclosed

Fixed with [7453].

Last edited 12 years ago by Garry Yao (previous) (diff)
Note: See TracTickets for help on using tickets.
© 2003 – 2022, CKSource sp. z o.o. sp.k. All rights reserved. | Terms of use | Privacy policy