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

Pasting plain text over existing data fails in Firefox #3475

Closed
jackwickham opened this issue Sep 22, 2019 · 5 comments
Closed

Pasting plain text over existing data fails in Firefox #3475

jackwickham opened this issue Sep 22, 2019 · 5 comments
Assignees
Labels
browser:firefox The issue can only be reproduced in the Firefox browser. regression This issue is a regression. status:confirmed An issue confirmed by the development team. target:minor Any docs related issue that can be merged into a master or major branch. type:bug A bug.
Milestone

Comments

@jackwickham
Copy link
Contributor

Type of report

Bug

Provide detailed reproduction steps (if any)

  1. Enter several words into the editor, such as

Some test content

  1. Select a section from the middle of the text, such as "test"
  2. Paste plain-text content, such as from the URL bar or a search box

Expected result

The selected text is replaced with the clipboard contents

Actual result

An IndexSizeError is thrown usually. When stepping through with the dev tools, it instead inserts the clipboard contents just before the selection, without removing it.

Cause

I think there are a couple of issues here. The first is that the Firefox-specific error handed in selection.selectRanges no longer matches because they have replaced it with a more user-friendly error string. Fixing this error means that the behaviour when stepping through with the dev tools (inserting the clipboard contents just before the selection rather than replacing it) occurs instead.

That Firefox error is caused by trying to call Range.setEnd (on a native range) with an outdated reference to the end container, I think from before the container was split to move the selected content into its own TextNode. That means that endIndex is > TextNode.length, so it is a nonsense range and the exception is triggered.

I've been trying to debug the issue further, but because it behaves differently when breakpoints are used I haven't been able to figure out more than that.

Other details

  • Browser: Firefox 69
  • OS: At least Windows and Linux
  • CKEditor version: major
  • Installed CKEditor plugins: default
@jackwickham jackwickham added the type:bug A bug. label Sep 22, 2019
@f1ames
Copy link
Contributor

f1ames commented Sep 23, 2019

Hello @jackwickham,
I'm not able to reproduce the above issue in Firefox 69 - tested it on Basic, Standard and Full preset (see here - https://ckeditor.com/docs/ckeditor4/latest/examples/basicpreset.html).

Are you able to reproduce the issue on our docs linked above? Do you use any 3rd-party plugins or have specific editor configuration?

@jackwickham
Copy link
Contributor Author

@f1ames I also can't reproduce it on the examples editor that you linked. However, I can reproduce it with my local clone, and I have hosted a reproducible demo at https://www.jackw.net/cke-firefox-paste-demo/ - it is running the latest major commit there, 9ea9e96, with no changes to plugins or any other wierdness.

To reproduce in that demo in Firefox:

  • Select "copyme" and copy it to your clipboard
  • Double click on "demo" in the editor, so that just "demo" is selected
  • Paste

@msamsel
Copy link
Contributor

msamsel commented Sep 25, 2019

I was able to confirm the issue. I also made a bisect to check when it was introduced. It appeard with this commit: b659fba
The issue can be reproduced on night build:
https://nightly.ckeditor.com/

@msamsel msamsel added browser:firefox The issue can only be reproduced in the Firefox browser. regression This issue is a regression. status:confirmed An issue confirmed by the development team. and removed status:pending labels Sep 25, 2019
@f1ames f1ames added this to the Next milestone Sep 25, 2019
@f1ames f1ames added the target:minor Any docs related issue that can be merged into a master or major branch. label Sep 26, 2019
@msamsel msamsel self-assigned this Oct 1, 2019
@msamsel
Copy link
Contributor

msamsel commented Oct 1, 2019

I'm pretty sure that issue is caused because of text node manipulation.
There is stored a selection, before paste operation. Then there is insert a plain text, which divides text node into 2 text nodes and inserts new content between them. So as a result from 1 text node we obtained 3.
Then there is a mechanism to restore selection for FF, which tries to put selection in a non-existing place as the node is now shorter.

@f1ames
Copy link
Contributor

f1ames commented Oct 2, 2019

Fixed in 7d48d2b.

@f1ames f1ames closed this as completed Oct 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
browser:firefox The issue can only be reproduced in the Firefox browser. regression This issue is a regression. status:confirmed An issue confirmed by the development team. target:minor Any docs related issue that can be merged into a master or major branch. type:bug A bug.
Projects
None yet
Development

No branches or pull requests

3 participants