[webkit-reviews] review granted: [Bug 223181] Fix interpolation of the caret-color CSS property : [Attachment 423172] Patch

[bugzilla-daemon at webkit.org]

Antti Koivisto <koivisto at iki.fi> has granted  review:
Bug 223181: Fix interpolation of the caret-color CSS property
https://bugs.webkit.org/show_bug.cgi?id=223181

Attachment 423172: Patch

https://bugs.webkit.org/attachment.cgi?id=423172&action=review




--- Comment #3 from Antti Koivisto <koivisto at iki.fi> ---
Comment on attachment 423172
  --> https://bugs.webkit.org/attachment.cgi?id=423172
Patch

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

>> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:2788
>> +		    return cssValuePool.createIdentifierValue(CSSValueAuto);
> 
> Please don't expand visited link support!

Sorry, I misread this. This just keeps the existing support.

> Source/WebCore/style/StyleBuilderCustom.h:830
> +inline void BuilderCustom::applyInitialCaretColor(BuilderState&
builderState)
> +{
> +    if (builderState.applyPropertyToRegularStyle())
> +	   builderState.style().setHasAutoCaretColor();
> +    if (builderState.applyPropertyToVisitedLinkStyle())
> +	   builderState.style().setHasVisitedLinkAutoCaretColor();
> +}
> +
> +inline void BuilderCustom::applyInheritCaretColor(BuilderState&
builderState)
> +{
> +    Color color = builderState.parentStyle().caretColor();
> +    if (builderState.parentStyle().hasAutoCaretColor())
> +	   builderState.style().setHasAutoCaretColor();
> +    else if (builderState.applyPropertyToRegularStyle())
> +	   builderState.style().setCaretColor(color);
> +    if (builderState.parentStyle().hasVisitedLinkAutoCaretColor())
> +	   builderState.style().setHasVisitedLinkAutoCaretColor();
> +    else if (builderState.applyPropertyToVisitedLinkStyle())
> +	   builderState.style().setVisitedLinkCaretColor(color);
> +}
> +
> +inline void BuilderCustom::applyValueCaretColor(BuilderState& builderState,
CSSValue& value)
> +{
> +    auto& primitiveValue = downcast<CSSPrimitiveValue>(value);
> +    if (primitiveValue.valueID() == CSSValueAuto)
> +	   builderState.style().setHasAutoCaretColor();
> +    else if (builderState.applyPropertyToRegularStyle())
> +	  
builderState.style().setCaretColor(builderState.colorFromPrimitiveValue(primiti
veValue, /* forVisitedLink */ false));
> +    if (primitiveValue.valueID() == CSSValueAuto)
> +	   builderState.style().setHasVisitedLinkAutoCaretColor();
> +    else if (builderState.applyPropertyToVisitedLinkStyle())
> +	  
builderState.style().setVisitedLinkCaretColor(builderState.colorFromPrimitiveVa
lue(primitiveValue, /* forVisitedLink */ true));
> +}

We should really add a separate type ("StyleColor") that can support these sort
of things without having to use custom wrappers or in-band signalling like
currentColor.