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

OT: context.forceNotSticky may not be 100% accurate #4100

Closed
scofalik opened this issue Jul 5, 2017 · 1 comment
Closed

OT: context.forceNotSticky may not be 100% accurate #4100

scofalik opened this issue Jul 5, 2017 · 1 comment
Labels
package:engine resolution:invalid This issue is invalid (e.g. reports a non-existent bug or a by-design behavior). type:improvement This issue reports a possible enhancement of an existing feature.

Comments

@scofalik
Copy link
Contributor

scofalik commented Jul 5, 2017

(Follow-up to ckeditor/ckeditor5-engine#977)

To speed up work on ckeditor/ckeditor5-engine#977 I've implemented context.forceNotSticky using some naive assumptions that may fail during selective undo.

Reminder: context.forceNotSticky disables MoveOperation#isSticky property during OT if it is set to true. This is needed for undo to work correctly, however, of course, the flag needs to be respected for proper transformation of MergeDelta, SplitDelta and UnwrapDelta.

Right now, stickiness is disabled (context.forceNotSticky == true) when the delta that we transform by has been undone or is a delta that undoes another delta:

if ( history.isUndoingDelta( originalB ) || history.isUndoneDelta( originalB ) ) {
	context.forceNotSticky = true;
}

This is a bit naive assumption that I though of when I considered in what cases the stickiness fail. The better way to solve it might be like with context.insertBefore. This means that we enable/disable stickiness basing on how the transformed delta was affected by the deltas it was previously transformed by.

BTW.
At one point I wanted to remove isSticky and move those transformation nuances to deltas special cases but this would require a lot lot more work than implementing context.forceNotSticky to fix #isSticky support.

@Reinmar Reinmar closed this as completed Aug 8, 2019
@Reinmar
Copy link
Member

Reinmar commented Aug 8, 2019

Such property does not exist any more.

@mlewand mlewand transferred this issue from ckeditor/ckeditor5-engine Oct 9, 2019
@mlewand mlewand added module:model resolution:invalid This issue is invalid (e.g. reports a non-existent bug or a by-design behavior). type:improvement This issue reports a possible enhancement of an existing feature. package:engine labels Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:engine resolution:invalid This issue is invalid (e.g. reports a non-existent bug or a by-design behavior). type:improvement This issue reports a possible enhancement of an existing feature.
Projects
None yet
Development

No branches or pull requests

3 participants