Bug 141774 - Support text decorations for ::selection
Summary: Support text decorations for ::selection
Status: RESOLVED CONFIGURATION CHANGED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Daniel Bates
URL:
Keywords: WebExposed
Depends on:
Blocks: 175784
  Show dependency treegraph
 
Reported: 2015-02-18 15:51 PST by Jonas Platte
Modified: 2022-07-14 13:41 PDT (History)
8 users (show)

See Also:


Attachments
Testcase (366 bytes, text/html)
2015-02-18 15:51 PST, Jonas Platte
no flags Details
Patch and layout test (21.53 KB, patch)
2018-10-29 13:00 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews122 for ios-simulator-wk2 (2.47 MB, application/zip)
2018-10-29 14:57 PDT, EWS Watchlist
no flags Details
Patch and layout test (22.36 KB, patch)
2018-10-30 16:45 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jonas Platte 2015-02-18 15:51:07 PST
Created attachment 246852 [details]
Testcase

Steps to reproduce:

Select a text with a `text-decoration: underline` part while the following CSS rule is active:

	::selection { color: red; }


Actual results:

The selected text, excluding the underline, is colored red.


Expected results:

The selected text, including the underline, is colored red.

I also reported this bug on the Mozilla Bugzilla, as it happens in Firefox too (with ::-moz-selection): https://bugzilla.mozilla.org/show_bug.cgi?id=1134444
Comment 1 Daniel Bates 2018-10-29 12:52:42 PDT
Retitling bug as this bug represents a more general problem: text decorations are not painted for ::selection. When painting selection we paint the text decoration for the line only accounting for the pseudo element ::first-line.
Comment 2 Daniel Bates 2018-10-29 12:58:01 PDT
The CSS text-decorations property is not an inherited property. Even if we substituted ::selection for ::-moz-selection in the attached test case (attachment #246852 [details]), a text decoration would not be expected to paint when selecting the underlined test because text-decoration is not inherited. If we changed the following snippet in the test case from:

::-moz-selection {
    color: red;
}

to

::selection {
    color: red;
    text-decoration: inherit;
}

then the text-decoration would be inherited.
Comment 3 Daniel Bates 2018-10-29 13:00:52 PDT
Created attachment 353312 [details]
Patch and layout test
Comment 4 EWS Watchlist 2018-10-29 13:04:32 PDT
Attachment 353312 [details] did not pass style-queue:


ERROR: Source/WebCore/rendering/InlineTextBox.cpp:588:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 1 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 Daniel Bates 2018-10-29 14:48:12 PDT
Oops! Ignore iOS EWS. Selection is painted by UIKit. So, will skip the text on iOS.
Comment 6 EWS Watchlist 2018-10-29 14:57:43 PDT Comment hidden (obsolete)
Comment 7 EWS Watchlist 2018-10-29 14:57:45 PDT Comment hidden (obsolete)
Comment 8 Daniel Bates 2018-10-30 16:45:37 PDT
Created attachment 353434 [details]
Patch and layout test
Comment 9 EWS Watchlist 2018-10-30 16:51:21 PDT
Attachment 353434 [details] did not pass style-queue:


ERROR: Source/WebCore/rendering/InlineTextBox.cpp:588:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 1 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 10 Daniel Bates 2018-10-30 21:11:31 PDT
If the patch for bug #191040 lands before the patch for this bug. Then I will update the patch for this bug to include the following diff:

[[
diff --git a/Source/WebCore/rendering/InlineTextBox.cpp b/Source/WebCore/rendering/InlineTextBox.cpp
index fbc1143ab65..ce22d4cd8a5 100644
--- a/Source/WebCore/rendering/InlineTextBox.cpp
+++ b/Source/WebCore/rendering/InlineTextBox.cpp
@@ -784,7 +784,7 @@ auto InlineTextBox::resolveStyleForMarkedText(const MarkedText& markedText, cons
             style.overlappedPseudoStyles.add(PseudoId::Selection);
 
         style.textDecorationsInEffect = ([&] {
-            if (auto pseudoStyle = renderer().selectionPseudoStyle())
+            if (auto pseudoStyle = renderer().selectionPseudoStyle(&lineStyle()))
                 return pseudoStyle->textDecorationsInEffect();
             return lineStyle().textDecorationsInEffect();
         })();
]]
Comment 11 Alex Christensen 2021-11-01 12:47:37 PDT
Comment on attachment 353434 [details]
Patch and layout test

This has been requesting review for more than one year.  If this is still needed, please rebase and re-request review.
Comment 12 Brent Fulgham 2022-07-14 13:41:45 PDT
Safari, Chrome, and Firefox all agree on rendering for this test case. I don't believe there is any remaining compatibility issue.