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

Selection should not retain attributes after parent block removal by an external change #4258

Closed
oleq opened this issue Feb 1, 2018 · 2 comments · Fixed by ckeditor/ckeditor5-engine#1343
Labels
package:engine type:bug This issue reports a buggy (incorrect) behavior.
Milestone

Comments

@oleq
Copy link
Member

oleq commented Feb 1, 2018

  1. Go to http://localhost:8125/ckeditor5-link/tests/manual/tickets/113/1.html.
  2. Take the 2nd scenario.
  3. Write some text.

Expected:

The selection should not retain previous (linkHref) attribute. The whole editing context has changed and most likely users will not expect that.

Actual:

Still writing in the same link despite that it's parent paragraph has been removed from the model.

kapture 2018-02-01 at 14 55 05

Spotted in ckeditor/ckeditor5-link#165.

Note: It's not about the link. Other attributes like bold, etc. are also retained.

@oleq oleq changed the title Selection should not retain attributes after parent block removal by external change Selection should not retain attributes after parent block removal by an external change Feb 1, 2018
@Reinmar
Copy link
Member

Reinmar commented Feb 1, 2018

This is weird. cc @scofalik @pjasiun

@pjasiun
Copy link

pjasiun commented Feb 1, 2018

That's definitely a bug. This is because selection attributes are updated only as the result of selection operations:

https://github.com/ckeditor/ckeditor5-engine/blob/6d0c555c0a6da4e0aad9786aa50b6258bd8c51ad/src/model/documentselection.js#L486-L488

This makes sense for attribute define "manually". If I click the bold button I don't want to lose this attribute because of external change (which may even not touch my selection). It should, apparently, behave differently for attributes which are set based on the surrounding text. We already distinguish these types of attributes using the attribute priority.

BTW: attributes priorities should be better described, now it's hard to learn what some parts of the code do.

scofalik referenced this issue in ckeditor/ckeditor5-engine Mar 8, 2018
Fix: `model.DocumentSelection` should update it's attributes after each change, including external changes. Closes #1267.
@mlewand mlewand transferred this issue from ckeditor/ckeditor5-engine Oct 9, 2019
@mlewand mlewand added this to the iteration 14 milestone Oct 9, 2019
@mlewand mlewand added module:selection 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.

5 participants