Skip to content

Commit

Permalink
Defer to non-anonymous ancestor when determining selection background…
Browse files Browse the repository at this point in the history
… color for anonymous blocks.

https://bugs.webkit.org/show_bug.cgi?id=263658

Reviewed by Ryosuke Niwa.

There is a precedent for anonymous blocks deferring to their non-anonymous
ancestors when deriving selectionBackgroundColor. For example RenderText
defers to its ancestor in order to ensure text blocks are colored correctly
when a custom color has been set with a Selection pseudo-element. This
change addresses the failure to defer to an ancestor when painting the
selection on gaps between two blocks with uneven widths. By deferring to
nonAnonymousAncestor at the RenderElement level, it should ensure a fix for
bug #263658 as well as potentially alleviating failures in other areas that
have not yet been diagnosed. Because the change is limited to anonymous nodes,
and because the selectionBackgroundColor is only consulted specifically when
managing the painting of selected ranges, I believe this is a low risk proposal
which cleanly addresses the linked bug report.

* LayoutTests/platform/mac/fast/selectors/selection-gap-background-color-expected.png: Added.
* LayoutTests/platform/mac/fast/selectors/selection-gap-background-color.html: Added.
* Source/WebCore/rendering/RenderElement.cpp:
(WebCore::RenderElement::selectionBackgroundColor const):

Canonical link: https://commits.webkit.org/271129@main
  • Loading branch information
danielpunkass authored and annevk committed Nov 27, 2023
1 parent 5fa1fad commit 76bb960
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 3 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<!DOCTYPE html>
<meta charset="utf-8">
<title>Selection gap custom background color</title>

<style>
::selection {
background-color: #FF0000;
}
</style>

<div>This test passes if the gap to the right of this sentence -></div>
<p>
Is colored with the same background color as this text.
</p>

<script>
document.execCommand("SelectAll");
</script>
26 changes: 26 additions & 0 deletions LayoutTests/fast/selectors/selection-gap-background-color.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<!DOCTYPE html>
<meta charset="utf-8">

<title>Selection gap custom background color</title>
<meta name="assert" content="Selection gaps between blocks of different widths are colored with an ancestor's ::selection pseudo-element background color.">

<body>

<style>
::selection {
background-color: red;
}
</style>

This test passes if the gap to the right of this sentence ->
<p>
Is colored with the same background color as this text.
</p>

<script>
window.addEventListener('load', async event => {
document.execCommand("SelectAll");
});
</script>

</body>
12 changes: 9 additions & 3 deletions Source/WebCore/rendering/RenderElement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1708,9 +1708,15 @@ Color RenderElement::selectionBackgroundColor() const
if (frame().selection().shouldShowBlockCursor() && frame().selection().isCaret())
return theme().transformSelectionBackgroundColor(style().visitedDependentColorWithColorFilter(CSSPropertyColor), styleColorOptions());

std::unique_ptr<RenderStyle> pseudoStyle = selectionPseudoStyle();
if (pseudoStyle && pseudoStyle->visitedDependentColorWithColorFilter(CSSPropertyBackgroundColor).isValid())
return theme().transformSelectionBackgroundColor(pseudoStyle->visitedDependentColorWithColorFilter(CSSPropertyBackgroundColor), styleColorOptions());
auto pseudoStyleCandidate = this;
if (pseudoStyleCandidate->isAnonymous())
pseudoStyleCandidate = pseudoStyleCandidate->firstNonAnonymousAncestor();

if (pseudoStyleCandidate) {
std::unique_ptr<RenderStyle> pseudoStyle = pseudoStyleCandidate->selectionPseudoStyle();
if (pseudoStyle && pseudoStyle->visitedDependentColorWithColorFilter(CSSPropertyBackgroundColor).isValid())
return theme().transformSelectionBackgroundColor(pseudoStyle->visitedDependentColorWithColorFilter(CSSPropertyBackgroundColor), styleColorOptions());
}

if (frame().selection().isFocusedAndActive())
return theme().activeSelectionBackgroundColor(styleColorOptions());
Expand Down

0 comments on commit 76bb960

Please sign in to comment.