[Edge] Nothing is pasted when text + images selected

[Reinmar]

It's strange because it soooometimes work (content appears, although images have local src, but that's a known Edge problem).

If we can't handle images in this case I'd expect that at least text is pasted.

[Reinmar]

1 minute later this appeared:

It's weird... Perhaps this is only a performance issue on the VM we use for tests. @Mgsy could you test it on a real machine?

[f1ames]

It is quite interesting what happens here and the fact that it works correctly in other browsers. I recall I have a thought about that when working on integration with image upload but it slipped away somewhere along the process...

1. When content is pasted from Word first it goes through PFO inputTransformation listener:

https://github.com/ckeditor/ckeditor5-paste-from-office/blob/87f7e7c3756c2404a4cb8e10019ab370a44d8e93/src/pastefromoffice.js#L41-L47

it fetches text/html from dataTransfer object, transforms it and sets as event data.content.

2. Then the event with data is passed along the pipeline until the listener in ImageUploadEditing class is called:

https://github.com/ckeditor/ckeditor5-image/blob/14dca372ab3e57e8aa4a8eead5d55fffe3143d5c/src/imageupload/imageuploadediting.js#L97-L137

it tries to fetch images (what happens here doesn't matter in this case) and edits the event data.content (which represents view) accordingly. As the fetching process is asynchronous the inputTransformation event is stopped and then re-fired (with modified data.content).

3. When the inputTransformation event is re-fired it goes through all listeners again. So we go back to 1. point - as PFO inputTransformation listener uses content directly from dataTransfer, changes in event data.contnet done by ImageUploadEditing doesn't matter here.

Why it doesn't work on Edge and works in other browsers?

That's the crucial part. According to specs (AFAIR), access to dataTransfer data must be synchronous. So when inputTransformation event is re-fired after async processing with dataTransfer object, PFO listener gets empty content when calling data.dataTransfer.getData( 'text/html' ) and whole processing is skipped. However, in Edge it seems you can access dataTransfer asynchronously thus whole processing starts all over causing an infinite loop.

[f1ames]

☝️ I think it shows some conceptual problems with clipboard pipeline. As at some point we have decided that inputTransformation event should be emitted with both data.content and data.dataTransfer it means some listeners may use content and others dataTransfer to process data which may lead to similar situations and data desynchronisation.

[Reinmar]

Isn't the biggest problem here that we re-fire the inputTransformation event? If not that (if for some reason we would not need to stop and refire it), there would be no problem, right?

[f1ames]

Isn't the biggest problem here that we re-fire the inputTransformation event? If not that (if for some reason we would not need to stop and refire it), there would be no problem, right?

Yes, that's the issue here.

[f1ames]

☝️ I even considered if this can be done without stopping inputTransformation event - ckeditor/ckeditor5-image#248 (comment):

1. Listen to inputTransformation event.
2. Find images with local sources in clipboard data.
3. Modify images in clipboard data so base64/blob is not propagated to model.
4. Fetch images (async) and start upload.

So in the second variant, the inputTransformation event is not paused/delayed by fetching images. It means the pasted content will appear faster in the editor. However, as images are not ready the content will appear without image previews. Preview images will appear when fetched which will result in resizing/moving some parts of the content. This is just an alternative which I was thinking of, not sure if it brings any real improvement, but wanted to mention it.

[Reinmar]

You're right that the conceptual issue is with the clipboard pipeline. It assumes synchronous processing for two reasons:

• as you wrote, dataTransfer is not available in the next tick of the event loop anyway,
• our event system did not support asynchronousity when we've been implementing the clipboard pipeline; we knew it may become a problem one day, but we didn't want to spend additional time back then on this.

And those problems appeared now :) In fact, a couple of them:

• Where to access dataTransfer to make sure it's only accessible (and accessed) synchronously? Should that be only the clipboardInput (cause it's synchronous by definition)? Or should it be part of inputTransformation (whatever it will be) but only up to a point when it's synchronous? How not to make this confusing?
• How should an asynchronous pipeline look? Would it still be an event system-based? Or perhaps we should introduce a new mechanism? Should we make it possible to add new callbacks before/after some existing ones? In other words – how to solve the issue of multiple independent features trying to figure out the order in which their callbacks are registered.
• How to account for asynchronous clipboard access that we'll have in the future?

I think we can start working on this problem. But I'd start from finding a hack which will solve the Edge problem (e.g. a flag which will prevent the second inputTransformation to override what the previous did). Also, since dataTransfer will not work on the refired inputTransformation, I think it'd be good to remove it from the API docs. Let it be our secret for now.

[Reinmar]

We talked with @f1ames F2F about https://github.com/ckeditor/ckeditor5-paste-from-office/issues/44#issuecomment-443168832.

We realised that this code:

https://github.com/ckeditor/ckeditor5-image/blob/14dca372ab3e57e8aa4a8eead5d55fffe3143d5c/src/imageupload/imageuploadediting.js#L101-L137

does not have to stop the event. It can create loader instances synchronously and set the uploadId attribute synchronously. However, it does not have file instances synchronously – that must remain asynchronous. Which means that we need to be able to create loader instances like this:

const loader = fileRepository.createLoader( new Promise( resolve => {
    resolve( file );
} ) );

loader.file; // -> null
loader.status; // -> 'uninitialized'
loader.read(); // errror!
loader.id; // -> 'idoftheloader'

// promise gets resolved by the `ImageUploadEngine` once `file` instance is returned by the native File API or whatever else...

loader.file // -> file instance
loader.status; // -> 'idle'
loader.id; // -> 'idoftheloader'

It's important that loader.id is accessible from the first moment (synchronously) so the main logic of that listener can stay synchronous.

cc @pjasiun

[Reinmar]

But for now we implement a workaround in ckeditor/ckeditor5-paste-from-office#47 and https://github.com/ckeditor/ckeditor5-image/pull/257/files.

[Reinmar]

The workaround is merged. That saves us some time.

[f1ames]

I have extracted the issue about async use of file loader to https://github.com/ckeditor/ckeditor5-upload/issues/87. Any further discussion could take place there if needed. cc @Reinmar @pjasiun