Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#12173 closed New Feature (fixed)

Basic paste integration with new drag and drop.

Reported by: Piotr Jasiun Owned by: Piotr Jasiun
Priority: Normal Milestone: CKEditor 4.5.0 Beta
Component: General Version:
Keywords: Cc:

Description (last modified by Jakub Ś)

Part of #11437.

editor.paste event emitted from native paste should contain same data as editor.paste event (e.g. dataTransfer).

There should be paste event listener (with 0 priority) and if it contains dataTransfer with html or text data, such data should be put as a dataValue.

Change History (16)

comment:1 Changed 5 years ago by Jakub Ś

Description: modified (diff)

comment:2 Changed 5 years ago by Jakub Ś

Status: newconfirmed

comment:3 Changed 5 years ago by Piotr Jasiun

Milestone: CKEditor 4.5.0
Owner: set to Piotr Jasiun
Status: confirmedassigned

comment:4 Changed 5 years ago by Piotr Jasiun

Status: assignedreview

Changes:

  • paste event may have no dataValue (html data can be created in the listener),
  • every paste event contains DataTransfer object,
  • every paste event has defined method (paste or drop)
  • if it is possible, data are taken directly from DataTransfer without using paste bin (Safari, Chrome, Firefox),
  • custom cut/copy handling on non-IE browsers,
  • Safari and Chrome show proper transfer type for cut/copy and paste (internal/external/cross editor), on other browsers transfer type is always external,
  • because IE show security dialog when we try to access to window.clipboardData (dataTransfer object) we do not use it there,
  • before paste is fired on drop the same way it works for paste.

Changes in t/12173.

Last edited 5 years ago by Piotr Jasiun (previous) (diff)

comment:5 Changed 5 years ago by Piotr Jasiun

Rebased on newest #12169.

comment:6 Changed 5 years ago by Piotr Jasiun

Rebased on newest #12169.

comment:7 Changed 5 years ago by Piotr Jasiun

Rebased on major.

comment:8 Changed 5 years ago by Piotr Jasiun

More changes:

  • do not use pasteBin on Chrome and Firefox at all,
  • introduced dataTransfer.isEmpty method and do not fire paste if both dataValue and dataTransfer are empty,
  • removed unsupported getClipboardDataDirectly on non-IE browsers,
  • removed targetEditor property from dataTransfer object,
  • added mock methods to bender.tools,
  • modified bender.tools.emulatePaste so it put HTML into dataTransfer on non-IE browsers,
  • added data attribute to the bender.tools.emulatePaste method.

Note that you need to checkout both CKEditor and benderjs-ckeditor repos.

Changes in ckeditor t/12173 and benderjs-ckeditor t/12173​.

comment:9 Changed 5 years ago by Piotr Jasiun

I rebased branch on the newest major and moved changes from benders-ckeditor into ckeditor-dev. Now all changes are in ckeditor t/12173.

comment:10 Changed 5 years ago by Piotr Jasiun

I rebased branch on the newest major.

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

I rebased branch on the newest major.

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

Status: reviewreview_passed

Results of a 3 days review

  • On editable#cut content should be deleted with lower priority (999). Currently it's hard to override cut data, because it's immediately gone.
  • initPasteDataTransfer has no documentation.
  • In initPasteDataTransfer and initDropDataTransfer you expect first argument to be what? Because docs says it's native event, when it's not.
  • I mentioned this during one of previous reviews and I noticed this again - we can't write conditional code based on repeating CKEDITOR.env.ie usage. This makes code hard to read and hard to maintain, because it's unclear what this condition means - why IE, why not other browsers, whether IE12, 13 too or not. You need to create meaningfull variables like dataTransferSetCustomDataSupport and use them. E.g. in pasteDataFromClipboard you created htmlAlwaysInDataTransfer - it's great, just make it global for this plugin and then create more of them.
  • CKEDITOR.DATA_TRANSFER_INTERNAL flags should start from 1 (1, 2, 4).

TC1 (Chrome,FF):

TC2 (IE8):

  • open http://ckeditor.dev/plugins/clipboard/dev/dnd.html
  • copy text from textarea
  • paste it... it's underlined :D
  • it happens only when pasting into the framed editor and it happens also when pasting using button in the toolbar (and allowing clipboard access)
  • and that happens because our pastebin doesn't grab data at all, but editor#paste is fired so... we access clipboard directly :|

What's next?

The code and tests look good, though I have to say that it's impossible to make a full review, because changes are huge.

Anyway, we can't fix all these issue in this ticket because too many depends on it already. I'm closing it for now and I'll report a separate tickets for these issues. These new tickets should be closed as soon as possible.

comment:13 Changed 5 years ago by Piotrek Koszuliński

Merged to major with git:fd2aeb8. Thanks!

I'll close this ticket after I report its follow-ups.

comment:14 Changed 5 years ago by Piotr Jasiun

Resolution: fixed
Status: review_passedclosed

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

Follow-ups reported in #12341.

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