Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#12961 closed Bug (fixed)

[Chrome] Pasting copied images into chrome from paint or "printscreen" should be supported

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

Description

Pasting images into CKEditor should support pasting copied images from MS Paint or from "PrintScreen" into Chrome as it does in IE11 and Firefox.

Comment #7 here seems to indicate that it is not possible for this to work for Blink browsers. http://dev.ckeditor.com/ticket/11526#comment:7

However, this particular functionality is supported by Chrome.

I was able to verify this by using the following code in the paste event on a simple contenteditable div:

var items = (event.clipboardData || event.originalEvent.clipboardData).items;
var blob = items[0].getAsFile();
var reader = new FileReader();
reader.onload = Function.createDelegate(this, function (event)
{
    var image = document.createElement("img");
    image.src = event.target.result;
    this.Container.appendChild(image);
}); 
reader.readAsDataURL(blob);

The gotcha is that clipboardData.items is cleared in Chrome at the end of the execution of the browser’s paste event. If you try to access the items collection outside of the paste event (after a setTimeout is called), the items collection has been cleared and is no longer accessible. In my testing items[0].getAsFile() needs to be called before the paste handler execution completes.

Change History (15)

comment:1 Changed 4 years ago by Jonathan

Keywords: paste image chrome removed
Type: New FeatureBug

comment:2 Changed 4 years ago by Piotr Jasiun

Owner: set to Piotr Jasiun
Status: newassigned
Summary: Pasting copied images into chrome from paint or "printscreen" should be supported[Chrome] Pasting copied images into chrome from paint or "printscreen" should be supported

comment:3 Changed 4 years ago by Piotr Jasiun

Milestone: CKEditor 4.5.0
Version: 4.5.0 (GitHub - major)

comment:4 Changed 4 years ago by Piotr Jasiun

Status: assignedreview

Changes done, in t/12961.

I wanted to report this on the Chromium bug tracker, but apparently it's not a bug, it's feature. According to this discussion image copied from MS Paint or from "PrintScreen" is not, in fact, a real file, so it does not land in the list of files. It makes some sense.

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

Status: reviewreview_failed

The branch wasn't pushed.

comment:6 Changed 4 years ago by Piotr Jasiun

Status: review_failedreview

My bad. I called the branch t/12691. Fixed.

comment:7 Changed 4 years ago by Piotrek Koszuliński

What if there are many items? Items are kept in an array, so it means that someone was expecting that it may happen. E.g. some program may allow to copy multiple "graphics items", just like it's possible in a file browser.

As far as I can see this is implemented by a private method which is then used while caching data. So if such situation will happen in the future, we will be able to change this without touching private API.

What worries me though is that someone may want to access items specifically or only files. What we do (merging files with items which are files) seems to be useful on most occasions, but I wonder if it won't create some problems. I think that it should be ok, because as a last resort one can access the native clipboard directly and get only what she/he wants. WDYT?

comment:8 in reply to:  7 Changed 4 years ago by Piotr Jasiun

I was also thinking about these limitation, but then I realized what is DataTransfer facade and why it is introduced. It is created to hide implementation difference in browsers and provide common, simple API, similar to the old clipboard API (so getData and files, no items, which are new, not yet native supported API). Most browsers put the clipboard image in the files so this is how facade should behave. Also if any program will give multiple image items we will not be able to guess witch one is correct anyway. For example it could be Photoshop layers and pasting all of then might not be a expected behavior.

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

Status: reviewreview_failed

comment:10 Changed 4 years ago by Piotrek Koszuliński

Besides this red test and #13029 everything works fine.

comment:11 Changed 4 years ago by Piotr Jasiun

Status: review_failedreview

I fixed the red test (actually it was not related race condition) and #13029 is unrelated, occur on major too.

I also find the issue with the file in both files and items (this is what Chrome do), added tests and fixed issue.

Changes in t/12961.

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

Status: reviewreview_failed

comment:13 Changed 4 years ago by Piotr Jasiun

Status: review_failedreview

Check it now. It still not related to this ticket and I am still not able to reproduce this bug, but this fix should help.

comment:14 Changed 4 years ago by Piotrek Koszuliński

Resolution: fixed
Status: reviewclosed

Fixed on major with git:910de7a.

comment:15 Changed 4 years ago by Jonathan

Thanks for fixing this!

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