Commit 9f7c6ac9 authored by Alison Maher's avatar Alison Maher Committed by Commit Bot

Force colors at the end of the cascade

As resolved by the CSSWG [1], we should respect system color rules
set by the author/user in Forced Colors Mode. This CL is the second
of several changes to make this happen.

Currently, in order to force colors in Forced Colors Mode, we revert
the related properties at the author origin. This logic is now removed
from the spec [2].

To accommodate the above changes, the plan is to simulate a revert at
the end of the cascade, but only if the computed value for a given
property is not already a system color and forced-color-adjust is set
to auto.

This CL moves the logic for reverting properties that are affected
by Forced Colors Mode to the end of the cascade using the
ForceColors() method (which was added here [3]).

[1] https://github.com/w3c/csswg-drafts/issues/4178
[2] https://github.com/w3c/csswg-drafts/commit/7c154ddc7f196df9b40a34a5f63bf6d157b1011b
[3] https://chromium-review.googlesource.com/c/chromium/src/+/2376666

Bug: 970285
Change-Id: I5039fdaf5ce6d5917a258e593fb2f527581c72e1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2388991
Commit-Queue: Alison Maher <almaher@microsoft.com>
Reviewed-by: default avatarAnders Hartvoll Ruud <andruud@chromium.org>
Cr-Commit-Position: refs/heads/master@{#804985}
parent 6360e2e3
...@@ -1175,6 +1175,7 @@ ...@@ -1175,6 +1175,7 @@
style_builder_template_args: { style_builder_template_args: {
initial_color: "ComputedStyleInitialValues::InitialBackgroundColor", initial_color: "ComputedStyleInitialValues::InitialBackgroundColor",
}, },
affected_by_forced_colors: true,
valid_for_first_letter: true, valid_for_first_letter: true,
valid_for_cue: true, valid_for_cue: true,
is_background: true, is_background: true,
......
...@@ -323,7 +323,6 @@ void StyleCascade::ApplyCascadeAffecting(CascadeResolver& resolver) { ...@@ -323,7 +323,6 @@ void StyleCascade::ApplyCascadeAffecting(CascadeResolver& resolver) {
LookupAndApply(GetCSSPropertyDirection(), resolver); LookupAndApply(GetCSSPropertyDirection(), resolver);
LookupAndApply(GetCSSPropertyWritingMode(), resolver); LookupAndApply(GetCSSPropertyWritingMode(), resolver);
LookupAndApply(GetCSSPropertyForcedColorAdjust(), resolver);
if (depends_on_cascade_affecting_property_) { if (depends_on_cascade_affecting_property_) {
// We could avoid marking these if this cascade provided a value, but // We could avoid marking these if this cascade provided a value, but
...@@ -544,32 +543,66 @@ void StyleCascade::LookupAndApplyInterpolation(const CSSProperty& property, ...@@ -544,32 +543,66 @@ void StyleCascade::LookupAndApplyInterpolation(const CSSProperty& property,
} }
void StyleCascade::ForceColors() { void StyleCascade::ForceColors() {
// TODO(almaher): Return if the color is already a system color. ComputedStyle* style = state_.Style();
if (!GetDocument().InForcedColorsMode() || if (!GetDocument().InForcedColorsMode() ||
state_.Style()->ForcedColorAdjust() == EForcedColorAdjust::kNone) style->ForcedColorAdjust() == EForcedColorAdjust::kNone)
return; return;
int bg_color_alpha = int bg_color_alpha =
state_.Style() style->VisitedDependentColor(GetCSSPropertyBackgroundColor()).Alpha();
->VisitedDependentColor(GetCSSPropertyBackgroundColor())
.Alpha(); MaybeForceColor(GetCSSPropertyColor());
MaybeForceColor(GetCSSPropertyBackgroundColor());
MaybeForceColor(GetCSSPropertyBorderBottomColor());
MaybeForceColor(GetCSSPropertyBorderLeftColor());
MaybeForceColor(GetCSSPropertyBorderRightColor());
MaybeForceColor(GetCSSPropertyBorderTopColor());
MaybeForceColor(GetCSSPropertyFill());
MaybeForceColor(GetCSSPropertyOutlineColor());
MaybeForceColor(GetCSSPropertyStroke());
MaybeForceColor(GetCSSPropertyTextDecorationColor());
MaybeForceColor(GetCSSPropertyColumnRuleColor());
MaybeForceColor(GetCSSPropertyWebkitTapHighlightColor());
MaybeForceColor(GetCSSPropertyWebkitTextEmphasisColor());
MaybeForceColor(GetCSSPropertyInternalVisitedColor());
MaybeForceColor(GetCSSPropertyInternalVisitedBackgroundColor());
MaybeForceColor(GetCSSPropertyInternalVisitedBorderBottomColor());
MaybeForceColor(GetCSSPropertyInternalVisitedBorderLeftColor());
MaybeForceColor(GetCSSPropertyInternalVisitedBorderRightColor());
MaybeForceColor(GetCSSPropertyInternalVisitedBorderTopColor());
MaybeForceColor(GetCSSPropertyInternalVisitedFill());
MaybeForceColor(GetCSSPropertyInternalVisitedOutlineColor());
MaybeForceColor(GetCSSPropertyInternalVisitedStroke());
MaybeForceColor(GetCSSPropertyInternalVisitedTextDecorationColor());
MaybeForceColor(GetCSSPropertyInternalVisitedColumnRuleColor());
MaybeForceColor(GetCSSPropertyInternalVisitedTextEmphasisColor());
auto* none = CSSIdentifierValue::Create(CSSValueID::kNone);
StyleBuilder::ApplyProperty(GetCSSPropertyTextShadow(), state_, *none);
StyleBuilder::ApplyProperty(GetCSSPropertyBoxShadow(), state_, *none);
if (!style->HasUrlBackgroundImage())
StyleBuilder::ApplyProperty(GetCSSPropertyBackgroundImage(), state_, *none);
// TODO(almaher): Do this for all Forced Colors Mode properties. // Preserve the author/user defined background alpha channel.
CSSPropertyName name(CSSPropertyID::kBackgroundColor); style->SetBackgroundColor(
CSSPropertyRef ref(name, state_.GetDocument()); StyleColor(style->BackgroundColor().ResolveWithAlpha(
DCHECK(ref.IsValid()); style->GetCurrentColor(), WebColorScheme::kLight, bg_color_alpha)));
}
StyleBuilder::ApplyProperty(ref.GetProperty(), state_, void StyleCascade::MaybeForceColor(const CSSProperty& property) {
*GetForcedColorValue(name)); DCHECK(GetDocument().InForcedColorsMode() &&
state_.Style()->ForcedColorAdjust() != EForcedColorAdjust::kNone);
// Preserve the author/user defined background alpha channel. // TODO(almaher): Return early if the current computed color is already a
state_.Style()->SetBackgroundColor( // system color.
StyleColor(state_.Style()->BackgroundColor().ResolveWithAlpha( StyleBuilder::ApplyProperty(
state_.Style()->GetCurrentColor(), WebColorScheme::kLight, property, state_, *GetForcedColorValue(property.GetCSSPropertyName()));
bg_color_alpha)));
} }
const CSSValue* StyleCascade::GetForcedColorValue(CSSPropertyName name) { const CSSValue* StyleCascade::GetForcedColorValue(CSSPropertyName name) {
DCHECK(GetDocument().InForcedColorsMode() &&
state_.Style()->ForcedColorAdjust() != EForcedColorAdjust::kNone);
CascadePriority* p = map_.Find(name, CascadeOrigin::kUserAgent); CascadePriority* p = map_.Find(name, CascadeOrigin::kUserAgent);
if (p) if (p)
return ValueAt(match_result_, p->GetPosition()); return ValueAt(match_result_, p->GetPosition());
...@@ -617,24 +650,12 @@ StyleCascade::TokenSequence::BuildVariableData() { ...@@ -617,24 +650,12 @@ StyleCascade::TokenSequence::BuildVariableData() {
has_font_units_, has_root_font_units_, base_url_, charset_); has_font_units_, has_root_font_units_, base_url_, charset_);
} }
bool StyleCascade::ShouldRevert(const CSSProperty& property,
const CSSValue& value,
CascadeOrigin origin) {
return IsRevert(value) ||
(state_.GetDocument().InForcedColorsMode() &&
state_.Style()->ForcedColorAdjust() != EForcedColorAdjust::kNone &&
property.IsAffectedByForcedColors() &&
!(property.PropertyID() == CSSPropertyID::kBackgroundImage &&
value.MayContainUrl()) &&
origin >= CascadeOrigin::kAuthor);
}
const CSSValue* StyleCascade::Resolve(const CSSProperty& property, const CSSValue* StyleCascade::Resolve(const CSSProperty& property,
const CSSValue& value, const CSSValue& value,
CascadeOrigin origin, CascadeOrigin origin,
CascadeResolver& resolver) { CascadeResolver& resolver) {
DCHECK(!property.IsSurrogate()); DCHECK(!property.IsSurrogate());
if (ShouldRevert(property, value, origin)) if (IsRevert(value))
return ResolveRevert(property, value, origin, resolver); return ResolveRevert(property, value, origin, resolver);
resolver.CollectAuthorFlags(property, origin); resolver.CollectAuthorFlags(property, origin);
if (const auto* v = DynamicTo<CSSCustomPropertyDeclaration>(value)) if (const auto* v = DynamicTo<CSSCustomPropertyDeclaration>(value))
......
...@@ -209,6 +209,7 @@ class CORE_EXPORT StyleCascade { ...@@ -209,6 +209,7 @@ class CORE_EXPORT StyleCascade {
// Update the ComputedStyle to use the colors specified in Forced Colors Mode. // Update the ComputedStyle to use the colors specified in Forced Colors Mode.
// https://www.w3.org/TR/css-color-adjust-1/#forced // https://www.w3.org/TR/css-color-adjust-1/#forced
void ForceColors(); void ForceColors();
void MaybeForceColor(const CSSProperty& property);
const CSSValue* GetForcedColorValue(CSSPropertyName name); const CSSValue* GetForcedColorValue(CSSPropertyName name);
// Whether or not we are calculating the style for the root element. // Whether or not we are calculating the style for the root element.
...@@ -345,8 +346,6 @@ class CORE_EXPORT StyleCascade { ...@@ -345,8 +346,6 @@ class CORE_EXPORT StyleCascade {
const Document& GetDocument() const; const Document& GetDocument() const;
const CSSProperty& ResolveSurrogate(const CSSProperty& surrogate); const CSSProperty& ResolveSurrogate(const CSSProperty& surrogate);
bool ShouldRevert(const CSSProperty&, const CSSValue&, CascadeOrigin);
void CountUse(WebFeature); void CountUse(WebFeature);
void MaybeUseCountRevert(const CSSValue&); void MaybeUseCountRevert(const CSSValue&);
void MaybeUseCountSummaryDisplayBlock(); void MaybeUseCountSummaryDisplayBlock();
......
...@@ -482,6 +482,9 @@ class ComputedStyle : public ComputedStyleBase, ...@@ -482,6 +482,9 @@ class ComputedStyle : public ComputedStyleBase,
bool HasBackgroundImage() const { bool HasBackgroundImage() const {
return BackgroundInternal().AnyLayerHasImage(); return BackgroundInternal().AnyLayerHasImage();
} }
bool HasUrlBackgroundImage() const {
return BackgroundInternal().AnyLayerHasUrlImage();
}
bool HasFixedAttachmentBackgroundImage() const { bool HasFixedAttachmentBackgroundImage() const {
return BackgroundInternal().AnyLayerHasFixedAttachmentImage(); return BackgroundInternal().AnyLayerHasFixedAttachmentImage();
} }
......
...@@ -79,6 +79,7 @@ FillLayer::FillLayer(EFillLayerType type, bool use_initial_values) ...@@ -79,6 +79,7 @@ FillLayer::FillLayer(EFillLayerType type, bool use_initial_values)
layers_clip_max_(0), layers_clip_max_(0),
any_layer_uses_content_box_(false), any_layer_uses_content_box_(false),
any_layer_has_image_(false), any_layer_has_image_(false),
any_layer_has_url_image_(false),
any_layer_has_local_attachment_image_(false), any_layer_has_local_attachment_image_(false),
any_layer_has_fixed_attachment_image_(false), any_layer_has_fixed_attachment_image_(false),
any_layer_has_default_attachment_image_(false), any_layer_has_default_attachment_image_(false),
...@@ -116,6 +117,7 @@ FillLayer::FillLayer(const FillLayer& o) ...@@ -116,6 +117,7 @@ FillLayer::FillLayer(const FillLayer& o)
layers_clip_max_(0), layers_clip_max_(0),
any_layer_uses_content_box_(false), any_layer_uses_content_box_(false),
any_layer_has_image_(false), any_layer_has_image_(false),
any_layer_has_url_image_(false),
any_layer_has_local_attachment_image_(false), any_layer_has_local_attachment_image_(false),
any_layer_has_fixed_attachment_image_(false), any_layer_has_fixed_attachment_image_(false),
any_layer_has_default_attachment_image_(false), any_layer_has_default_attachment_image_(false),
...@@ -346,6 +348,8 @@ void FillLayer::ComputeCachedProperties() const { ...@@ -346,6 +348,8 @@ void FillLayer::ComputeCachedProperties() const {
any_layer_uses_content_box_ = any_layer_uses_content_box_ =
Clip() == EFillBox::kContent || Origin() == EFillBox::kContent; Clip() == EFillBox::kContent || Origin() == EFillBox::kContent;
any_layer_has_image_ = !!GetImage(); any_layer_has_image_ = !!GetImage();
any_layer_has_url_image_ =
any_layer_has_image_ && GetImage()->CssValue()->MayContainUrl();
any_layer_has_local_attachment_image_ = any_layer_has_local_attachment_image_ =
any_layer_has_image_ && Attachment() == EFillAttachment::kLocal; any_layer_has_image_ && Attachment() == EFillAttachment::kLocal;
any_layer_has_fixed_attachment_image_ = any_layer_has_fixed_attachment_image_ =
...@@ -360,6 +364,7 @@ void FillLayer::ComputeCachedProperties() const { ...@@ -360,6 +364,7 @@ void FillLayer::ComputeCachedProperties() const {
EnclosingFillBox(LayersClipMax(), next_->LayersClipMax())); EnclosingFillBox(LayersClipMax(), next_->LayersClipMax()));
any_layer_uses_content_box_ |= next_->any_layer_uses_content_box_; any_layer_uses_content_box_ |= next_->any_layer_uses_content_box_;
any_layer_has_image_ |= next_->any_layer_has_image_; any_layer_has_image_ |= next_->any_layer_has_image_;
any_layer_has_url_image_ |= next_->any_layer_has_url_image_;
any_layer_has_local_attachment_image_ |= any_layer_has_local_attachment_image_ |=
next_->any_layer_has_local_attachment_image_; next_->any_layer_has_local_attachment_image_;
any_layer_has_fixed_attachment_image_ |= any_layer_has_fixed_attachment_image_ |=
......
...@@ -231,6 +231,10 @@ class CORE_EXPORT FillLayer { ...@@ -231,6 +231,10 @@ class CORE_EXPORT FillLayer {
ComputeCachedPropertiesIfNeeded(); ComputeCachedPropertiesIfNeeded();
return any_layer_has_image_; return any_layer_has_image_;
} }
bool AnyLayerHasUrlImage() const {
ComputeCachedPropertiesIfNeeded();
return any_layer_has_url_image_;
}
bool AnyLayerHasLocalAttachmentImage() const { bool AnyLayerHasLocalAttachmentImage() const {
ComputeCachedPropertiesIfNeeded(); ComputeCachedPropertiesIfNeeded();
return any_layer_has_local_attachment_image_; return any_layer_has_local_attachment_image_;
...@@ -337,6 +341,8 @@ class CORE_EXPORT FillLayer { ...@@ -337,6 +341,8 @@ class CORE_EXPORT FillLayer {
mutable unsigned any_layer_uses_content_box_ : 1; mutable unsigned any_layer_uses_content_box_ : 1;
// True if any of this or subsequent layers has image. // True if any of this or subsequent layers has image.
mutable unsigned any_layer_has_image_ : 1; mutable unsigned any_layer_has_image_ : 1;
// True if any of this of subsequent layers has a url() image.
mutable unsigned any_layer_has_url_image_ : 1;
// True if any of this or subsequent layers has local attachment image. // True if any of this or subsequent layers has local attachment image.
mutable unsigned any_layer_has_local_attachment_image_ : 1; mutable unsigned any_layer_has_local_attachment_image_ : 1;
// True if any of this or subsequent layers has fixed attachment image. // True if any of this or subsequent layers has fixed attachment image.
......
...@@ -5702,8 +5702,14 @@ crbug.com/991243 [ Linux ] external/wpt/workers/semantics/multiple-workers/003.h ...@@ -5702,8 +5702,14 @@ crbug.com/991243 [ Linux ] external/wpt/workers/semantics/multiple-workers/003.h
crbug.com/970285 external/wpt/forced-colors-mode/* [ Failure ] crbug.com/970285 external/wpt/forced-colors-mode/* [ Failure ]
# Virtual forced colors mode tests should fail on macOS since macOS high # Virtual forced colors mode tests should fail on macOS since macOS high
# contrast does not imply forced colors mode. # contrast does not imply forced colors mode. The tests should pass on all other
# platforms.
crbug.com/970285 [ Mac ] virtual/forced-high-contrast-colors/* [ Failure ] crbug.com/970285 [ Mac ] virtual/forced-high-contrast-colors/* [ Failure ]
crbug.com/970285 [ Win ] virtual/forced-high-contrast-colors/* [ Pass ]
crbug.com/970285 [ Linux ] virtual/forced-high-contrast-colors/* [ Pass ]
crbug.com/970285 [ Android ] virtual/forced-high-contrast-colors/* [ Pass ]
crbug.com/1081945 virtual/forced-high-contrast-colors/fast/css/forced-colors-mode/forced-colors-mode-13.html [ Crash ]
# Expected failures for dark mode tests when the corresponding flags # Expected failures for dark mode tests when the corresponding flags
# are not enabled. # are not enabled.
......
...@@ -5,20 +5,23 @@ ...@@ -5,20 +5,23 @@
<link rel="help" href="https://www.w3.org/TR/css-color-adjust-1/#forced-colors-properties"> <link rel="help" href="https://www.w3.org/TR/css-color-adjust-1/#forced-colors-properties">
<link rel=match href="forced-colors-mode-28-ref.html"> <link rel=match href="forced-colors-mode-28-ref.html">
<style> <style>
:root {
--url-image: url("resources/test-image.jpg");
}
div { div {
height: 20px; height: 20px;
width: 10px; width: 10px;
} }
#div1 { #div1 {
background-image: url("resources/test-image.jpg"); background-image: var(--url-image);
background-image: linear-gradient(red, yellow); background-image: linear-gradient(red, yellow);
} }
#div2 { #div2 {
background-image: linear-gradient(red, yellow); background-image: linear-gradient(red, yellow);
background-image: url("resources/test-image.jpg"); background-image: var(--url-image);
} }
#div3 { #div3 {
background-image: url("resources/test-image.jpg"); background-image: var(--url-image);
} }
#div4 { #div4 {
background-image: linear-gradient(red, yellow); background-image: linear-gradient(red, yellow);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment