Opened 7 years ago

Closed 6 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 6 years ago.
Fix for Firefox&IE
7366_2.patch (2.1 KB) - added by Garry Yao 6 years ago.
7366_3.patch (5.8 KB) - added by Piotrek Koszuliński 6 years ago.
7366_4.patch (1.5 KB) - added by Garry Yao 6 years ago.
7366_5.patch (2.2 KB) - added by Garry Yao 6 years ago.
7366_6.patch (2.6 KB) - added by Garry Yao 6 years ago.

Download all attachments as: .zip

Change History (34)

comment:1 Changed 7 years ago by Anna Tomanek

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

comment:2 Changed 7 years ago by Satya Minnekanti

Cc: satya_minnekanti@… added

comment:3 Changed 7 years ago by Wiktor Walc

Looks similar to #7304

comment:4 Changed 7 years ago by Garry Yao

Status: confirmedpending

Can you verify the bug is fixed on trunk?

comment:5 Changed 7 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 7 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 6 years ago by Garry Yao

Link 2.x bug #756.

comment:8 Changed 6 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 6 years ago by Piotrek Koszuliński

Attachment: 7366.patch added

Fix for Firefox&IE

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

Owner: set to Piotrek Koszuliński
Status: confirmedreview

Changed 6 years ago by Garry Yao

Attachment: 7366_2.patch added

comment:10 Changed 6 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 6 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 6 years ago by Piotrek Koszuliński (previous) (diff)

Changed 6 years ago by Piotrek Koszuliński

Attachment: 7366_3.patch added

comment:12 Changed 6 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 6 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 6 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 6 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 6 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 6 years ago by Garry Yao

Attachment: 7366_4.patch added

comment:17 Changed 6 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 6 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 6 years ago by spoutnik

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

comment:20 Changed 6 years ago by Garry Yao

Status: reviewreview_failed

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

Changed 6 years ago by Garry Yao

Attachment: 7366_5.patch added

comment:21 Changed 6 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 6 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 6 years ago by Piotrek Koszuliński (previous) (diff)

comment:23 Changed 6 years ago by Jakub Ś

#8867 was marked as duplicate

comment:24 Changed 6 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 6 years ago by Frederico Caldeira Knabben

Status: reviewreview_failed

Changed 6 years ago by Garry Yao

Attachment: 7366_6.patch added

comment:27 Changed 6 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 6 years ago by Frederico Caldeira Knabben

Status: reviewreview_passed

comment:29 Changed 6 years ago by Garry Yao

Resolution: fixed
Status: review_passedclosed

Fixed with [7453].

Last edited 6 years ago by Garry Yao (previous) (diff)
Note: See TracTickets for help on using tickets.
© 2003 – 2017 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy