Bug 126296

Summary: Remove hardcoded composition background color; use RenderStyle::compositionFillColor()
Product: WebKit Reporter: Daniel Bates <dbates>
Component: WebCore Misc.Assignee: Sam Weinig <sam>
Status: RESOLVED WONTFIX    
Severity: Normal CC: enrica, sam, svillar
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch darin: review+

Description Daniel Bates 2013-12-30 10:23:37 PST
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.
Comment 1 Sam Weinig 2014-01-25 21:16:54 PST
Created attachment 222250 [details]
Patch
Comment 2 Sam Weinig 2014-01-25 21:18:06 PST
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.
Comment 3 Darin Adler 2014-01-27 10:01:55 PST
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.
Comment 4 Enrica Casucci 2014-09-10 17:35:10 PDT
The property has been removed in revision 173499.
This patch is no longer applicable.