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.

Sign up for GitHub

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

Jump to bottom

Undo after splitting elements crashes the editor. #9296

Closed
magda-chrzescian opened this issue Mar 18, 2021 · 5 comments · Fixed by #17071
Closed

Undo after splitting elements crashes the editor. #9296

magda-chrzescian opened this issue Mar 18, 2021 · 5 comments · Fixed by #17071
Assignees
@scofalik
Labels
domain:ui/ux This issue reports a problem related to UI or UX. package:engine squad:core Issue to be handled by the Core team. support:1 An issue reported by a commercially licensed client. type:bug This issue reports a buggy (incorrect) behavior.
Milestone
iteration 78 (v43...

Comments

@magda-chrzescian
Copy link
Contributor

magda-chrzescian commented Mar 18, 2021 •
edited
Loading

📝 Provide detailed reproduction steps (if any)

Scenario 1:

  1. Open any editor instance.
  2. Type a few letters.
  3. Set the caret between the letters and press enter 4 times.
  4. Press backspace 4 times.
  5. Press Ctrl+Z 5 times -> The action is not executed properly.
  6. Press Ctrl+Shift+Z -> The editor crashes, no action possible except of typing.

This is the error thrown in the 6th point:

Read more: https://ckeditor.com/docs/ckeditor5/latest/framework/guides/support/error-codes.html#error-split-operation-how-many-invalid
    at l._validate (https://ckeditor.com/docs/ckeditor5/latest/snippets/build-classic-source/snippet.js:4:101751)
    at ft.on.priority (https://ckeditor.com/docs/ckeditor5/latest/snippets/build-classic-source/snippet.js:4:405830)
    at ft.fire (https://ckeditor.com/docs/ckeditor5/latest/snippets/build-classic-source/snippet.js:4:56829)
    at ft.<computed> [as applyOperation] (https://ckeditor.com/docs/ckeditor5/latest/snippets/build-classic-source/snippet.js:4:59799)
    at u._undo (https://ckeditor.com/docs/ckeditor5/latest/snippets/build-classic-source/snippet.js:4:462589)
    at Object.callback (https://ckeditor.com/docs/ckeditor5/latest/snippets/build-classic-source/snippet.js:4:463626)
    at ft._runPendingChanges (https://ckeditor.com/docs/ckeditor5/latest/snippets/build-classic-source/snippet.js:4:409575)
    at ft.enqueueChange (https://ckeditor.com/docs/ckeditor5/latest/snippets/build-classic-source/snippet.js:4:406706)
    at u.execute (https://ckeditor.com/docs/ckeditor5/latest/snippets/build-classic-source/snippet.js:4:463408)
    at u.<anonymous> (https://ckeditor.com/docs/ckeditor5/latest/snippets/build-classic-source/snippet.js:4:59748)

This is the error thrown while trying to do anything else with the editor:

Read more: https://ckeditor.com/docs/ckeditor5/latest/framework/guides/support/error-codes.html#error-model-nodelist-offset-out-of-bounds
    at offsetToIndex (https://ckeditor.com/docs/ckeditor5/latest/snippets/build-classic-source/snippet.js:4:191156)
    at c.offsetToIndex (https://ckeditor.com/docs/ckeditor5/latest/snippets/build-classic-source/snippet.js:4:94211)
    at a (https://ckeditor.com/docs/ckeditor5/latest/snippets/build-classic-source/snippet.js:4:29321)
    at r.get textNode [as textNode] (https://ckeditor.com/docs/ckeditor5/latest/snippets/build-classic-source/snippet.js:4:23466)
    at W (https://ckeditor.com/docs/ckeditor5/latest/snippets/build-classic-source/snippet.js:4:390188)
    at H._validateSelectionRange (https://ckeditor.com/docs/ckeditor5/latest/snippets/build-classic-source/snippet.js:4:390027)
    at m.<anonymous> (https://ckeditor.com/docs/ckeditor5/latest/snippets/build-classic-source/snippet.js:4:135889)
    at m.fire (https://ckeditor.com/docs/ckeditor5/latest/snippets/build-classic-source/snippet.js:4:56829)
    at m._setRanges (https://ckeditor.com/docs/ckeditor5/latest/snippets/build-classic-source/snippet.js:4:193998)
    at m.setTo (https://ckeditor.com/docs/ckeditor5/latest/snippets/build-classic-source/snippet.js:4:193027)

Scenario 2:

  1. Open any editor instance.
  2. Create content with a header and paragraph.
  3. Set the caret at the beginning of the header and press enter 2 times, then backspace 2 times.
  4. Set the caret between the letters of the header and press enter 2 times, then backspace 2 times.
  5. Keep pressing Ctrl-Z until you get a console error and the editor crashes.

This is the error thrown in the 5th point:

Read more: https://ckeditor.com/docs/ckeditor5/latest/framework/guides/support/error-codes.html#error-rename-operation-wrong-name
    at a._validate (https://ckeditor.com/docs/ckeditor5/latest/snippets/build-classic-source/snippet.js:4:174102)
    at ft.on.priority (https://ckeditor.com/docs/ckeditor5/latest/snippets/build-classic-source/snippet.js:4:405830)
    at ft.fire (https://ckeditor.com/docs/ckeditor5/latest/snippets/build-classic-source/snippet.js:4:56829)
    at ft.<computed> [as applyOperation] (https://ckeditor.com/docs/ckeditor5/latest/snippets/build-classic-source/snippet.js:4:59799)
    at l._undo (https://ckeditor.com/docs/ckeditor5/latest/snippets/build-classic-source/snippet.js:4:462589)
    at Object.callback (https://ckeditor.com/docs/ckeditor5/latest/snippets/build-classic-source/snippet.js:4:463078)
    at ft._runPendingChanges (https://ckeditor.com/docs/ckeditor5/latest/snippets/build-classic-source/snippet.js:4:409575)
    at ft.enqueueChange (https://ckeditor.com/docs/ckeditor5/latest/snippets/build-classic-source/snippet.js:4:406706)
    at l.execute (https://ckeditor.com/docs/ckeditor5/latest/snippets/build-classic-source/snippet.js:4:463052)
    at l.<anonymous> (https://ckeditor.com/docs/ckeditor5/latest/snippets/build-classic-source/snippet.js:4:59748)

This is the error thrown while trying to do anything else with the editor:

Read more: https://ckeditor.com/docs/ckeditor5/latest/framework/guides/support/error-codes.html#error-model-nodelist-offset-out-of-bounds
    at offsetToIndex (https://ckeditor.com/docs/ckeditor5/latest/snippets/build-classic-source/snippet.js:4:191156)
    at c.offsetToIndex (https://ckeditor.com/docs/ckeditor5/latest/snippets/build-classic-source/snippet.js:4:94211)
    at a (https://ckeditor.com/docs/ckeditor5/latest/snippets/build-classic-source/snippet.js:4:29321)
    at r.get textNode [as textNode] (https://ckeditor.com/docs/ckeditor5/latest/snippets/build-classic-source/snippet.js:4:23466)
    at W (https://ckeditor.com/docs/ckeditor5/latest/snippets/build-classic-source/snippet.js:4:390188)
    at H._validateSelectionRange (https://ckeditor.com/docs/ckeditor5/latest/snippets/build-classic-source/snippet.js:4:390027)
    at m.<anonymous> (https://ckeditor.com/docs/ckeditor5/latest/snippets/build-classic-source/snippet.js:4:135889)
    at m.fire (https://ckeditor.com/docs/ckeditor5/latest/snippets/build-classic-source/snippet.js:4:56829)
    at m._setRanges (https://ckeditor.com/docs/ckeditor5/latest/snippets/build-classic-source/snippet.js:4:193998)
    at m.setTo (https://ckeditor.com/docs/ckeditor5/latest/snippets/build-classic-source/snippet.js:4:193027)

2021-03-22 09 04 13

If you'd like to see this fixed sooner, add a 👍 reaction to this post.
@magda-chrzescian magda-chrzescian added type:bug This issue reports a buggy (incorrect) behavior. domain:ui/ux This issue reports a problem related to UI or UX. package:engine labels Mar 18, 2021
@urbanspr1nter
Copy link
Contributor

I was able to hit this pretty easily by just splitting a text node with some attributes (like bold, italic, etc) and pasting in plain text in the following order:

  1. end of the text node
  2. middle of the text node (causing the text node to split)
  3. beginning of the text node
    Then perform the undo operations... The editor crashes.

@mabryl mabryl added the squad:core Issue to be handled by the Core team. label Dec 2, 2022
@mabryl
Copy link
Contributor

mabryl commented Dec 2, 2022

Can confirm that issue still persists in our online demos.

@martynawierzbicka martynawierzbicka added the support:2 An issue reported by a commercially licensed client. label Dec 2, 2022
@lslowikowska lslowikowska added support:1 An issue reported by a commercially licensed client. and removed support:2 An issue reported by a commercially licensed client. labels Jul 24, 2023
@scofalik
Copy link
Contributor

I can reproduce scenario 1 without typing letters, just make two enters, then two backspace, then undo all and do one redo. I get: split-operation-how-many-invalid. I also can see that undos are not done properly.

I can reproduce second scenario too but only if I start re-doing. I get the same error as above.

@scofalik
Copy link
Contributor

Fixed by #17071

@scofalik scofalik mentioned this issue Sep 16, 2024
Merged
@scofalik scofalik self-assigned this Sep 16, 2024
@oleq oleq closed this as completed in 471390b Sep 23, 2024
oleq added a commit that referenced this issue Sep 23, 2024
@oleq
Merge pull request #17071 from ckeditor/cc/4371-3
a639c35 
Fix (engine): Fixed incorrect marker handling in some scenarios involving undo and real-time collaboration, which earlier led to a `model-nodelist-offset-out-of-bounds` error. See #9296.

Fix (engine): Fixed incorrect handling of merge changes during undo in some scenarios involving real-time collaboration, which earlier led to a `model-nodelist-offset-out-of-bounds` error. See #9296.

Fix (engine): Fixed conflict resolution error, which led to editor crash in some scenarios where two users removed bigger intersecting part of the content and used undo. See #9296.

Fix (engine): Fixed incorrect undo behavior leading to an editor crash when a user pressed enter key multiple times, then pressed backspace that many times, then undo all the changes. Closes #9296.
@CKEditorBot CKEditorBot added this to the iteration 78 milestone Sep 23, 2024
@CKEditorBot CKEditorBot mentioned this issue Sep 24, 2024
Open
@scofalik
Copy link
Contributor

scofalik commented Sep 27, 2024 •
edited
Loading

It appeared that the second scenario can be still reproduced, only I didn't follow the steps precisely. Also, I managed to narrow down this case. I created a new issue for it. #17178

@scofalik scofalik mentioned this issue Sep 27, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@scofalik scofalik

Labels
domain:ui/ux This issue reports a problem related to UI or UX. package:engine squad:core Issue to be handled by the Core team. support:1 An issue reported by a commercially licensed client. type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
Milestone
iteration 78 (v43.2.0)
Development

Successfully merging a pull request may close this issue.

Fix MarkerOperation x MoveOperation OT for undo cases ckeditor/ckeditor5
7 participants
@urbanspr1nter @scofalik @magda-chrzescian @CKEditorBot @lslowikowska @martynawierzbicka @mabryl