Skip to content

Commit

Permalink
[CSS] color-mix() should respect :visited style to resolve "currentco…
Browse files Browse the repository at this point in the history
…lor"

https://bugs.webkit.org/show_bug.cgi?id=259063
rdar://112419198

Reviewed by Tim Nguyen.

This patch passes the visitedLink parameter down the stack
to correctly resolve "currentcolor".

* LayoutTests/imported/w3c/web-platform-tests/css/css-color/color-mix-currentcolor-visited-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-color/color-mix-currentcolor-visited-getcomputedstyle-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-color/color-mix-currentcolor-visited-getcomputedstyle.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-color/color-mix-currentcolor-visited-ref.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-color/color-mix-currentcolor-visited.html: Added.
* Source/WebCore/rendering/style/RenderStyle.cpp:
(WebCore::RenderStyle::colorResolvingCurrentColor const):
* Source/WebCore/rendering/style/RenderStyle.h:
* Source/WebCore/style/StyleBuilderState.cpp:
(WebCore::Style::BuilderState::colorFromPrimitiveValueWithResolvedCurrentColor const): Deleted.
* Source/WebCore/style/StyleBuilderState.h:

Canonical link: https://commits.webkit.org/267271@main
  • Loading branch information
mdubet committed Aug 25, 2023
1 parent f754a3d commit 7b938d0
Show file tree
Hide file tree
Showing 9 changed files with 59 additions and 11 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<!DOCTYPE html>
<html>
<meta charset="utf-8">
<title>currentcolor is taken from :visited pseudo-class correctly in color-mix()</title>
<style>
a { color: green; background-color: color-mix(in srgb, green, white 75%); }
</style>

<a href="unvisited">This background should be green and not red</a>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
This background should be green and not red

PASS Property background-color value 'color-mix(in srgb, currentcolor, white)' should not leak :visited for computed style

Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<!DOCTYPE html>
<meta charset="utf-8">
<title>color-mix and currentcolor with :visited don't leak information via getComputedStyle</title>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/css/support/computed-testcommon.js"></script>

<style>
a:link { color: red; }
a:visited { color: green; }
</style>

<a href=""><span id="target">This background should be green and not red</span></a>

<script>
test_computed_value("background-color", "color-mix(in srgb, currentcolor, white)", "color(srgb 1 0.5 0.5)", "should not leak :visited for computed style");
</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<!DOCTYPE html>
<html>
<meta charset="utf-8">
<title>currentcolor is taken from :visited pseudo-class correctly in color-mix()</title>
<style>
a { color: green; background-color: color-mix(in srgb, green, white 75%); }
</style>

<a href="unvisited">This background should be green and not red</a>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<!DOCTYPE html>
<meta charset="utf-8">
<link rel="match" href="color-mix-currentcolor-visited-ref.html">
<title>currentcolor is taken from :visited pseudo-class correctly in color-mix()</title>
<link rel="help" href="https://drafts.csswg.org/css-color-5/#color-mix">
<link rel="author" title="Matthieu Dubet" href="https://github.com/mdubet">
<style>
a:link { color: red; }
a:visited { color: green; }
span {
background-color: color-mix(in srgb, currentcolor, white 75%);
}
</style>
<a href=""><span>This background should be green and not red</span></a>
6 changes: 3 additions & 3 deletions Source/WebCore/rendering/style/RenderStyle.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2435,12 +2435,12 @@ Color RenderStyle::colorResolvingCurrentColor(CSSPropertyID colorProperty, bool
return visitedLink ? visitedLinkColor() : color();
}

return colorResolvingCurrentColor(result);
return colorResolvingCurrentColor(result, visitedLink);
}

Color RenderStyle::colorResolvingCurrentColor(const StyleColor& color) const
Color RenderStyle::colorResolvingCurrentColor(const StyleColor& color, bool visitedLink) const
{
return color.resolveColor(this->color());
return color.resolveColor(visitedLink ? visitedLinkColor() : this->color());
}

Color RenderStyle::visitedDependentColor(CSSPropertyID colorProperty, OptionSet<PaintBehavior> paintBehavior) const
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/rendering/style/RenderStyle.h
Original file line number Diff line number Diff line change
Expand Up @@ -1741,7 +1741,7 @@ class RenderStyle {
Color colorResolvingCurrentColor(CSSPropertyID colorProperty, bool visitedLink) const;

// Resolves the currentColor keyword, but must not be used for the "color" property which has a different semantic.
WEBCORE_EXPORT Color colorResolvingCurrentColor(const StyleColor&) const;
WEBCORE_EXPORT Color colorResolvingCurrentColor(const StyleColor&, bool visitedLink = false) const;

WEBCORE_EXPORT Color visitedDependentColor(CSSPropertyID, OptionSet<PaintBehavior> paintBehavior = { }) const;
WEBCORE_EXPORT Color visitedDependentColorWithColorFilter(CSSPropertyID, OptionSet<PaintBehavior> paintBehavior = { }) const;
Expand Down
5 changes: 0 additions & 5 deletions Source/WebCore/style/StyleBuilderState.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -159,11 +159,6 @@ StyleColor BuilderState::colorFromPrimitiveValue(const CSSPrimitiveValue& value,
return { WebCore::Style::colorFromPrimitiveValue(document(), m_style, value, forVisitedLink) };
}

Color BuilderState::colorFromPrimitiveValueWithResolvedCurrentColor(const CSSPrimitiveValue& value) const
{
return WebCore::Style::colorFromPrimitiveValueWithResolvedCurrentColor(document(), m_style, value);
}

void BuilderState::registerContentAttribute(const AtomString& attributeLocalName)
{
if (style().styleType() == PseudoId::Before || style().styleType() == PseudoId::After)
Expand Down
2 changes: 0 additions & 2 deletions Source/WebCore/style/StyleBuilderState.h
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,6 @@ class BuilderState {

static bool isColorFromPrimitiveValueDerivedFromElement(const CSSPrimitiveValue&);
StyleColor colorFromPrimitiveValue(const CSSPrimitiveValue&, ForVisitedLink = ForVisitedLink::No) const;
// FIXME: Remove. 'currentcolor' should be resolved at use time. All call sites are broken with inheritance.
Color colorFromPrimitiveValueWithResolvedCurrentColor(const CSSPrimitiveValue&) const;

const Vector<AtomString>& registeredContentAttributes() const { return m_registeredContentAttributes; }
void registerContentAttribute(const AtomString& attributeLocalName);
Expand Down

0 comments on commit 7b938d0

Please sign in to comment.