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

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has denied Artur Moryc <a.moryc at samsung.com>'s
request 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 223336: Updating solution for backgroundcolor and fontsize issue 
https://bugs.webkit.org/attachment.cgi?id=223336&action=review

------- Additional Comments from Ryosuke Niwa <rniwa at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=223336&action=review


Please review our contribution guide:
http://www.webkit.org/coding/contributing.html
and address my review comments.

Keep posting patches wouldn't change my mind as long as they have the same
problems I've pointed out thus far.

>
LayoutTests/editing/inserting/fontsize-varying-cssBackgroundcolor-adjustment-if
-continue-style-expected.txt:23
> +|   <font>
> +|	 size="2"
> +|	 <span>
> +|	   style="background-color: rgb(0, 255, 0);"
> +|	   "222"

We shouldn't be creating a nested font/span structure like that.
"background-color" property should have been added on the font element.

> Source/WebCore/editing/EditingStyle.cpp:1506
> +	   color = style->visitedDependentColor(CSSPropertyBackgroundColor);

Calling visitedDependentColor here is WRONG as I have repeatedly pointed out.
visited dependent color leaks history information so we can't do this. r-.