Opened 6 years ago

Closed 3 years ago

#7498 closed Bug (fixed)

Font size: nested span tags

Reported by: Wiktor Walc Owned by: Garry Yao
Priority: Normal Milestone:
Component: General Version: 3.0
Keywords: IBM Cc: satya_minnekanti@…

Description

  • Load replacebyclass sample, clear all contents
  • Select font size "12"
  • Select font size "16"
  • Type some text
  • Result:
    <span style="font-size: 12px;"><span style="font-size: 16px;">Some text</span></span>
    
  • Expected result:
    <span style="font-size: 16px;">Some text</span>
    

Note: it is a bit different case than the one reported in #2203

Attachments (3)

7498.patch (4.4 KB) - added by Garry Yao 6 years ago.
7498_2.patch (4.4 KB) - added by Garry Yao 6 years ago.
7498_3.patch (6.6 KB) - added by Garry Yao 6 years ago.

Download all attachments as: .zip

Change History (15)

comment:1 Changed 6 years ago by Alfonso Martínez de Lizarrondo

In a first test it seems that #7492 also fixes this problem (after all is a problem with the overrides behavior)

comment:2 Changed 6 years ago by Satya Minnekanti

Cc: satya_minnekanti@… added
Keywords: IBM added

Same issue happens with Font Names as well

comment:3 Changed 6 years ago by Jakub Ś

Status: newconfirmed

This is true for IE and Firefox since CKEditor version 3.0 and Opera 11.01 since CKEditor version 3.4.1

In Webkit till version 3.5.2 stable (inclusive) choosing font size forr no text selected had no effect. This progress/regress has first occurred in CKEditor revision [6461]

Changed 6 years ago by Garry Yao

Attachment: 7498.patch added

comment:4 Changed 6 years ago by Garry Yao

Owner: set to Garry Yao
Status: confirmedreview

comment:5 Changed 6 years ago by Frederico Caldeira Knabben

Status: reviewreview_failed

While the resulting output is correct, we're introducing a new behavior to the editor when removing font and color styles, which is not consistent with other styles, like bold and italic.

For example, with the following HTML and collapsed selection:

<strong><span style="font-size:16px;">This is |a test.</span></strong>
  • Click the Bold button: it's removed from the entire paragraph.
  • Click the Font Size combo and select "16": it's removed from the selection only, breaking the <span> tag.

We must have a consistent behavior here. Chancing the current way is not an option nor the scope of this ticket.

Instead, we must make it in a way that it splits tags only when overriding (when splitOnRemove), but not for the style to be applied. Would it be possible?

Changed 6 years ago by Garry Yao

Attachment: 7498_2.patch added

comment:6 Changed 6 years ago by Garry Yao

Status: review_failedreview

Ticket TC added with [6935].

comment:7 Changed 6 years ago by Frederico Caldeira Knabben

Status: reviewreview_failed

Now we have a similar issue with the color. Just load the following HTML and selection:

<span style="background-color:#ffff00;">This is some |sample text.</span>

Select that same background color (yellow). A new span with the same color will be create, instead of removing the color (current behavior).

In any case, I see that a explicit remove() call is done in the font plugin. This should not be needed, considering that the font to be applied *should* already remove conflicting styles (which doesn't happen).

Changed 6 years ago by Garry Yao

Attachment: 7498_3.patch added

comment:8 in reply to:  7 Changed 6 years ago by Garry Yao

Status: review_failedreview

Replying to fredck:

Now we have a similar issue with the color...A new span with the same color will be create, instead of removing the color (current behavior)...

The current behavior is incomplete, it's rational for this specific case, while it will also remove the whole color element when a different style (color) is applied.

the font to be applied *should* already remove conflicting styles (which doesn't happen)...

I'm not sure if u mean by incorporate the "remove" into style::apply, but that's not the fact, as it differs from style to style on how a "conflicting style" is defined, e.g. a tag based style (e.g. bold) is conflicting with itself, while a CSS style based style (e.g. font) is conflicted on the same CSS property. So it looks nature to rely on the style applier to roll out the removal.

comment:9 Changed 6 years ago by Garry Yao

Another way of the fix would be #8045, which doesn't require an explicit remove call from the plugin itself, just mark the style itself as overrides on definition.

comment:10 Changed 4 years ago by Jakub Ś

#8125 was marked as duplicate.

comment:11 Changed 4 years ago by Jakub Ś

This ticket can be considered as part of one large issue mentioned in #5503.

comment:12 Changed 3 years ago by Piotrek Koszuliński

Resolution: fixed
Status: reviewclosed

Fixed in 4.4.6.

Note: See TracTickets for help on using tickets.
© 2003 – 2017 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy