Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Workaround for Edge not supporting custom MIME types in dataTransfer.setData #962

Closed
f1ames opened this issue Sep 26, 2017 · 6 comments
Closed
Assignees
Labels
browser:edge The issue can only be reproduced in the Edge (edgeHTML engine based) browser. changelog:skip A changelog entry should not be added for a given issue. plugin:clipboard The plugin which probably causes the issue. status:confirmed An issue confirmed by the development team. type:task Any other issue (refactoring, typo fix, etc).
Milestone

Comments

@f1ames
Copy link
Contributor

f1ames commented Sep 26, 2017

Are you reporting a feature request or a bug?

Task

Provide detailed reproduction steps (if any)

Extracted from #468.
Upstream issue https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/8007622.

The Edge browser still does not support custom MIME types in dataTransfer.setData. This are used in clipboard plugin for storing dataTransfer id (which is used to much dataTransfer objects between events).

The proposal is to apply a workaround which will save custom MIME types in text/html MIME type as an HTML comment like:

<!--cke-data:DATA-->

Where DATA is stringifed and encoded JSON object.

This will happen dynamically as Edge throws an error/exception when custom MIME types is set. The exception will be caught and fallback used.

@f1ames f1ames added browser:edge The issue can only be reproduced in the Edge (edgeHTML engine based) browser. plugin:clipboard The plugin which probably causes the issue. status:confirmed An issue confirmed by the development team. type:task Any other issue (refactoring, typo fix, etc). labels Sep 26, 2017
@f1ames f1ames self-assigned this Sep 26, 2017
@f1ames
Copy link
Contributor Author

f1ames commented Sep 28, 2017

There is one specific issue I stumbled upon when working on the fix:

  1. Go to http://tests.ckeditor.dev/tests/plugins/clipboard/manual/buttons.
  2. Perform copy and paste (as in scenario).

sep-28-2017 11-55-42

OpenClipboard Failed error is thrown (in this line - https://github.com/ckeditor/ckeditor-dev/blob/8a96a9adfc0af653538e32f455a5be0b20b2c3db/plugins/clipboard/plugin.js#L2089). Looks like some Edge issue with synchronous access to clipboard on paste. Haven't found any reports on https://developer.microsoft.com/en-us/microsoft-edge/platform/issues, but clearly looks like native issue.


I wasn't able to reproduce this issue with plain contenteditable, however OpenClipboard Failed error seems to be caused by two or more applications accessing the clipboard at the same time (not sure why it happens here still)... Simply changing the code to access clipboardData only once solves the issue - see 2347c18.

@f1ames
Copy link
Contributor Author

f1ames commented Oct 2, 2017

There is one more issue worth mentioning here. While the code setting a cke/id mime type is executed in all normal flows (copy/paste, drag/drop), the id is not set in native dataTransfer object for external transfers (browsers with CKEDITOR.plugins.clipboard.isCustomDataTypesSupported = true).

It happens due to a fact that for such transfers the CKEDITOR.plugins.clipboard.dataTransfer object is created on paste or drop event and there setting or editing native dataTransfer mime types has no effect (see this demo) as described in the specification. The cke/id is however stored locally in CKEDITOR.plugins.clipboard.dataTransfer object which is fine as it does not have to be passed to any further events.

However, the paste behaviour in Edge browser is quite different which causes an issue here:

  • The drop behaves the same as in other browsers so setting/editing mime types on drop has no effect on native dataTransfer.
  • The paste behaviour allows to set/edit mime types, however when setting new mime type all others are cleared (making them empty) - this causes an issue when plain text (text/plain mime type) is copied and on paste, the cke/id comment is set as text/html which clears text/plain mime types contents.

The solution might be resetting all available mime types (so nativeDataTransfer.setData( type, nativeDataTransfer.getData( type ) ) ) as it solves the problem with resetting mime types.

@f1ames
Copy link
Contributor Author

f1ames commented Oct 2, 2017

Reported issue with dataTransfer store not in readonly mode on paste here: https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/14038089/.

@mlewand
Copy link
Contributor

mlewand commented Oct 2, 2017

The solution might be resetting all available mime types (so nativeDataTransfer.setData( type, nativeDataTransfer.getData( type ) ) ) as it solves the problem with resetting mime types.

This is just a hack relying on current browser implementation bug.

Instead the problem here is that clipboard api tries to overwrite datatransfer in paste/dragend events, even with seeing that it's not a proper way.

The solution here would be to fix the source of it, and call (our) dataTransfer.setData() with clipboardIdDataType only in proper events. Seems like initPasteDataTransfer and initDragDataTransfer are the best candidate for that.

@f1ames
Copy link
Contributor Author

f1ames commented Oct 4, 2017

OpenClipboard Failed error issues reported to upstream: https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/14073387/.

@f1ames
Copy link
Contributor Author

f1ames commented Dec 1, 2017

This was closed with its parent task #468 in #1153.

@f1ames f1ames closed this as completed Dec 1, 2017
@mlewand mlewand added this to the 4.8.0 milestone Dec 1, 2017
@mlewand mlewand added the changelog:skip A changelog entry should not be added for a given issue. label Dec 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
browser:edge The issue can only be reproduced in the Edge (edgeHTML engine based) browser. changelog:skip A changelog entry should not be added for a given issue. plugin:clipboard The plugin which probably causes the issue. status:confirmed An issue confirmed by the development team. type:task Any other issue (refactoring, typo fix, etc).
Projects
None yet
Development

No branches or pull requests

2 participants