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

Image selection: Uncaught TypeError: Cannot read property 'checkReadOnly' of undefined #2517

Closed
ashklianko opened this issue Oct 25, 2018 · 4 comments · Fixed by #3553
Closed
Labels
browser:chrome The issue can only be reproduced in the Chrome browser. browser:firefox The issue can only be reproduced in the Firefox browser. good first issue Relatively easy to fix. This is a perfect issue if you are willing to create a Pull Request. plugin:widget The plugin which probably causes the issue. status:confirmed An issue confirmed by the development team. type:bug A bug.

Comments

@ashklianko
Copy link

ashklianko commented Oct 25, 2018

Type of report

Bug

Provide detailed reproduction steps (if any)

  1. Insert captioned image as a first entry in editor (nothing should be above it, even empty p or div)
  2. Type some text after image
  3. Start selecting text and release mouse button over image
  4. Open image dialog and try inserting other image

Expected result

New image inserted in place of selected text and image

Actual result

Nothing happens, error in dev console:

Uncaught TypeError: Cannot read property 'checkReadOnly' of undefined

Other details

Reproduced even on https://ckeditor.com/ckeditor-4/

Other details

  • Browser: Chrome, Firefox
@mlewand mlewand self-assigned this Oct 25, 2018
@mlewand mlewand added type:bug A bug. status:confirmed An issue confirmed by the development team. plugin:image2 The plugin which probably causes the issue. target:minor Any docs related issue that can be merged into a master or major branch. labels Oct 25, 2018
@mlewand
Copy link
Contributor

mlewand commented Oct 25, 2018

On Firefox a different exception is thrown: TypeError: b is null, works fine on Edge.

@mlewand mlewand added browser:chrome The issue can only be reproduced in the Chrome browser. browser:firefox The issue can only be reproduced in the Firefox browser. labels Oct 25, 2018
@mlewand mlewand removed their assignment Oct 25, 2018
ashklianko added a commit to enonic/app-contentstudio-old that referenced this issue Oct 31, 2018
@mlewand mlewand added the good first issue Relatively easy to fix. This is a perfect issue if you are willing to create a Pull Request. label Feb 19, 2019
@engineering-this engineering-this self-assigned this Feb 19, 2019
@engineering-this
Copy link
Contributor

engineering-this commented Feb 19, 2019

There is an issue with selection.

Chrome:

startContainer: div.cke_widget_wrapper.cke_widget_block.cke_widget_image,
startOffset: 2

Focusing editor in this case removes selection ranges.

Firefox

startContainer: span.cke_image_resizer_wrapper,
startOffset: 1

In this case widget is not inserted because range.checkReadOnly returns true, and then editor tries to focus widget that wasn't inserted which results in error.

@engineering-this
Copy link
Contributor

See #2866 (review). There are other closely related issues #3007 and #3008 caused by buggy selection. Currently #2866 contains fix for selection on widget finalizeCreation. However as there are more related issues and every browser allows different selection it would be better to normalize selection. widgetselection plugin is right place for such logic.

If one end of range is in widget, and it's not at the start of it selection should be moved out of a widget.

@engineering-this
Copy link
Contributor

engineering-this commented Apr 9, 2019

There is another steps to reproduce this issue:

Steps

  1. Open https://codepen.io/anon/pen/dLNPGg
  2. Open browser console.
  3. Press button below editor.
  4. Press Image2 button.
  5. Paste or type any src, and press OK.

Expected

Image is inserted into editor.

Actual

  • Image is not inserted.
  • Error is thrown.

Note: such selection is created when I try to select whole editor content in IE11 with mouse.

Edit:

Actually it's different bug caused by range.deleteContents. Extracted to #3041

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
browser:chrome The issue can only be reproduced in the Chrome browser. browser:firefox The issue can only be reproduced in the Firefox browser. good first issue Relatively easy to fix. This is a perfect issue if you are willing to create a Pull Request. plugin:widget The plugin which probably causes the issue. status:confirmed An issue confirmed by the development team. type:bug A bug.
Projects
None yet
5 participants