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

CKEditor erroneously perceives internal copy & paste as type "external" #921

Closed
mmichaelis opened this issue Sep 15, 2017 · 12 comments
Closed
Labels
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. target:major Any docs related issue that should be merged into a major branch. type:bug A bug.
Milestone

Comments

@mmichaelis
Copy link

mmichaelis commented Sep 15, 2017

Are you reporting a feature request or a bug?

Bug

Check if the issue is already reported

Provide detailed reproduction steps (if any)

  1. Open https://ckeditor.com/ in Edge
  2. Execute the following JavaScript:
cke=CKEDITOR.instances[Object.keys(CKEDITOR.instances)[0]]
cke.on('paste',function(e){console.log("paste/0, internal=1, cross-editor=2, external=3 => ", e.data.dataTransfer.getTransferType(e.editor))}, null, null, 0)
  1. Do copy & paste within the CKEditor (Ctrl+C + Ctrl+V)

Expected result

Log-Output states: Internal paste.

Actual result

Log-Output states: External paste.

The overall result is bad, as the content processing differs for internal and external pastes. In our environment it even leads to the pasteFromWord plugin being triggered (cannot reproduce on ckeditor.com though).

In Chrome the above code correctly returns '1' for internal paste. Analyzing the event's editor property and the dataTransfer's sourceEditor property you will see that on Chrome both are the same (thus internal c&p) while in Edge the event's editor is correct, while sourceEditor is undefined.

Reason for this is that currently Edge does not support Clipboard API, where CKEditor4 clipboard API stores the source editor id as a custom mime type.

Other details

  • Browser: Microsoft Edge 40.15063.0.0
  • OS: Windows 10
  • CKEditor version: 4.7.3
@mmichaelis
Copy link
Author

With a little more debugging the problem seems to be, that method addPasteListenerstoEditable (clipboard/plugin.js) only applies listeners to the editable when isCustomCopyCutSupported is true. And this is the only place where a valid sourceEditor is set on editable's copy and cut command.

As it seems isCustomCopyCutSupported seems to be false for MSIE11... thus the sourceEditor cannot be determined on copy or cut. So if this is not possible, can we guess the sourceEditor in some way?

As the information of the transfer type is also used for drag & drop it might be possible that we detect another symptom of this defect: In our editor drag & drop within MSIE11 is not possible at all (even internally).

@mmichaelis
Copy link
Author

If there is no way to determine the sourceEditor what about introducing an additional state for the transfer type: unknown? I guess that in case of unknown plugins might be better off assuming a similar state as to internal.

@mlewand mlewand added plugin:clipboard The plugin which probably causes the issue. status:confirmed An issue confirmed by the development team. type:bug A bug. labels Sep 19, 2017
@mlewand
Copy link
Contributor

mlewand commented Sep 19, 2017

Thanks for investigation. It looks that, as you have provided, the source editor is not defined. AFAICS reason for it is that it's being passed in dataTransfer object, and IE do not use dataTransfer at all. It would be better to fix that problem at all, same thing might happen on iOS.

@mlewand mlewand added the browser:ie The issue can only be reproduced in the Internet Explorer browser. label Sep 19, 2017
@mlewand
Copy link
Contributor

mlewand commented Sep 25, 2017

Referenced old #12873 issue. This issue should be fixed in #468 (which we're actively working on) for Microsoft Edge, however IE11 will not benefit from it as it's no longer supported browser by Microsoft.

And fix for this issue could be rather complex.

@mmichaelis
Copy link
Author

Thanks for the analysis. Then I'd suggest to close this issue either as won't fix — or to consider to introduce a workaround, i. e. a possibility to hook into the consideration if this is an internal or external paste. I suppose that, knowing our product, we could use additional strategies to distinguish internal from external paste.

@mmichaelis
Copy link
Author

Just re-validated for a supported Browser: The same issue occurs in Edge. Emulation settings in Edge: Browser Profile: Desktop, User Agent String: Microsoft Edge (Default).

Chrome and Firefox correctly answer the paste-listener above with "internal".

@mmichaelis
Copy link
Author

Regarding MSIE11: Can you give a reference for the end of support for MSIE11? All I see that all older MSIE browsers are EOL as of January 2016.

References:

@f1ames
Copy link
Contributor

f1ames commented Sep 27, 2017

Hello @mmichaelis,
from https://developer.microsoft.com/en-us/microsoft-edge/platform/faq/:

Will Internet Explorer 11 continue to receive updates?
The latest features and platform updates will only be available in Microsoft Edge. We will continue to deliver security updates to Internet Explorer 11 through its supported lifespan. To ensure consistent behavior across Windows versions, we will evaluate Internet Explorer 11 bugs for servicing on a case by case basis.

Which means some bug fixes may be provided, however dataTransfer support is quite a big feature so it is extremely unlikely it will get fixed/introduced in IE11 (especially that dataTransfer is not yet fully supported in the latest Edge preview).

@mmichaelis
Copy link
Author

Hello @f1ames,
thanks for the reference. Your report about the dataTransfer state also matches my observation that the problem also occurs in Edge. So any idea how to provide workarounds especially for those application paths which choose different behaviors for different transfer types (like filtering class attributes for external type)?
Is it feasible in any way for example to introduce a type unknown and provide a CKEditor setting to customize of unknown should better go the internal, cross-editor or external path?

@f1ames
Copy link
Contributor

f1ames commented Sep 29, 2017

@mmichaelis, as mentioned before for Edge browser it will be fixed in #468. As for IE11 I don't see any reasonable improvement. As the browser makes it nearly impossible to accurately distinguish external copy/paste from internal/cross-editor there is no point in introducing unknown type as it will mean exactly the same as external.

In your case if you would like to apply some additional checks you may listen to paste event to do some additional checks (really depends on solution you have in mind). However, this will require to create some custom code/solution which is not a part of this issue.

@f1ames f1ames changed the title MSIE11: CKEditor erroneously perceives internal copy & paste as type "external" Edge/IE11: CKEditor erroneously perceives internal copy & paste as type "external" Sep 29, 2017
@f1ames f1ames added the browser:edge The issue can only be reproduced in the Edge (edgeHTML engine based) browser. label Sep 29, 2017
@mlewand
Copy link
Contributor

mlewand commented Sep 29, 2017

For IE11 there's nothing we can do to support that. Thus I'll change this issue to track Edge only.

@mmichaelis If you're willing to go great lengths to adjust it somehow for IE11 on your setup, your best place is to customize dataTransfer.getTransferType. You'll need simply to override CKEDITOR.plugins.clipboard.dataTransfer.prototype.getTransferType.

Just watch out, depending whether you're working with a built CKE version it might be available synchronously, or asynchronously after the clipboard plugin is loaded.

@mlewand mlewand changed the title Edge/IE11: CKEditor erroneously perceives internal copy & paste as type "external" CKEditor erroneously perceives internal copy & paste as type "external" Sep 29, 2017
@mlewand mlewand removed the browser:ie The issue can only be reproduced in the Internet Explorer browser. label Sep 29, 2017
@mlewand mlewand added the target:major Any docs related issue that should be merged into a major branch. label Nov 21, 2017
@mlewand mlewand added this to the 4.8.0 milestone Nov 21, 2017
@mlewand
Copy link
Contributor

mlewand commented Nov 21, 2017

Fixed in latest Edge by #1153. Why did GH not close it automatically... 🤔

@mlewand mlewand closed this as completed Nov 21, 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. plugin:clipboard The plugin which probably causes the issue. status:confirmed An issue confirmed by the development team. target:major Any docs related issue that should be merged into a major branch. type:bug A bug.
Projects
None yet
Development

No branches or pull requests

3 participants