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.

Sign up for GitHub

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

Jump to bottom

Mapper: default position mapping algorithms may crash editor. #4025

Closed
scofalik opened this issue Mar 21, 2017 · 3 comments · Fixed by ckeditor/ckeditor5-engine#885
Closed

Mapper: default position mapping algorithms may crash editor. #4025

scofalik opened this issue Mar 21, 2017 · 3 comments · Fixed by ckeditor/ckeditor5-engine#885
Assignees
@scofalik
Labels
package:engine type:improvement This issue reports a possible enhancement of an existing feature.
Milestone
iteration 9

Comments

@scofalik
Copy link
Contributor

At the moment, model-to-view and view-to-model position mapping use first the default algorithm and then additional callbacks may be fired and alter the position calculated using default algorithm.

This behaviour sounds okay, but is a problem in List feature because for some list structures the default algorithm throws.

Hence I propose that default algorithm should be used only if no custom callback has been used.
@scofalik scofalik self-assigned this Mar 21, 2017
pjasiun referenced this issue in ckeditor/ckeditor5-engine Mar 24, 2017
Merge pull request #885 from ckeditor/t/884
5627993 
Other: Default conversion.Mapper position mapping algorithms are now added as callbacks with low priority and are fired only if earlier callbacks did not provide a result. Closes #884.

BREAKING CHANGES: Since default position mapping algorithms are attached with low priority, custom position mapping callbacks added with higher priority won't receive position calculated by default algorithms in data. To execute default position mapping algorithms and use their value, hook custom callback with lower priority.
@Reinmar
Copy link
Member

Reinmar commented Mar 24, 2017

Unfortunately, ckeditor/ckeditor5-engine#885 blew up other features: https://travis-ci.org/ckeditor/ckeditor5/builds/214576912

@Reinmar Reinmar reopened this Mar 24, 2017
@Reinmar
Copy link
Member

Reinmar commented Mar 24, 2017

And in MQ manual tests (on editor load):

image

@scofalik
Copy link
Contributor Author

Those errors are because of model-to-view position mapping callback for List feature. They will disappear after PR in List is merged.

@Reinmar Reinmar closed this as completed Mar 27, 2017
@mlewand mlewand transferred this issue from ckeditor/ckeditor5-engine Oct 9, 2019
@mlewand mlewand added this to the iteration 9 milestone Oct 9, 2019
@mlewand mlewand added module:conversion type:improvement This issue reports a possible enhancement of an existing feature. package:engine labels Oct 9, 2019
@scofalik scofalik mentioned this issue Oct 11, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@scofalik scofalik

Labels
package:engine type:improvement This issue reports a possible enhancement of an existing feature.
Projects
None yet
Milestone
iteration 9
Development

Successfully merging a pull request may close this issue.

T/884 Mapper: default position mapping algorithms may crash editor ckeditor/ckeditor5-engine
3 participants
@Reinmar @scofalik @mlewand