Opened 15 years ago
Closed 14 years ago
#7492 closed Bug (fixed)
Overrides of Styles don't work correctly for classes
| Reported by: | Alfonso Martínez de Lizarrondo | Owned by: | Alfonso Martínez de Lizarrondo |
|---|---|---|---|
| Priority: | Normal | Milestone: | CKEditor 3.6.3 |
| Component: | Core : Styles | Version: | |
| Keywords: | Cc: |
Description
Load the XHTML output sample.[br] Select a word an apply for example the Comic font to it. Now select Times New Roman
Expected results:[br] A single span that makes the text appear in Times New Roman.
Actual results:[br] Two nested spans with and the text is shown in Comic.
This is the way that the font styles are defined for that sample:
font_style :
{
element : 'span',
attributes : { 'class' : '#(family)' },
overrides : [ { element : 'span', attributes : { 'class' : /^Font(?:Comic|Courier|Times)$/ } } ]
},
As the "element" is the same one that it's used in the overrides definition it seems to be cause of the problems.
The patch fixes this problem, but there might be better ways to fix it if it was planned in a different way (and there might be other places to review)
Attachments (4)
Change History (15)
comment:1 Changed 15 years ago by
| Status: | new → review |
|---|
Changed 15 years ago by
| Attachment: | 7492.patch added |
|---|
comment:2 Changed 15 years ago by
| Milestone: | → CKEditor 3.6.1 |
|---|---|
| Status: | review → review_passed |
I'm attaching a test for this case.
While we need to work on an overall review on the styles system, the proposed patch seems to properly fix this case. It looks logic "enough".
comment:3 Changed 15 years ago by
| Resolution: | → fixed |
|---|---|
| Status: | review_passed → closed |
Fixed with [6983]
comment:4 Changed 14 years ago by
| Milestone: | CKEditor 3.6.1 |
|---|---|
| Resolution: | fixed |
| Status: | closed → reopened |
comment:5 Changed 14 years ago by
The overrides definitions in the xhtml_output sample page is reverted with [7062].
comment:6 Changed 14 years ago by
| Status: | reopened → review |
|---|
The new patch restores the xhtml sample and fixes #8078 by calling removeOverrides with a new parameter (so it doesn't affect the rest of the code) when the element is a block type so it isn't removed.
This way we avoid nesting spans as the first patches did and we remove the element only when it's an inline and we don't have left over spans without attributes.
comment:8 Changed 14 years ago by
| Status: | review → review_failed |
|---|
comment:9 Changed 14 years ago by
| Status: | review_failed → review |
|---|
Sorry, don't know what I tested before creating the patch but it was obviously wrong.
comment:10 Changed 14 years ago by
| Milestone: | → CKEditor 3.6.3 |
|---|---|
| Status: | review → review_passed |
comment:11 Changed 14 years ago by
| Resolution: | → fixed |
|---|---|
| Status: | review_passed → closed |
Fixed with [7407]

Proposed patch