Unable to change font-size

[Karen Ananiev]

1. Open an example and select all text.
   
2. Set font-size to 20.
   
3. Select any word by double click.
   
4. Set font-size to 14.
   
5. Set font-size to 20 -> font-size will not be changed.

[Charlie]

The problem seems to be more generally a problem with in-line styles. When an embedded inline style is applied, I.E. <style 1>text <style 2>text</style 2> text </style 1>

The first attempt to change the inner style only serves to change it to the following: <style 1>text </style 1><style 2>text</style 2><style 1> text </style 1>

Then a second attempt to apply style one (instead of two) to the inner span yields the appropriate: <style 1>text text text </style 1>

The two possible solutions are: 1) Initally create the split span tags instead of creating embedded spans with conflicting style properties 2) Have it recognize that instead of just splitting the outer style, it needs to re-look at the style we want to apply and potentially re-merge everything.

This is all from just playing around, hopefully it'll give someone ideas. I'll try to delve deeper and see if I can't figure out more

[Charlie]

maybe?

[Charlie]

Ok, have a look at the patch. The problem was that when the element we were altering was compared to the parent, CKEditor realized that this entire style element didn't need to be there (because of the parents) and stopped trying to 'add' the style.

The problem with this however, is that the style element already existed. So when it stopped, the element simply stayed as it was. I changed this to force it to add the blank element, thus replacing the old element, then remove the blank element.

There might be a more elequent way to do this as Iäm not familiar with all the commands available to me, so please let me know if you think this works or how it could be better!

[Tobiasz Cudnik]

Thank you for the patch, we will review it ASAP.

[Frederico Caldeira Knabben]

The review keywords are reserved to the core team. HasPatch must be used in these cases.

[Tobiasz Cudnik]

Patch proposed before didn't resolve the issue in case of deeper nesting (step 4 repeated more than once).

[Sa'ar Zac Elias]

With this patch I'm getting this code:

<p>
	<span style="font-size: 20px;">This is <span style="font-size: 20px;">some</span> <strong>sample text</strong>. You are using <a href="http://ckeditor.com/">CKEditor</a>.</span></p>

Notice that there is a nested redundant span. In this case the inner span should be removed.

[Tobiasz Cudnik]

Added garbage collection of selected style and fixed styles and attributes inheritance direction.

[Garry Yao]

For me we're just missing one condition block to cover the case described here.

[Tobiasz Cudnik]

​5931_4.patch fails for following TC:

• Use content below

  <p>
  	<span style="font-size: 20px;">lorem ipsum <span style="font-size: 26px;">lorem FOO lorem</span> lorem ipsum</span></p>
  
  

• Change font size of "FOO" to 20px

Result: No change.

I'm opting for another review of ​5931_3.patch which passes this TC.

[Garry Yao]

How about combining these two patches which looks good for me.

[Tobiasz Cudnik]

I think it's a successful merge which reuses existing mechanisms and reduces the code amount, while achieving the same goal.

[Tobiasz Cudnik]

R+ for 5931_5.patch.

[Garry Yao]

Fixed with [5951].

[Garry Yao]

Basic style application is broken by the change, revert with [5952].

[Garry Yao]

Fixed now with [5961].