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

Weird case with attribute removal in model post-fixer #1645

Closed
jodator opened this issue Mar 22, 2019 · 4 comments
Closed

Weird case with attribute removal in model post-fixer #1645

jodator opened this issue Mar 22, 2019 · 4 comments
Assignees
Labels
browser:chrome browser:firefox type:bug This issue reports a buggy (incorrect) behavior.
Milestone

Comments

@jodator
Copy link
Contributor

jodator commented Mar 22, 2019

Is this a bug report or feature request? (choose one)

🐞 Bug report

💻 Version of CKEditor

latest master

📋 Steps to reproduce

  1. Register attribute post-fixer:
editor.model.document.registerPostFixer( writer => {
	const changes = editor.model.document.differ.getChanges();

	let wasChanged = false;

	for ( const change of changes ) {
		if ( change.type == 'insert' || change.type == 'remove' ) {
			const textNode = change.position.textNode;

			if ( change.name == '$text' && textNode && textNode.hasAttribute( 'bold' ) ) {

				writer.removeAttribute( 'bold', textNode );
				wasChanged = true;
			}
		}
	}

	return wasChanged;
} );
  1. bold text ie bold
  2. delete some char, ie l (forward or backward delete)
  3. the model has correct state <paragraph><$text bold="true">bo[]d</$text></paragraph>
  4. the view has incorrect state: <p>bo[]l</p>
  5. execute undo

✅ Expected result

The text after deletion is bod in the view.
The text after undo id bold in the view.
The text after undo has bold attribute in the model.

❎ Actual result

The text after deletion in the editing area is bol.
The text after undo id boll in the view.
The text after undo doesn't have bold attribute.

📃 Other details that might be useful

Screen cast:
Peek 2019-03-22 10-15

@mlewand mlewand added this to the iteration 23 milestone Mar 22, 2019
@jodator
Copy link
Contributor Author

jodator commented Mar 22, 2019

ps.: The same happens on FF and Chrome.

@mlewand mlewand added type:bug This issue reports a buggy (incorrect) behavior. browser:chrome browser:firefox labels Mar 22, 2019
@scofalik
Copy link
Contributor

I confirm the bug.

Quick observation: when removing, always the last letter is removed, unless the first letter is removed. When typing, the typing itself is fine but undo fails.

@scofalik
Copy link
Contributor

This is a bug in Differ. A quite serious one, it is funny that it didn't come up earlier but I guess we never had a scenario where a word changed an attribute and also got something removed/added.

@scofalik
Copy link
Contributor

Okaaay, so ckeditor/ckeditor5-engine#1705 this fixes the issue with Differ. Attribute change was incorrectly stored if there was a remove change with intersecting range.

However, this does not fix not-bolding on undo. In fact, everything works correctly. The bold is applied, but since also there is a letter inserted (or removed) during the undo, the bold is immediately taken off.

So the post-fixer like this for mentions is not correct.

Reinmar added a commit to ckeditor/ckeditor5-engine that referenced this issue Mar 28, 2019
Fix: Attribute and remove change on intersecting ranges done in the same change block will be correctly saved in `Differ` and downcasted. Closes ckeditor/ckeditor5#1645.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
browser:chrome browser:firefox type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
Development

No branches or pull requests

3 participants