692752 - text selection highlighting should paint behind text-shadow

Status: RESOLVED FIXED

(Core :: Layout: Text and Fonts, defect)

Description

[Jonathan Kew [:jfkthame]]

Currently, it appears that the text-selection highlight is painted on top of any text-shadow in effect. IMO, this makes for a jarring user experience when selecting text that uses shadows. I note that Safari paints the shadow on top of the selection highlight, and I believe this is preferable behavior.

Simple demonstration:
data:text/html,<p style="font-size:40px; text-shadow:2px 2px 1px red;">Hello, shadowed world</p>

Drag back and forth across the text to select/unselect part of it, and note how the shadow is "lost" under the selection highlight. Compare behavior in Safari.

Comment 1

[Jonathan Kew [:jfkthame]]

Attachment: patch, paint text-shadow after selection highlight background

Something like this ought to improve the user experience - seems to work in simple testing, at least, and passes unit tests on try.

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 565577 [details] [diff] [review]
patch, paint text-shadow after selection highlight background

Review of attachment 565577 [details] [diff] [review]:
-----------------------------------------------------------------

Reftest? Should at least be able to use a != test

Comment 3

[Jonathan Kew [:jfkthame]]

Attachment: reftest for painting selected text with shadow

I think we can test in both != and == forms, by faking the reference rendering using two overlaid divs. Pushed this to tryserver to check that it passes on various platforms (worked ok locally).

Comment 4

[Jonathan Kew [:jfkthame]]

(In reply to Jonathan Kew (:jfkthame) from comment #3)
> Created attachment 565721 [details] [diff] [review] [diff] [details] [review]
> reftest for painting selected text with shadow
> 
> I think we can test in both != and == forms, by faking the reference
> rendering using two overlaid divs. Pushed this to tryserver to check that it
> passes on various platforms (worked ok locally).

Hmm, the == test fails on platforms where the text color is automagically changed to white when selected. Making the text explicitly "color:white" solves this. Re-running tryserver job to verify there are no other surprises.

Comment 5

[Jonathan Kew [:jfkthame]]

Attachment: reftest v2 - updated to use white text

Updated to use color:white; now passes on all platforms. Carrying forward r=roc.

Comment 6

[Jonathan Kew [:jfkthame]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b87e8e7e7916 (patch)
https://hg.mozilla.org/integration/mozilla-inbound/rev/bd5f05cdf119 (reftest)

Comment 7

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://hg.mozilla.org/mozilla-central/rev/b87e8e7e7916
https://hg.mozilla.org/mozilla-central/rev/bd5f05cdf119