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

model.Differ does misses some cases for attribute changes transformations #4338

Closed
scofalik opened this issue Apr 10, 2018 · 1 comment · Fixed by ckeditor/ckeditor5-engine#1405
Assignees
Labels
package:engine type:bug This issue reports a buggy (incorrect) behavior.
Milestone

Comments

@scofalik
Copy link
Contributor

Well, the thing is simple: right now we are handling just one out of four possible cases for attribute changes. We need to handle all four "intersecting" scenarios:

{   [   }   ]    -->    {   }[      ]
{   [   ]   }    -->    {           }
[   {   }   ]    -->    [           ]
[   {   ]   }    -->    [      ]{   }

Where {} is incoming change and [] is already buffered chage.

@scofalik scofalik self-assigned this Apr 10, 2018
@scofalik
Copy link
Contributor Author

Actually, after further research, I realized that there are only three possible scenarios:

  1. Incoming change is same as buffered change.
  2. Incoming change contains buffered change.
  3. Incoming change is inside buffered change.

AFAICS, there will never be intersecting scenarios because model.Differ iterates over item in the model tree:

for ( const item of operation.range.getItems() ) {
	if ( this._isInInsertedElement( item.parent ) ) {
		continue;
	}

	this._markAttribute( item );
}

So there never will be a situation with intersecting attribute change items.

pjasiun referenced this issue in ckeditor/ckeditor5-engine Apr 11, 2018
Fix: `model.Differ` did not handle attribute change transformation correctly. Closes #1404.
@mlewand mlewand transferred this issue from ckeditor/ckeditor5-engine Oct 9, 2019
@mlewand mlewand added this to the iteration 16 milestone Oct 9, 2019
@mlewand mlewand added module:model type:bug This issue reports a buggy (incorrect) behavior. 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 type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants