Opened 3 years ago

Closed 3 years ago

#13513 closed Bug (fixed)

Divarea and getData throw error when image is only data in editor.

Reported by: Jakub Ś Owned by: Szymon Cofalik
Priority: Normal Milestone: CKEditor 4.5.7
Component: General Version: 4.4.6
Keywords: Blink Webkit Support Cc: glen.84@…

Description

Use attached sample to reproduce the problem.

  1. Copy image from some webpage
  2. Paste it into editor with Ctrl+V
  3. Click Clear
  4. Paste image into editor with Ctrl+V
  5. Click getData

Result: In Webkit and Blink JS error is thrown.

Message: Uncaught IndexSizeError - Failed to execute 'extend' on 'Selection': 1 is larger than the given node's length.
Line: 224
URI: core/selection.js

Attachments (1)

CKEditor.html (894 bytes) - added by Jakub Ś 3 years ago.

Download all attachments as: .zip

Change History (24)

Changed 3 years ago by Jakub Ś

Attachment: CKEditor.html added

comment:1 Changed 3 years ago by Jakub Ś

Status: newconfirmed

comment:2 Changed 3 years ago by Jonathan

Tracking.

comment:3 Changed 3 years ago by Jonathan

We are seeing this exact same stack trace from users about 20 times a day from Chrome users. All of them occur in the context of calling editor.getData(). We do not allow pasting images in production yet, but these errors may share a root cause. Production is running CKEditor 4.4.7.

comment:4 Changed 3 years ago by Piotrek Koszuliński

Milestone: CKEditor 4.5.2

OK, this is another ticket pointing to the same extend(). We've already worked on some fixes around it, so I don't know if one of the patches on review does not fix it, but to remember to check I'm adding this ticket to 4.5.2 too.

comment:5 Changed 3 years ago by Szymon Cofalik

Fixed by #13284.

comment:6 Changed 3 years ago by Jonathan

The workflow above appears to be fixed by #13284, but there is a related workflow that still causes the error. These two workflows only happen with the 'image2' plugin enabled.

  1. Copy image from some webpage
  2. Paste it into editor with Ctrl+V
  3. Delete the image.
  4. Type some text without a space
  5. Click getData

Another workflow:

  1. Copy image from some webpage
  2. Paste it into the editor with Ctrl+V
  3. Hit the right arrow button to place the caret right next to the image.
  4. Type some text without a space.
  5. Click getData

Removing the 'image2' plugin, the error manifests itself differently:

  1. Copy image from some webpage
  2. Paste it into the editor with Ctrl+V
  3. Hit the right arrow button to place the caret right next to the image.
  4. Type some text without a space.
  5. Click getData
  6. Click clear

You can see the issue with the 'image2' plugin here. If you want, I can remove the plugin so that you can see the other issue. Example

comment:7 Changed 3 years ago by Szymon Cofalik

Owner: set to Szymon Cofalik
Status: confirmedassigned

comment:8 Changed 3 years ago by Szymon Cofalik

Status: assignedreview

@jbalinski: thanks for providing test cases!

I've pushed branch:t/13513 with additional fix. It seems like a weird bug on Chrome's end but I can't reproduce it outside CKEditor "environment". The fix is separate from #13284 and branch:t/13284 but both are connected to similar issues.

comment:9 Changed 3 years ago by Jonathan

The latest fix appears to prevent the error from occurring.

comment:10 Changed 3 years ago by Piotrek Koszuliński

Milestone: CKEditor 4.5.2CKEditor 4.5.3
Status: reviewreview_failed

comment:11 Changed 3 years ago by Szymon Cofalik

Status: review_failedassigned

comment:12 Changed 3 years ago by Szymon Cofalik

Status: assignedreview

I rebased branch:t/13513 with master and pushed tests.

I narrowed test cases to two, simpler scenarios. Errors are caused by ZWS, so any command that inserts ZWS into editor results in same error, it doesn't have to be an image. I used bold. Instead messing with image, now I just use bold command to insert strong element with ZWS in it. I also reduced plugins set to a minimal group that is needed to reproduce the error.

@jbalinski provided 3 test cases. TC1 and TC2 are basically same, and TC3 is the same thing but we fire setData( '' ) instead of getData(). I was able to automate the tests. While writing tests I noticed that the error is thrown only when getData/setData is fired after click the button, which lead me to discover that it, in fact, is only thrown if editor is blurred before getData/setData. Maybe this information will be useful during review. I simulated this by firing blur() directly on editable().$ element.

comment:13 Changed 3 years ago by Piotrek Koszuliński

Small note here - instead of blurring editable, focus some other field (e.g. <input>). This is more predictable.

comment:14 Changed 3 years ago by Szymon Cofalik

Fixes also #13389 - I pushed a test that checks that case.

comment:15 Changed 3 years ago by Szymon Cofalik

@Reinmar: It seems that, because of unknown reason, it crashes only if I use blur() on contenteditable. focus() on another field does not make the code to throw an error.

comment:16 Changed 3 years ago by Olek Nowodziński

Milestone: CKEditor 4.5.3CKEditor 4.5.4

comment:17 Changed 3 years ago by Piotrek Koszuliński

Another related ticket #12727.

comment:18 Changed 3 years ago by Piotrek Koszuliński

I'd wait with this for results of @a.nowodzinski's work on a complete filling char rewrite. Perhaps all this won't be necessary after that.

comment:19 Changed 3 years ago by Piotrek Koszuliński

Milestone: CKEditor 4.5.4CKEditor 4.5.5

comment:20 Changed 3 years ago by darkangel

Cc: glen.84@… added

comment:21 Changed 3 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.5CKEditor 4.5.6

This issue will be fixed by #13816 in the next milestone.

comment:22 Changed 3 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.6CKEditor 4.5.7

comment:23 Changed 3 years ago by Marek Lewandowski

Resolution: fixed
Status: reviewclosed

Fixed with git:2d078ff42a by #13816.

Note: See TracTickets for help on using tickets.
© 2003 – 2017 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy