[go: nahoru, domu]
Skip to content

Commit

Permalink
Highlight pseudos use the forced-color status of their originating el…
Browse files Browse the repository at this point in the history 
…ement

Per the resolution in [1], forced-color-adjust should not be one of the
supported properties [2] in highlight pseudos, and the forced-color
state of highlights should be taken from the originating element. This
change updates the HighlightInheritance implementation to match the
resolution.

There are a few parts to this:
- css_properties.json5 is updated so that forced-color-adjust is no
  longer a valid property for highlight pseudos. But, since shipping
  this change for ::selection may cause compatibility issues, we
  introduce a new valid_for_highlight_legacy parameter that maintains
  the old behavior and we update valid_for_highlight, which will be used
  only for highlights with the new inheritance model enabled, to use the
  new behavior.
- This uncovered an older bug where DetermineValidPropertyFilter wasn't
  using any filter at all for ::highlight(), ::spelling-error, and
  ::grammar-error. This is now fixed.
- StyleResolver::ApplyInheritance is updated to propagate forced-colors
  status from the originating element to the corresponding highlight
  pseudo.

The new test is disabled on Mac/Linux due to some small differences
in text decoration painting between those from highlight pseudos
and from non-highlight rules, unrelated to the contents of this change.

[1] w3c/csswg-drafts#7264 (comment)
[2] https://drafts.csswg.org/css-pseudo-4/#highlight-styling

Bug: 1309835, 1024156
Change-Id: I732afd24bbba2619a6718270faa0a1d2fcb98512
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3665644
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Delan Azabani <dazabani@igalia.com>
Commit-Queue: Dan Clark <daniec@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#1009639}
NOKEYCHECK=True
GitOrigin-RevId: e3dd4411efafbe9d4c285898e2c195c55612fe89
  • Loading branch information
@dandclark
dandclark authored and Copybara-Service committed Jun 1, 2022
1 parent 8e13519 commit f3d6440
Show file tree
Hide file tree
Showing 13 changed files with 178 additions and 17 deletions.
2 changes: 2 additions & 0 deletions blink/renderer/build/scripts/core/css/css_properties.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ def validate_property(prop, longhands):
'Only longhands can be valid_for_cue [%s]' % name
assert not prop['valid_for_marker'] or prop['is_longhand'], \
'Only longhands can be valid_for_marker [%s]' % name
assert not prop['valid_for_highlight_legacy'] or prop['is_longhand'], \
'Only longhands can be valid_for_highlight_legacy [%s]' % name
assert not prop['valid_for_highlight'] or prop['is_longhand'], \
'Only longhands can be valid_for_highlight [%s]' % name
assert not prop['is_internal'] or prop['computable'] is None, \
Expand Down
1 change: 1 addition & 0 deletions blink/renderer/build/scripts/core/css/properties/templates/css_properties.h.tmpl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ namespace {{namespace}} {
(property.is_highlight_colors and 'kHighlightColors' or ''),
(property.is_visited_highlight_colors and 'kVisitedHighlightColors' or ''),
(property.tree_scoped_value and 'kTreeScopedValue' or ''),
(property.valid_for_highlight_legacy and 'kValidForHighlightLegacy' or ''),
(property.valid_for_highlight and 'kValidForHighlight' or ''),
(property.logical_property_group and 'kInLogicalPropertyGroup' or ''),
] | reject('==', '') | join(' | ') %}
Expand Down
49 changes: 41 additions & 8 deletions blink/renderer/core/css/css_properties.json5
View file
Original file line number Diff line number Diff line change
Expand Up @@ -490,6 +490,18 @@
valid_type: "bool",
},

// - valid_for_highlight_legacy: true
//
// Theoretically matches
// https://drafts.csswg.org/css-pseudo-4/#highlight-styling,
// but includes additional properties for compatibility reasons.
// Used when RuntimeEnabledFeatures::HighlightInheritanceEnabled()
// is not enabled.
valid_for_highlight_legacy: {
default: false,
valid_type: "bool",
},

// - valid_for_highlight: true
//
// https://drafts.csswg.org/css-pseudo-4/#highlight-styling
Expand Down Expand Up @@ -887,6 +899,7 @@
valid_for_first_line: true,
valid_for_cue: true,
valid_for_marker: true,
valid_for_highlight_legacy: true,
valid_for_highlight: true,
valid_for_canvas_formatted_text: true,
valid_for_canvas_formatted_text_run: true,
Expand Down Expand Up @@ -1250,7 +1263,7 @@
keywords: ["auto", "none", "preserve-parent-color"],
typedom_types: ["Keyword"],
default_value: "auto",
valid_for_highlight: true,
valid_for_highlight_legacy: true,
computable: false,
},
{
Expand Down Expand Up @@ -1526,6 +1539,7 @@
valid_for_first_letter: true,
valid_for_first_line: true,
valid_for_cue: true,
valid_for_highlight_legacy: true,
valid_for_highlight: true,
is_background: true,
is_highlight_colors: true,
Expand Down Expand Up @@ -2169,7 +2183,7 @@
style_builder_custom_functions: ["initial", "inherit", "value"],
keywords: ["auto", "currentcolor"],
typedom_types: ["Keyword"],
valid_for_highlight: true,
valid_for_highlight_legacy: true,
},
{
name: "clear",
Expand Down Expand Up @@ -2267,7 +2281,7 @@
type_name: "Vector<AtomicString>",
default_value: "Vector<AtomicString, 0>()",
field_template: "external",
valid_for_highlight: true,
valid_for_highlight_legacy: true,
computable: false,
},
{
Expand Down Expand Up @@ -2410,7 +2424,7 @@
default_value: "auto",
style_builder_custom_functions: ["initial", "inherit", "value"],
typedom_types: ["Keyword"],
valid_for_highlight: true,
valid_for_highlight_legacy: true,
},
{
name: "cx",
Expand Down Expand Up @@ -2503,6 +2517,7 @@
style_builder_template_args: {
initial_color: "ComputedStyleInitialValues::InitialFillPaint",
},
valid_for_highlight_legacy: true,
valid_for_highlight: true,
},
{
Expand Down Expand Up @@ -4265,6 +4280,7 @@
style_builder_template_args: {
initial_color: "ComputedStyleInitialValues::InitialStrokePaint",
},
valid_for_highlight_legacy: true,
valid_for_highlight: true,
},
{
Expand Down Expand Up @@ -4358,6 +4374,7 @@
default_value: "UnzoomedLength(Length::Fixed(1))",
converter: "ConvertUnzoomedLength",
typedom_types: ["Length", "Percentage"],
valid_for_highlight_legacy: true,
valid_for_highlight: true,
},
{
Expand Down Expand Up @@ -4453,6 +4470,7 @@
valid_for_first_letter: true,
valid_for_first_line: true,
valid_for_cue: true,
valid_for_highlight_legacy: true,
valid_for_highlight: true,
supports_incremental_style: true,
valid_for_canvas_formatted_text: true,
Expand All @@ -4471,6 +4489,7 @@
valid_for_first_letter: true,
valid_for_first_line: true,
valid_for_cue: true,
valid_for_highlight_legacy: true,
valid_for_highlight: true,
valid_for_canvas_formatted_text: true,
valid_for_canvas_formatted_text_run: true,
Expand All @@ -4488,6 +4507,7 @@
valid_for_first_line: true,
valid_for_cue: true,
valid_for_marker: true,
valid_for_highlight_legacy: true,
valid_for_highlight: true,
valid_for_canvas_formatted_text: true,
valid_for_canvas_formatted_text_run: true,
Expand All @@ -4503,6 +4523,7 @@
valid_for_first_letter: true,
valid_for_first_line: true,
valid_for_cue: true,
valid_for_highlight_legacy: true,
valid_for_highlight: true,
valid_for_canvas_formatted_text: true,
valid_for_canvas_formatted_text_run: true,
Expand All @@ -4522,6 +4543,7 @@
typedom_types: ["Keyword", "Length", "Percentage"],
valid_for_first_letter: true,
valid_for_first_line: true,
valid_for_highlight_legacy: true,
valid_for_highlight: true,
computable: false,
valid_for_canvas_formatted_text: true,
Expand Down Expand Up @@ -4583,6 +4605,7 @@
valid_for_first_line: true,
valid_for_cue: true,
valid_for_marker: true,
valid_for_highlight_legacy: true,
valid_for_highlight: true,
valid_for_canvas_formatted_text: true,
valid_for_canvas_formatted_text_run: true,
Expand Down Expand Up @@ -5411,6 +5434,7 @@
converter: "ConvertStyleColor",
style_builder_template: "color",
valid_for_marker: true,
valid_for_highlight_legacy: true,
valid_for_highlight: true,
},
{
Expand Down Expand Up @@ -5444,7 +5468,8 @@
computed_style_custom_functions: ["getter"],
converter: "ConvertStyleColor",
style_builder_template: "color",
valid_for_highlight: true,
valid_for_highlight_legacy: true,
valid_for_highlight: true
},
{
name: "-webkit-text-security",
Expand All @@ -5468,7 +5493,7 @@
computed_style_custom_functions: ["getter"],
converter: "ConvertStyleColor",
style_builder_template: "color",
valid_for_highlight: true,
valid_for_highlight_legacy: true,
},
{
name: "-webkit-text-stroke-width",
Expand All @@ -5479,7 +5504,7 @@
default_value: "0",
type_name: "float",
converter: "ConvertTextStrokeWidth",
valid_for_highlight: true,
valid_for_highlight_legacy: true,
},
{
name: "-webkit-transform-origin-x",
Expand Down Expand Up @@ -6890,6 +6915,7 @@
valid_for_first_line: true,
valid_for_cue: true,
valid_for_marker: true,
valid_for_highlight_legacy: true,
valid_for_highlight: true,
is_visited_highlight_colors: true,
},
Expand All @@ -6909,7 +6935,7 @@
style_builder_template_args: {
initial_color: "StyleAutoColor::AutoColor",
},
valid_for_highlight: true,
valid_for_highlight_legacy: true,
},
{
name: "-internal-visited-column-rule-color",
Expand Down Expand Up @@ -6942,6 +6968,7 @@
valid_for_first_letter: true,
valid_for_first_line: true,
valid_for_cue: true,
valid_for_highlight_legacy: true,
valid_for_highlight: true,
is_visited_highlight_colors: true,
},
Expand Down Expand Up @@ -7073,6 +7100,7 @@
style_builder_template_args: {
initial_color: "ComputedStyleInitialValues::InitialFillPaint",
},
valid_for_highlight_legacy: true,
valid_for_highlight: true,
},
{
Expand Down Expand Up @@ -7105,6 +7133,7 @@
style_builder_template_args: {
initial_color: "ComputedStyleInitialValues::InitialStrokePaint",
},
valid_for_highlight_legacy: true,
valid_for_highlight: true,
},
{
Expand All @@ -7122,6 +7151,7 @@
valid_for_first_letter: true,
valid_for_first_line: true,
valid_for_cue: true,
valid_for_highlight_legacy: true,
valid_for_highlight: true,
},
{
Expand All @@ -7137,6 +7167,7 @@
computed_style_custom_functions: ["getter", "setter"],
converter: "ConvertStyleColor",
style_builder_template: "visited_color",
valid_for_highlight_legacy: true,
valid_for_highlight: true,
},
{
Expand All @@ -7152,6 +7183,7 @@
computed_style_custom_functions: ["getter", "setter"],
converter: "ConvertStyleColor",
style_builder_template: "visited_color",
valid_for_highlight_legacy: true,
valid_for_highlight: true,
},
{
Expand All @@ -7167,6 +7199,7 @@
computed_style_custom_functions: ["getter", "setter"],
converter: "ConvertStyleColor",
style_builder_template: "visited_color",
valid_for_highlight_legacy: true,
valid_for_highlight: true,
},

Expand Down
13 changes: 8 additions & 5 deletions blink/renderer/core/css/properties/css_property.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class LayoutObject;

class CORE_EXPORT CSSProperty : public CSSUnresolvedProperty {
public:
using Flags = uint32_t;
using Flags = uint64_t;

static const CSSProperty& Get(CSSPropertyID);

Expand Down Expand Up @@ -65,7 +65,6 @@ class CORE_EXPORT CSSProperty : public CSSUnresolvedProperty {
bool IsValidForFirstLine() const { return flags_ & kValidForFirstLine; }
bool IsValidForCue() const { return flags_ & kValidForCue; }
bool IsValidForMarker() const { return flags_ & kValidForMarker; }
bool IsValidForHighlight() const { return flags_ & kValidForHighlight; }
bool IsValidForCanvasFormattedText() const {
return flags_ & kValidForCanvasFormattedText;
}
Expand Down Expand Up @@ -167,8 +166,10 @@ class CORE_EXPORT CSSProperty : public CSSUnresolvedProperty {
kBorderRadius = 1 << 17,
// Set if the property values are tree-scoped references.
kTreeScopedValue = 1 << 18,
// https://drafts.csswg.org/css-pseudo-4/#highlight-styling
kValidForHighlight = 1 << 19,
// Similar to the list at
// https://drafts.csswg.org/css-pseudo-4/#highlight-styling, with some
// differences for compatibility reasons.
kValidForHighlightLegacy = 1 << 19,
// https://drafts.csswg.org/css-logical/#logical-property-group
kInLogicalPropertyGroup = 1 << 20,
// https://drafts.csswg.org/css-pseudo-4/#first-line-styling
Expand All @@ -195,7 +196,9 @@ class CORE_EXPORT CSSProperty : public CSSUnresolvedProperty {
// See valid_for_keyframes in css_properties.json5
kValidForKeyframe = 1 << 30,
// See valid_for_position_fallback in css_properties.json5
kValidForPositionFallback = 1u << 31,
kValidForPositionFallback = 1ull << 31,
// https://drafts.csswg.org/css-pseudo-4/#highlight-styling
kValidForHighlight = 1ull << 32,
};

constexpr CSSProperty(CSSPropertyID property_id,
Expand Down
3 changes: 2 additions & 1 deletion blink/renderer/core/css/properties/longhands/variable.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ class CORE_EXPORT Variable : public Longhand {
explicit constexpr Variable(CSSProperty::Flags flags)
: Longhand(CSSPropertyID::kVariable,
kProperty | kValidForFirstLetter | kValidForFirstLine |
kValidForMarker | kValidForHighlight | flags,
kValidForMarker | kValidForHighlightLegacy |
kValidForHighlight | flags,
'\0') {}
};

Expand Down
2 changes: 2 additions & 0 deletions blink/renderer/core/css/resolver/cascade_expansion.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ CascadeFilter AddValidPropertiesFilter(
return filter.Add(CSSProperty::kValidForFirstLine, false);
case ValidPropertyFilter::kMarker:
return filter.Add(CSSProperty::kValidForMarker, false);
case ValidPropertyFilter::kHighlightLegacy:
return filter.Add(CSSProperty::kValidForHighlightLegacy, false);
case ValidPropertyFilter::kHighlight:
return filter.Add(CSSProperty::kValidForHighlight, false);
}
Expand Down
24 changes: 23 additions & 1 deletion blink/renderer/core/css/resolver/cascade_expansion_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -479,13 +479,35 @@ TEST_F(CascadeExpansionTest, FilterMarker) {
EXPECT_EQ(CSSPropertyID::kFontSize, e[0]->ref.GetProperty().PropertyID());
}

TEST_F(CascadeExpansionTest, FilterHighlightLegacy) {
MatchResult result;
result.FinishAddingUARules();
result.FinishAddingUserRules();
result.FinishAddingPresentationalHints();
result.AddMatchedProperties(
ParseDeclarationBlock("display:block;background-color:lime;forced-color-adjust:none"),
AddMatchedPropertiesOptions::Builder()
.SetValidPropertyFilter(ValidPropertyFilter::kHighlightLegacy)
.Build());
result.FinishAddingAuthorRulesForTreeScope(GetDocument());

auto e = ExpansionAt(result, 0);
ASSERT_EQ(3u, e.size());
EXPECT_EQ(CSSPropertyID::kBackgroundColor,
e[0]->ref.GetProperty().PropertyID());
EXPECT_EQ(CSSPropertyID::kInternalVisitedBackgroundColor,
e[1]->ref.GetProperty().PropertyID());
EXPECT_EQ(CSSPropertyID::kForcedColorAdjust,
e[2]->ref.GetProperty().PropertyID());
}

TEST_F(CascadeExpansionTest, FilterHighlight) {
MatchResult result;
result.FinishAddingUARules();
result.FinishAddingUserRules();
result.FinishAddingPresentationalHints();
result.AddMatchedProperties(
ParseDeclarationBlock("display:block;background-color:lime;"),
ParseDeclarationBlock("display:block;background-color:lime;forced-color-adjust:none"),
AddMatchedPropertiesOptions::Builder()
.SetValidPropertyFilter(ValidPropertyFilter::kHighlight)
.Build());
Expand Down
14 changes: 12 additions & 2 deletions blink/renderer/core/css/resolver/style_resolver.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -967,6 +967,10 @@ void StyleResolver::ApplyInheritance(Element& element,

state.SetStyle(ComputedStyle::Clone(*state.ParentStyle()));
state.Style()->SetInsideLink(state.ElementLinkState());
state.Style()->SetInForcedColorsMode(
style_request.originating_element_style->InForcedColorsMode());
state.Style()->SetForcedColorAdjust(
style_request.originating_element_style->ForcedColorAdjust());
} else {
scoped_refptr<ComputedStyle> style = CreateComputedStyle();
style->InheritFrom(
Expand Down Expand Up @@ -1675,8 +1679,14 @@ bool StyleResolver::ApplyAnimatedStyle(StyleResolverState& state,
CascadeFilter filter;
if (state.Style()->StyleType() == kPseudoIdMarker)
filter = filter.Add(CSSProperty::kValidForMarker, false);
if (IsHighlightPseudoElement(state.Style()->StyleType()))
filter = filter.Add(CSSProperty::kValidForHighlight, false);
if (IsHighlightPseudoElement(state.Style()->StyleType())) {
if (RuntimeEnabledFeatures::HighlightInheritanceEnabled() ||
state.Style()->StyleType() == PseudoId::kPseudoIdHighlight) {
filter = filter.Add(CSSProperty::kValidForHighlight, false);
} else {
filter = filter.Add(CSSProperty::kValidForHighlightLegacy, false);
}
}
filter = filter.Add(CSSProperty::kAnimation, true);

cascade.Apply(filter);
Expand Down
Loading

0 comments on commit f3d6440

Please sign in to comment.