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

Undo, redo after paste crashes the editor #1287

Closed
Reinmar opened this issue Oct 5, 2018 · 3 comments
Closed

Undo, redo after paste crashes the editor #1287

Reinmar opened this issue Oct 5, 2018 · 3 comments
Assignees
Labels
package:engine type:bug This issue reports a buggy (incorrect) behavior.
Milestone

Comments

@Reinmar
Copy link
Member

Reinmar commented Oct 5, 2018

There are two TCs here:

TC1

Similar to #1278 but paste happens on a non-collapsed selection:

oct-05-2018 11-00-06

Undo and redo causes:

ckeditorerror.js:46 Uncaught CKEditorError: merge-operation-how-many-invalid: Merge operation specifies wrong number of nodes to move. Read more: https://ckeditor.com/docs/ckeditor5/latest/framework/guides/support/error-codes.html#error-merge-operation-how-many-invalid


CKEditorError @ ckeditorerror.js:46
_validate @ mergeoperation.js:164
Model.on @ model.js:83
fire @ emittermixin.js:196
(anonymous function) @ observablemixin.js:253
_undo @ basecommand.js:161
editor.model.enqueueChange @ redocommand.js:44
_runPendingChanges @ model.js:496
enqueueChange @ model.js:205
execute @ redocommand.js:38
on @ observablemixin.js:249
fire @ emittermixin.js:196
(anonymous function) @ observablemixin.js:253
execute @ commandcollection.js:67
execute @ editor.js:292
listenTo @ undoui.js:58
fire @ emittermixin.js:196
ButtonView.setTemplate.on.click.bind.to.evt @ buttonview.js:149
callback @ template.js:946
fire @ emittermixin.js:196
domListener @ emittermixin.js:235

TC2

The same scenario as in #1278 but keep doing undo, redo after pasting. First, that makes the content invalid and then crashes the editor:

oct-05-2018 11-09-22

The error:

ckeditorerror.js:46 Uncaught CKEditorError: merge-operation-target-position-invalid: Merge target position is invalid. Read more: https://ckeditor.com/docs/ckeditor5/latest/framework/guides/support/error-codes.html#error-merge-operation-target-position-invalid


CKEditorError @ ckeditorerror.js:46
_validate @ mergeoperation.js:157
Model.on @ model.js:83
fire @ emittermixin.js:196
(anonymous function) @ observablemixin.js:253
_undo @ basecommand.js:161
editor.model.enqueueChange @ redocommand.js:44
_runPendingChanges @ model.js:496
enqueueChange @ model.js:205
execute @ redocommand.js:38
on @ observablemixin.js:249
fire @ emittermixin.js:196
(anonymous function) @ observablemixin.js:253
execute @ commandcollection.js:67
execute @ editor.js:292
listenTo @ undoui.js:58
fire @ emittermixin.js:196
ButtonView.setTemplate.on.click.bind.to.evt @ buttonview.js:149
callback @ template.js:946
fire @ emittermixin.js:196
domListener @ emittermixin.js:235
@scofalik
Copy link
Contributor

scofalik commented Oct 9, 2018

I created a PR which fixes TC1. However after you do steps form TC1 and click undo again, it will create unexpected content and then redo crashes. That error and the one from TC2 look similar, so I believe it may be the same thing. However, they get so complicated that I am having difficulty debugging it. As sad and laughable as it may sound, I am not sure if I will be able to fix it without bigger change connected with undo&OT.

@scofalik
Copy link
Contributor

I managed to fix TC2, but unfortunately it didn't fix undo-redo-undo in TC1.

@scofalik
Copy link
Contributor

I managed to fix all those bugs. I hope.

pjasiun pushed a commit to ckeditor/ckeditor5-engine that referenced this issue Oct 23, 2018
Fix: Corrected transformations for pasting and undo scenarios. Closes ckeditor/ckeditor5#1287.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:engine type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
Development

No branches or pull requests

2 participants