Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Undo and redo no longer crashes in scenarios featuring pasting content into earlier pasted content #1678

Merged
merged 2 commits into from
Feb 18, 2019

Conversation

scofalik
Copy link
Contributor

@scofalik scofalik commented Feb 14, 2019

Suggested merge commit message (convention)

Fix: Undo and redo no longer crashes in scenarios featuring pasting content into earlier pasted content. Closes ckeditor/ckeditor5#1385.


Additional information

Requires https://github.com/ckeditor/ckeditor5-undo/pull/96/files

I had to re-introduce forceWeakRemove flag. This mechanism/flag was used before when we had deltas. I figured it was no longer needed after the refactorings we had. It wasn't needed for long, however, unfortunately, those cases need it. They can't be solved in a way other than explicitly saying: "this is undo, do (not) do this or that".

@Mgsy please test extensively undo after this PR as it touches multiple scenarios. Please test scenarios with merges, splits and removes. All unit tests were fine, though.

@coveralls
Copy link

Coverage Status

Coverage increased (+7.0e-06%) to 99.992% when pulling e668c7f on t/ckeditor5/1385 into 7815ab0 on master.

@Mgsy Mgsy self-requested a review February 14, 2019 14:04
Copy link
Member

@Mgsy Mgsy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works fine. I haven't found any regression.

@Reinmar
Copy link
Member

Reinmar commented Feb 18, 2019

I pasted the same content twice and undid that twice and additional list item was left:

feb-18-2019 09-59-41

@Reinmar
Copy link
Member

Reinmar commented Feb 18, 2019

ok, I can see that this is an unrelated issue. I'll extract it to a separate ticket. @Mgsy, unless you know about similar ticket already?

Reinmar added a commit to ckeditor/ckeditor5-undo that referenced this pull request Feb 18, 2019
Internal: Used `forceWeakRemove` flag in undo OT. See ckeditor/ckeditor5-engine#1678.
@Reinmar Reinmar merged commit 551ab50 into master Feb 18, 2019
@Reinmar Reinmar deleted the t/ckeditor5/1385 branch February 18, 2019 09:12
@Mgsy
Copy link
Member

Mgsy commented Feb 18, 2019

@Mgsy, unless you know about similar ticket already?

AFAIR, we don't have it reported.

@Reinmar
Copy link
Member

Reinmar commented Feb 18, 2019

Reported: ckeditor/ckeditor5#1540

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
4 participants