Ticket #7498 (review Bug)

Opened 3 years ago

Last modified 16 months ago

Font size: nested span tags

Reported by: wwalc 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

7498.patch (4.4 KB) - added by garry.yao 3 years ago.
7498_2.patch (4.4 KB) - added by garry.yao 3 years ago.
7498_3.patch (6.6 KB) - added by garry.yao 3 years ago.

Change History

comment:1 Changed 3 years ago by alfonsoml

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

comment:2 Changed 3 years ago by satya

  • Cc satya_minnekanti@… added
  • Keywords IBM added

Same issue happens with Font Names as well

comment:3 Changed 3 years ago by j.swiderski

  • Status changed from new to confirmed

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 3 years ago by garry.yao

comment:4 Changed 3 years ago by garry.yao

  • Status changed from confirmed to review
  • Owner set to garry.yao

comment:5 Changed 3 years ago by fredck

  • Status changed from review to review_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 3 years ago by garry.yao

comment:6 Changed 3 years ago by garry.yao

  • Status changed from review_failed to review

Ticket TC added with [6935].

comment:7 follow-up: ↓ 8 Changed 3 years ago by fredck

  • Status changed from review to review_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 3 years ago by garry.yao

comment:8 in reply to: ↑ 7 Changed 3 years ago by garry.yao

  • Status changed from review_failed to review

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 3 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 16 months ago by j.swiderski

#8125 was marked as duplicate.

comment:11 Changed 16 months ago by j.swiderski

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

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