Bug 141774

Summary: Support text decorations for ::selection
Product: WebKit Reporter: Jonas Platte <jplatte+webkit>
Component: CSSAssignee: Daniel Bates <dbates>
Status: RESOLVED CONFIGURATION CHANGED    
Severity: Normal CC: achristensen, bfulgham, dbates, dino, ews-watchlist, mmaxfield, simon.fraser, zalan
Priority: P2 Keywords: WebExposed
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 175784    
Attachments:
Description Flags
Testcase
none
Patch and layout test
none
Archive of layout-test-results from ews122 for ios-simulator-wk2
none
Patch and layout test none

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.