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

definition.priority -> definition.converterPriority in Conversion#attributeToElement #4454

Closed
pjasiun opened this issue Dec 21, 2018 · 3 comments · Fixed by ckeditor/ckeditor5-engine#1630
Closed

definition.priority -> definition.converterPriority in Conversion#attributeToElement #4454

pjasiun opened this issue Dec 21, 2018 · 3 comments · Fixed by ckeditor/ckeditor5-engine#1630
Assignees
@jodator
Labels
package:engine type:bug This issue reports a buggy (incorrect) behavior.
Milestone
iteration 22

Comments

@pjasiun
Copy link

pjasiun commented Dec 21, 2018

Follow-up of ckeditor/ckeditor5-engine#1613.

https://github.com/ckeditor/ckeditor5-engine/blob/2f41916122b32ec9c06c5464ad2e615f7a1dc5b3/src/conversion/conversion.js#L399

- converterPriority: definition.priority
+ converterPriority: definition.converterPriority

I check the documentation and there is nothing like definition.priority. There is definition.converterPriority in both type definition and the code usage samples.

There should be also a test added to be sure that we will have no such bug in the future.
@scofalik
Copy link
Contributor

AFAIR this was fixed by @f1ames recently. Did you check master?

@scofalik
Copy link
Contributor

Ok I see it wasn't changed everywhere.

@f1ames
Copy link
Contributor

f1ames commented Dec 21, 2018

@scofalik The other part was fixed https://github.com/ckeditor/ckeditor5-engine/pull/1611/files#diff-8f355710085f3ab802231c0973944effR413, because there was priority: definition.priority and was changed to converterPriority: definition.priority but should probably be converterPriority: definition.converterPriority as @pjasiun mentioned.

scofalik referenced this issue in ckeditor/ckeditor5-engine Jan 3, 2019
@scofalik
Merge pull request #1630 from ckeditor/t/1617
fe6d17d 
Fix: Converter priority passing in `conversion.attributeToElement()`. Closes #1617.
@mlewand mlewand transferred this issue from ckeditor/ckeditor5-engine Oct 9, 2019
@mlewand mlewand added this to the iteration 22 milestone Oct 9, 2019
@mlewand mlewand added module:conversion type:bug This issue reports a buggy (incorrect) behavior. package:engine labels Oct 9, 2019
@jodator jodator 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

@jodator jodator

Labels
package:engine type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
Milestone
iteration 22
Development

Successfully merging a pull request may close this issue.

T/1617: Converter priority passing in conversion.attributeToElement() ckeditor/ckeditor5-engine
5 participants
@jodator @f1ames @pjasiun @scofalik @mlewand