Skip to content

Commit

Permalink
Custom Highlight uses new Highlight Inheritance model by default
Browse files Browse the repository at this point in the history
This change enables Custom Highlights to use the new Highlight
Inheritance model without the need of enabling the new model's feature
flag, and without affecting the behaviour of other types of highlights.

Bug: 1024156,1164461
Change-Id: Ic2231b5700e7bc4fbcbea9f4aa67366e3253637e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3498908
Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: Dan Clark <daniec@microsoft.com>
Reviewed-by: Anders Hartvoll Ruud <andruud@chromium.org>
Reviewed-by: Delan Azabani <dazabani@igalia.com>
Commit-Queue: Luis Sanchez Padilla <lusanpad@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#987061}
  • Loading branch information
Luis Juan Sanchez Padilla authored and Chromium LUCI CQ committed Mar 30, 2022
1 parent 7ff821d commit cc6c0ae
Show file tree
Hide file tree
Showing 10 changed files with 137 additions and 50 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -952,8 +952,9 @@ static bool AllowsInheritance(const StyleRequest& style_request,
void StyleResolver::ApplyInheritance(Element& element,
const StyleRequest& style_request,
StyleResolverState& state) {
if (RuntimeEnabledFeatures::HighlightInheritanceEnabled() &&
IsHighlightPseudoElement(style_request.pseudo_id)) {
if ((RuntimeEnabledFeatures::HighlightInheritanceEnabled() &&
IsHighlightPseudoElement(style_request.pseudo_id)) ||
style_request.pseudo_id == PseudoId::kPseudoIdHighlight) {
// When resolving highlight styles for children, we need to default all
// properties (whether or not defined as inherited) to parent values.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ StyleResolverState::StyleResolverState(
: ElementType::kElement),
nearest_container_(style_recalc_context.container),
is_for_highlight_(IsHighlightPseudoElement(style_request.pseudo_id)),
is_for_custom_highlight_(style_request.pseudo_id ==
PseudoId::kPseudoIdHighlight),
can_cache_base_style_(blink::CanCacheBaseStyle(style_request)) {
DCHECK(!!parent_style_ == !!layout_parent_style_);

Expand All @@ -92,7 +94,8 @@ bool StyleResolverState::IsInheritedForUnset(
const CSSProperty& property) const {
return property.IsInherited() ||
(is_for_highlight_ &&
RuntimeEnabledFeatures::HighlightInheritanceEnabled());
(RuntimeEnabledFeatures::HighlightInheritanceEnabled() ||
is_for_custom_highlight_));
}

void StyleResolverState::SetStyle(scoped_refptr<ComputedStyle> style) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,8 @@ class CORE_EXPORT StyleResolverState {

// True if we are resolving styles for a highlight pseudo-element.
const bool is_for_highlight_;
// True if we are resolving styles for a custom highlight pseudo-element.
const bool is_for_custom_highlight_;

// True if the base style can be cached to optimize style recalculations for
// animation updates or transition retargeting.
Expand Down
83 changes: 45 additions & 38 deletions third_party/blink/renderer/core/dom/element.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3533,47 +3533,52 @@ StyleRecalcChange Element::RecalcOwnStyle(
if (new_style && !ShouldStoreComputedStyle(*new_style))
new_style = nullptr;

if (RuntimeEnabledFeatures::HighlightInheritanceEnabled() && new_style) {
if (new_style) {
const StyleHighlightData* parent_highlights =
parent_style ? parent_style->HighlightData().get() : nullptr;

if (new_style->HasPseudoElementStyle(kPseudoIdSelection)) {
StyleHighlightData& highlights = new_style->MutableHighlightData();
const ComputedStyle* highlight_parent =
parent_highlights ? parent_highlights->Selection() : nullptr;
StyleRequest style_request{kPseudoIdSelection, highlight_parent};
highlights.SetSelection(
StyleForPseudoElement(new_style_recalc_context, style_request));
}
if (RuntimeEnabledFeatures::HighlightInheritanceEnabled()) {
if (new_style->HasPseudoElementStyle(kPseudoIdSelection)) {
StyleHighlightData& highlights = new_style->MutableHighlightData();
const ComputedStyle* highlight_parent =
parent_highlights ? parent_highlights->Selection() : nullptr;
StyleRequest style_request{kPseudoIdSelection, highlight_parent};
highlights.SetSelection(
StyleForPseudoElement(new_style_recalc_context, style_request));
}

if (new_style->HasPseudoElementStyle(kPseudoIdTargetText)) {
StyleHighlightData& highlights = new_style->MutableHighlightData();
const ComputedStyle* highlight_parent =
parent_highlights ? parent_highlights->TargetText() : nullptr;
StyleRequest style_request{kPseudoIdTargetText, highlight_parent};
highlights.SetTargetText(
StyleForPseudoElement(new_style_recalc_context, style_request));
}
if (new_style->HasPseudoElementStyle(kPseudoIdTargetText)) {
StyleHighlightData& highlights = new_style->MutableHighlightData();
const ComputedStyle* highlight_parent =
parent_highlights ? parent_highlights->TargetText() : nullptr;
StyleRequest style_request{kPseudoIdTargetText, highlight_parent};
highlights.SetTargetText(
StyleForPseudoElement(new_style_recalc_context, style_request));
}

if (new_style->HasPseudoElementStyle(kPseudoIdSpellingError)) {
StyleHighlightData& highlights = new_style->MutableHighlightData();
const ComputedStyle* highlight_parent =
parent_highlights ? parent_highlights->SpellingError() : nullptr;
StyleRequest style_request{kPseudoIdSpellingError, highlight_parent};
highlights.SetSpellingError(
StyleForPseudoElement(new_style_recalc_context, style_request));
}
if (new_style->HasPseudoElementStyle(kPseudoIdSpellingError)) {
StyleHighlightData& highlights = new_style->MutableHighlightData();
const ComputedStyle* highlight_parent =
parent_highlights ? parent_highlights->SpellingError() : nullptr;
StyleRequest style_request{kPseudoIdSpellingError, highlight_parent};
highlights.SetSpellingError(
StyleForPseudoElement(new_style_recalc_context, style_request));
}

if (new_style->HasPseudoElementStyle(kPseudoIdGrammarError)) {
StyleHighlightData& highlights = new_style->MutableHighlightData();
const ComputedStyle* highlight_parent =
parent_highlights ? parent_highlights->GrammarError() : nullptr;
StyleRequest style_request{kPseudoIdGrammarError, highlight_parent};
highlights.SetGrammarError(
StyleForPseudoElement(new_style_recalc_context, style_request));
if (new_style->HasPseudoElementStyle(kPseudoIdGrammarError)) {
StyleHighlightData& highlights = new_style->MutableHighlightData();
const ComputedStyle* highlight_parent =
parent_highlights ? parent_highlights->GrammarError() : nullptr;
StyleRequest style_request{kPseudoIdGrammarError, highlight_parent};
highlights.SetGrammarError(
StyleForPseudoElement(new_style_recalc_context, style_request));
}
}

if (new_style->HasPseudoElementStyle(kPseudoIdHighlight)) {
// Use new inheritance model for custom highlights even if it is not enabled
// for other types of highlights.
if (new_style && new_style->HasPseudoElementStyle(kPseudoIdHighlight)) {
const StyleHighlightData* parent_highlights =
parent_style ? parent_style->HighlightData().get() : nullptr;
StyleHighlightData& highlights = new_style->MutableHighlightData();

const HashSet<AtomicString>* custom_highlight_names =
Expand Down Expand Up @@ -6302,8 +6307,9 @@ const ComputedStyle* Element::CachedStyleForPseudoElement(
const AtomicString& pseudo_argument) {
// Highlight pseudos are resolved into StyleHighlightData during originating
// style recalc, and should never be stored in StyleCachedData.
DCHECK(!RuntimeEnabledFeatures::HighlightInheritanceEnabled() ||
!IsHighlightPseudoElement(pseudo_id));
DCHECK((!RuntimeEnabledFeatures::HighlightInheritanceEnabled() ||
!IsHighlightPseudoElement(pseudo_id)) &&
pseudo_id != PseudoId::kPseudoIdHighlight);

const ComputedStyle* style = GetComputedStyle();

Expand All @@ -6329,8 +6335,9 @@ scoped_refptr<ComputedStyle> Element::UncachedStyleForPseudoElement(
const StyleRequest& request) {
// Highlight pseudos are resolved into StyleHighlightData during originating
// style recalc, where we have the actual StyleRecalcContext.
DCHECK(!RuntimeEnabledFeatures::HighlightInheritanceEnabled() ||
!IsHighlightPseudoElement(request.pseudo_id));
DCHECK((!RuntimeEnabledFeatures::HighlightInheritanceEnabled() ||
!IsHighlightPseudoElement(request.pseudo_id)) &&
request.pseudo_id != PseudoId::kPseudoIdHighlight);

return StyleForPseudoElement(
StyleRecalcContext::FromInclusiveAncestors(*this), request);
Expand Down
15 changes: 10 additions & 5 deletions third_party/blink/renderer/core/paint/highlight_painting_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -209,8 +209,10 @@ Color HighlightColor(const Document& document,
pseudo_argument);

mojom::blink::ColorScheme color_scheme = style.UsedColorScheme();
if (pseudo_style && (!RuntimeEnabledFeatures::HighlightInheritanceEnabled() ||
!UseUaHighlightColors(pseudo, *pseudo_style))) {
if (pseudo_style &&
((!RuntimeEnabledFeatures::HighlightInheritanceEnabled() &&
pseudo != PseudoId::kPseudoIdHighlight) ||
!UseUaHighlightColors(pseudo, *pseudo_style))) {
if (!document.InForcedColorsMode() ||
pseudo_style->ForcedColorAdjust() != EForcedColorAdjust::kAuto) {
if (pseudo_style->ColorIsCurrentColor()) {
Expand Down Expand Up @@ -238,7 +240,8 @@ scoped_refptr<const ComputedStyle> HighlightPaintingUtils::HighlightPseudoStyle(
const ComputedStyle& style,
PseudoId pseudo,
const AtomicString& pseudo_argument) {
if (!RuntimeEnabledFeatures::HighlightInheritanceEnabled()) {
if (!RuntimeEnabledFeatures::HighlightInheritanceEnabled() &&
pseudo != PseudoId::kPseudoIdHighlight) {
return HighlightPseudoStyleWithOriginatingInheritance(node, pseudo,
pseudo_argument);
}
Expand Down Expand Up @@ -278,8 +281,10 @@ Color HighlightPaintingUtils::HighlightBackgroundColor(
HighlightPseudoStyle(node, style, pseudo, pseudo_argument);

mojom::blink::ColorScheme color_scheme = style.UsedColorScheme();
if (pseudo_style && (!RuntimeEnabledFeatures::HighlightInheritanceEnabled() ||
!UseUaHighlightColors(pseudo, *pseudo_style))) {
if (pseudo_style &&
((!RuntimeEnabledFeatures::HighlightInheritanceEnabled() &&
pseudo != PseudoId::kPseudoIdHighlight) ||
!UseUaHighlightColors(pseudo, *pseudo_style))) {
if (!document.InForcedColorsMode() ||
pseudo_style->ForcedColorAdjust() != EForcedColorAdjust::kAuto) {
Color highlight_color =
Expand Down
5 changes: 3 additions & 2 deletions third_party/blink/renderer/core/style/computed_style.cc
Original file line number Diff line number Diff line change
Expand Up @@ -204,8 +204,9 @@ static bool PseudoElementStylesEqual(const ComputedStyle& old_style,
continue;
// Highlight pseudo styles are stored in StyleHighlightData, and compared
// like any other inherited field, yielding Difference::kInherited.
if (RuntimeEnabledFeatures::HighlightInheritanceEnabled() &&
IsHighlightPseudoElement(pseudo_id))
if ((RuntimeEnabledFeatures::HighlightInheritanceEnabled() &&
IsHighlightPseudoElement(pseudo_id)) ||
pseudo_id == PseudoId::kPseudoIdHighlight)
continue;
const ComputedStyle* new_pseudo_style =
new_style.GetCachedPseudoElementStyle(pseudo_id);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1255,6 +1255,8 @@
depends_on: ["WebHID"],
status: {"Android": "", "default": "stable"},
},
// HighlightAPI's custom highlights use HighlightInheritance's behavior even
// if the HighlightInheritance feature is not enabled.
{
name: "HighlightAPI",
status: "experimental",
Expand Down
4 changes: 2 additions & 2 deletions third_party/blink/web_tests/TestExpectations
Original file line number Diff line number Diff line change
Expand Up @@ -3247,6 +3247,7 @@ crbug.com/1147859 virtual/css-highlight-overlay-painting/external/wpt/css/css-hi
crbug.com/1147859 virtual/css-highlight-overlay-painting/external/wpt/css/css-highlight-api/painting/custom-highlight-painting-iframe-004.html [ Pass ]
crbug.com/1147859 virtual/css-highlight-overlay-painting/external/wpt/css/css-highlight-api/painting/custom-highlight-painting-iframe-005.html [ Pass ]
crbug.com/1147859 virtual/css-highlight-overlay-painting/external/wpt/css/css-highlight-api/painting/custom-highlight-painting-iframe-006.html [ Pass ]
crbug.com/1147859 virtual/css-highlight-overlay-painting/external/wpt/css/css-highlight-api/painting/custom-highlight-painting-inheritance-003.html [ Failure ]
crbug.com/1147859 virtual/css-highlight-overlay-painting/external/wpt/css/css-highlight-api/painting/custom-highlight-painting-invalidation-001.html [ Pass ]
crbug.com/1147859 virtual/css-highlight-overlay-painting/external/wpt/css/css-highlight-api/painting/custom-highlight-painting-invalidation-002.html [ Pass ]
crbug.com/1147859 virtual/css-highlight-overlay-painting/external/wpt/css/css-highlight-api/painting/custom-highlight-painting-invalidation-003.html [ Pass ]
Expand Down Expand Up @@ -6696,8 +6697,7 @@ crbug.com/1274174 external/wpt/css/css-highlight-api/painting/custom-highlight-p
crbug.com/1274174 external/wpt/css/css-highlight-api/painting/custom-highlight-painting-text-decoration-dynamic-001.html [ Pass ]
crbug.com/1147859 [ Linux ] external/wpt/css/css-highlight-api/painting/* [ Failure ]
crbug.com/1147859 [ Linux ] virtual/css-highlight-inheritance/external/wpt/css/css-highlight-api/painting/* [ Failure ]
crbug.com/1024156 external/wpt/css/css-highlight-api/painting/custom-highlight-painting-inheritance-* [ Failure ]
crbug.com/1024156 virtual/css-highlight-inheritance/external/wpt/css/css-highlight-api/painting/custom-highlight-painting-inheritance-* [ Pass ]
crbug.com/1147859 [ Linux ] paint/custom-highlight-only-inheritance.html [ Failure ]

# Sheriff 2021-06-11
crbug.com/1218667 [ Win ] virtual/portals/wpt_internal/portals/portals-dangling-markup.sub.html [ Pass Timeout ]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<!DOCTYPE html>
<meta charset="UTF-8">
<style>
#highlight {
background-color: green;
color: yellow;

}
#selection {
color: green;
}

#selection strong{
color: initial;
}
</style>
<body>
<div>There should be a green background color and yellow font coloe below from [ to ]:</div>
<div>[<span id="highlight">Custom <strong>Highlight</strong> Inheritance</span>]</div>
<div>There should be a selection below from [ to ], but background color only visible around 'Selection', and 'Text' and 'Sample' font color should be green:</div>
<div>[<span id="selection">Text <strong>Selection</strong> Sample</span>]</div>
<script>
let selection_range = new Range();
const selection_node = document.querySelector('#selection strong');
selection_range.setStart(selection_node, 0);
selection_range.setEnd(selection_node, 1);
window.getSelection().addRange(selection_range);
</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
<!DOCTYPE html>
<meta charset="UTF-8">
<title>CSS Highlight API Test: Inheritance is enabled for custom highlight but not for other types of highlights</title>
<style>
div::highlight(div-highlight) {
background-color: green;
}
span::highlight(span-highlight) {
color: yellow;
}
div::selection {
background-color: yellow;
color: blue;
}
span::selection {
color: green;
}
</style>
<body>
<div>There should be a green background color and yellow font coloe below from [ to ]:</div>
<div id="highlight_target">[<span>Custom <strong>Highlight</strong> Inheritance</span>]</div>
<div>There should be a selection below from [ to ], but background color only visible around 'Selection', and 'Text' and 'Sample' font color should be green:</div>
<div id="selection_target">[<span>Text <strong>Selection</strong> Sample</span>]</div>
<script>
let highlight_range = new Range();
const highlight_node = document.getElementById("highlight_target");
highlight_range.setStart(highlight_node, 1);
highlight_range.setEnd(highlight_node, 2);

CSS.highlights.set("div-highlight", new Highlight(highlight_range));
CSS.highlights.set("span-highlight", new Highlight(highlight_range));

let selection_range = new Range();
const selection_node = document.getElementById("selection_target");
selection_range.setStart(selection_node, 1);
selection_range.setEnd(selection_node, 2);
window.getSelection().addRange(selection_range);
</script>

0 comments on commit cc6c0ae

Please sign in to comment.