[webkit-reviews] review requested: [Bug 121415] The size of the background colour is not properly adjusted when the font size is being changed : [Attachment 223231] Updating solution for the font size and backgroundcolor changes

[bugzilla-daemon at webkit.org]

Artur Moryc <a.moryc at samsung.com> has asked  for review:
Bug 121415: The size of the background colour is not properly adjusted when the
font size is being changed
https://bugs.webkit.org/show_bug.cgi?id=121415

Attachment 223231: Updating solution for the font size and backgroundcolor
changes 
https://bugs.webkit.org/attachment.cgi?id=223231&action=review

------- Additional Comments from Artur Moryc <a.moryc at samsung.com>
Response to previous comments:
1.
> Source/WebCore/editing/EditingStyle.cpp:-1608
> -PassRefPtr<CSSValue> backgroundColorInEffect(Node* node)
> -{
>Why are you rewriting this function? This was a perfectly reasonable
implementation.

Please have a look at  Darin Adler's suggestions.  

2. 
>We shouldn't have this outer span with background color.
Yes. I agree, however you were not content if redundant spans were removed. 

Current solution leads to the following results:
1. in the fontsize-varying-backgroundcolor-adjustment.html test there is proper
span and font tags distribution without outer span.
2. in the fontsize-varying-backgroundcolor-adjustment-if-continue-style.html
test when it comes to going from the default fontsize the next font tag is
being created as a child node (why not a sibling node?) but the result is
proper.
<font>
|   <span>
|     style="background-color: rgb(0, 255, 0);"
|     "333"
|   <font>
|     size="2"
|     <span>
|	style="background-color: rgb(0, 255, 0);"
|	"222"

3. in the fontsize-varying-cssBackgroundcolor-adjustment-if-continue-style.html
test when it comes to going from the default fontsize the next font tag is
being created as a child node and extra span tag with background is added 
which 
leads to shadowing the background size by the default fontsize. If there is no
going over the default fontsize then backgroundcolor is being added by css
style to all nodes and it works.     
 <font>
|   class="backGround"
|   "333"
|   <font>
|     size="2"
|     <span>
|	style="background-color: rgb(0, 255, 0);"
|	"222"
|   <font>
|     size="1"
|     <span>
|	style="background-color: rgb(0, 255, 0);"
|	"111<#selection-caret>"

Can you give any hints how to deal with that?