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

Cutting the widget scrolls editor to the end (with t/468) #1160

Closed
f1ames opened this issue Nov 14, 2017 · 5 comments
Closed

Cutting the widget scrolls editor to the end (with t/468) #1160

f1ames opened this issue Nov 14, 2017 · 5 comments
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:bug A bug.

Comments

@f1ames
Copy link
Contributor

f1ames commented Nov 14, 2017

Are you reporting a feature request or a bug?

Bug

Provide detailed reproduction steps (if any)

  1. Go to http://tests.ckeditor.test:1030/tests/plugins/clipboard/manual/draganddrop
  2. Select the widget from the first editor.
  3. Press CTRL + X.

Expected result

Editor scrolls to the end.

Actual result

Editor does not scroll at all or scroll position is restored.

Other details

@f1ames f1ames added 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:bug A bug. labels Nov 14, 2017
@f1ames
Copy link
Contributor Author

f1ames commented Nov 14, 2017

Looks like the breaking commit is the first one: 5571884. Not sure why ATM.

@f1ames
Copy link
Contributor Author

f1ames commented Nov 14, 2017

The exact line is https://github.com/ckeditor/ckeditor-dev/blob/2bcbf1ffd39e4bb897c9c1b1930337214da83136/plugins/clipboard/plugin.js#L526

while the previous flow doesn't executed this code (due to CKEDITOR.plugins.clipboard.isCustomCopyCutSupported) the issue was not visible.


When this line is skipped, on widget cut editor content does not jump (which happens on major) so this is a little different case.

@f1ames
Copy link
Contributor Author

f1ames commented Nov 15, 2017

This issue is caused by the fact that when copying/cutting widgets, pastebin is still used. Widgets copySingleWidget function:

https://github.com/ckeditor/ckeditor-dev/blob/2bcbf1ffd39e4bb897c9c1b1930337214da83136/plugins/widget/plugin.js#L3083-L3169

is called on copy/cut here:
https://github.com/ckeditor/ckeditor-dev/blob/2bcbf1ffd39e4bb897c9c1b1930337214da83136/plugins/widget/plugin.js#L3503-L3520

or here:
https://github.com/ckeditor/ckeditor-dev/blob/2bcbf1ffd39e4bb897c9c1b1930337214da83136/plugins/widget/plugin.js#L2690-L2704

Initially I thought scrolling is caused by container holding real selection, but the cause here is pastebin container.

The quick workaround for the issue is to fix pastebin container type from:

copybinName = ( editor.blockless || CKEDITOR.env.ie ) ? 'span' : 'div'

to:

copybinName = ( editor.blockless || ( CKEDITOR.env.ie && CKEDITOR.env.version < 16 ) ) ? 'span' : 'div'

which brings the same behaviour as on the major branch (the editor jumps - scrolling back to where caret is after widget removal).


Is pastebin really needed here?

The clipboard plugin uses pastebin only for browsers not supporting Clipboard API (IE browsers), but for widgets it is used always. I think we should consider restricting its usage only from browsers which really needs it, especially that:

  1. copySingleWidget is called first and it utilizes CKEDITOR.editable.getHtmlFromRange to prepare HTML which is copied by the browser to a clipboard/dataTransfer.
  2. clipboard plugin listeners are executed, which for browsers supporting Clipboard API, creates CKEDITOR.plugins.clipboard.dataTransfer.
  3. CKEDITOR.plugins.clipboard.dataTransfer object upon creation inserts text/html data to clipboard (using CKEDITOR.editor.getSelectedHtml) overwriting html which was inserted via pastebin.

Also CKEDITOR.editable.getHtmlFromRange is the base of the CKEDITOR.editor.getSelectedHtml method so the output should is the same.

@f1ames
Copy link
Contributor Author

f1ames commented Nov 15, 2017

Extracted #1169.

@Comandeer
Copy link
Member

Closed in #1170.

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:bug A bug.
Projects
None yet
Development

No branches or pull requests

2 participants