| 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
Daniel Bates
2013-12-30 10:23:37 PST
Created attachment 222250 [details]
Patch
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 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. The property has been removed in revision 173499. This patch is no longer applicable. |