Opened 8 years ago
Closed 8 years ago
#16957 closed Bug (fixed)
[PFW] Styles.inliner.inline does not take selectors specificity into account and duplicates rules.
Reported by: | kkrzton | Owned by: | kkrzton |
---|---|---|---|
Priority: | Normal | Milestone: | CKEditor 4.7.0 |
Component: | General | Version: | |
Keywords: | PFW | Cc: |
Description
Steps to reproduce
Having a html in dataTransfer object like
<style> td { color:black; } .xl65 { color:#C00000; } </style> <td class="xl65"> ... </td>
and using CKEDITOR.plugins.pastefromword.styles.inliner.inline
produces a styles object like
{ .xl65: { color: '#C00000' }, td: { color: 'black' } }
Those styles are the applied to a specific element. This means the colors are applied in the incorrect order (first, color from .xl65
class, then from td
- which overrides the first one). The CSS order specificity should be keep in mind here. Also the inliner does not check if same style is already defined so that the resulted styles will have two color
rules. It wille result in style like color:#C00000;color:black;
(it is not a big issue unless the order is correct, however it might be also improved).
See:
- https://github.com/ckeditor/ckeditor-dev/blob/major/plugins/pastefromword/filter/default.js#L867
- https://github.com/ckeditor/ckeditor-dev/blob/major/plugins/pastefromword/filter/default.js#L852
Two improvements could be applied here:
- Use the proper order for CSS selectors (now there is only problem with single element and single class selectors order - copying content from Excel does not seems to create more complex selectors).
- If the rule already exists, overwrite it.
Change History (6)
comment:1 Changed 8 years ago by
Status: | new → confirmed |
---|
comment:2 Changed 8 years ago by
Owner: | set to kkrzton |
---|---|
Status: | confirmed → assigned |
comment:3 Changed 8 years ago by
Status: | assigned → review |
---|
comment:4 Changed 8 years ago by
Milestone: | CKEditor 4.7.0 → CKEditor 4.7.1 |
---|
Moving to 4.7.1. If it can be fixed with very little efforts it's still fine to do it in 4.7.0, but it's not a crucial ticket.
comment:5 Changed 8 years ago by
It should be possible to include it in 4.7.0 - see https://github.com/ckeditor/ckeditor-dev/pull/348#issuecomment-297005563.
comment:6 Changed 8 years ago by
Milestone: | CKEditor 4.7.1 → CKEditor 4.7.0 |
---|---|
Resolution: | → fixed |
Status: | review → closed |
Merged to 4.7.0 with git:dec0800.
https://github.com/ckeditor/ckeditor-dev/pull/348