Bug 126296 – Remove hardcoded composition background color; use RenderStyle::compositionFillColor()

• Component: WebCore Misc.
• Status: RESOLVED WONTFIX

[Daniel Bates]

We should remove the hardcoded composition background color in InlineTextBox::paintCompositionBackground(), <http://trac.webkit.org/browser/trunk/Source/WebCore/rendering/InlineTextBox.cpp?rev=160951#L792> used by non-iOS ports. Instead we should always query RenderStyle::compositionFillColor() for this color. We may also need to support a platform-specific default composition background color.

[Sam Weinig]

Created attachment 222250 [details]
Patch

[Sam Weinig]

I'm not convinced -webkit-composition-fill-color is needed at all, but if it is, we should support it everywhere.  If not, we should keep the RenderTheme part of this patch and remove the rest.

[Darin Adler]

Comment on attachment 222250 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=222250&action=review

> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:2640
> +        case CSSPropertyWebkitCompositionFillColor: {
> +            if (!style->compositionFillColor().isValid())
> +                return cssValuePool().createColorValue(m_node->document().renderView()->theme().compositionFillColor().rgb());
> +            return cssValuePool().createColorValue(style->compositionFillColor().rgb());
> +        }

No need for these braces.

I can’t find any other code in this function that dereferences renderView(); do we have a solid guarantee that it’s non-null?

Is there a good reason to use m_node->document() rather than styledNode->document()?

> Source/WebCore/platform/graphics/Color.h:-156
> -    static const RGBA32 tap = 0x4D1A1A1A;

Obviously didn’t belong here, but where is the tap color now?

> Source/WebCore/platform/graphics/Color.h:156
>  
>  private:

Looks like we are leaving an extra blank line behind here.

> Source/WebCore/rendering/style/RenderStyle.h:1446
> +    void setCompositionFillColor(const Color &c) { SET_VAR(rareInheritedData, compositionFillColor, c); }

Move the "&" next to Color, please.

[Enrica Casucci]

The property has been removed in revision 173499.
This patch is no longer applicable.