721750 - Should "text-shadow" apply to "-moz-selection" now due to fix of bug 692752? (text-shadow doesn't disappear in selection)

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Matvey]

User Agent: Mozilla/5.0 (Windows NT 6.1; rv:10.0) Gecko/20100101 Firefox/10.0
Build ID: 20120123235200

Steps to reproduce:

Removed text-shadow from selected text:
::-moz-selection{ text-shadow:none; /**/ }
::selection{ text-shadow:none;}

Minimal example: http://jsfiddle.net/r4zTG/3/


Actual results:

text-shadow didn't disappear.


Expected results:

text-shadow should have disappeared in selection like it did in FF 9.

Comment 1

[Matthias Versen [:Matti]]

works for me with  Mozilla/5.0 (Windows NT 6.1; rv:12.0a1) Gecko/20120125 Firefox/12.0a1 SeaMonkey/2.9a1

Comment 2

[Thomas Ahlblom]

WFM:
Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv:1.9.2.25) Gecko/20111212 Firefox/3.6.25
Mozilla/5.0 (X11; Linux x86_64; rv:9.0.1) Gecko/20100101 Firefox/9.0.1
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:9.0.1) Gecko/20100101 Firefox/9.0.1
Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/535.7 (KHTML, like Gecko) Chrome/16.0.912.75 Safari/535.7

Reproduced:
Mozilla/5.0 (X11; Linux x86_64; rv:10.0) Gecko/20100101 Firefox/10.0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:10.0) Gecko/20100101 Firefox/10.0
Mozilla/5.0 (X11; Linux x86_64; rv:11.0a2) Gecko/20120127 Firefox/11.0a2
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0a2) Gecko/20120127 Firefox/11.0a2
Mozilla/5.0 (X11; Linux x86_64; rv:12.0a1) Gecko/20120127 Firefox/12.0a1
Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:12.0a1) Gecko/20120127 Firefox/12.0a1
Opera/9.80 (X11; Linux x86_64; U; en) Presto/2.10.229 Version/11.61

Comment 3

[Thomas Ahlblom]

Last good nightly: 2011-10-09
First bad nightly: 2011-10-10

Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=b4da2d439cbc&tochange=e9c620a5c85f


My wild guess: Bug 692752

Comment 4

[Thomas Ahlblom]

The first bad revision is:
changeset:   78399:b87e8e7e7916
user:        Jonathan Kew <[e-mail]>
date:        Sat Oct 08 17:27:14 2011 +0100
summary:     Bug 692752 - paint text-shadow after selection highlight background. r=roc

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

Comment 5

[j.j.]

The change in bug 692752 is intended.

> ::-moz-selection{ text-shadow:none }

You can't do this because "text-shadow" doesn't apply to ::-moz-selection.
See
https://developer.mozilla.org/En/CSS/%3A%3Aselection#Notes

Comment 6

[j.j.]

Maybe I closed this a bit premature as invalid.

Quote form http://html5boilerplate.com/docs/css/
  Highlighted text can look a mess if it also has text shadows of certain colours
  or sizes. Existing browsers don't let you reset just text-shadow in 
  ::selection without reset the background highlight color.
  Therefore, to avoid the text-shadow-when-highlighted problem we set 
  text-shadow: none but must also declare a background and color. 
(That's not exactly true but ... )

Therefore the have this in their style.css file:

 ::-moz-selection { background: #fe57a1; color: #fff; text-shadow: none; }
 ::selection { background: #fe57a1; color: #fff; text-shadow: none; }

The -moz-selection part was useless before bug 692752 was fixed and doesn't work now because "text-shadow" doesn't apply to ::-moz-selection.

I don't know if "Highlighted text can look a mess" is a real problem, but consider that "text-shadow" will become even more popular when IE10 ships.

Comment 7

[j.j.]

Attachment: testcase

Comment 8

[Jonathan Kew [:jfkthame]]

(Removed the "regression" keyword, as this was not a regression, it was an intended bugfix. Comment #0 was relying on a quirk of the old rendering, not on behavior of ::-moz-selection.)

Offhand, I don't see any strong reason _not_ to support text-shadow on ::-moz-selection, but I think we should see whether people like dbaron or roc with greater CSS experience have an opinion on this.

Comment 10

[Cacycle]

Of course we should support text-shadow on ::(-moz-)selection. Chrome does it (use updated testcase) and without, selected text becomes unreadable in most cases.

Comment 11

[Cacycle]

Attachment: testcase selection text-shadow (browser independent)

Comment 12

[Jonathan Kew [:jfkthame]]

It would be nice if there was some kind of spec for this. It's noticeably absent from CSS3 Selectors (http://www.w3.org/TR/selectors/#selection). There was a short thread on www-style recently ("Status of the ::selection pseudo-element"), but no conclusion AFAICT.

Meanwhile, in the absence of either a clear spec or interoperable implementations, it seems like designers would be wise to simply avoid color/background/shadow combinations that may end up looking bad.

Comment 13

[Cacycle]

Users have come to expect "normalized plain text" (white on dark blue, without shadows) in selections, also as a last resort to make text readable on pages with strange styling. Since FF 10 it is no longer even possible to imitate this standard behaviour! This is a serious usability issue.

> would be wise to simply avoid color/background/shadow combinations 

This would either mean not to use shadows at all or not to use inverted selection color schemes. Both options are not practical.

Comment 14

[Jonathan Kew [:jfkthame]]

(In reply to Cacycle from comment #13)
> Users have come to expect "normalized plain text" (white on dark blue,
> without shadows) in selections,

That may be "normal" on one platform, but it's certainly not true everywhere. E.g. on my Mac, neither TextEdit nor MS Word hide the shadow when I select some text, nor do they change the text color to white. I don't expect web sites to do so, either.

> also as a last resort to make text readable
> on pages with strange styling. 

If designers are creating pages with "strange styling" that makes the text unreadable, I have no sympathy for them, and I don't think it is the browser's job to work around their poor design.

> Since FF 10 it is no longer even possible to
> imitate this standard behaviour! This is a serious usability issue.
> 
> > would be wise to simply avoid color/background/shadow combinations 
> 
> This would either mean not to use shadows at all or not to use inverted
> selection color schemes. Both options are not practical.

Really? Much of the web works quite nicely without them.

Note that I'm not saying we should refuse point-blank to implement text-shadow for -moz-selection; see comment #8. But the lack of any kind of spec - the old draft of ::selection, which incidentally did *not* include text-shadow among the supported properties, having been removed - does concern me somewhat. Implementing features ad hoc just because someone says "I want this..." is a recipe for chaos.

Comment 15

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

I think we should probably support text-shadow here.

Comment 16

[Jonathan Kew [:jfkthame]]

Attachment: patch, support text-shadow on ::-moz-selection

OK, here's an attempt to implement this - seems to work as expected in simple testcases, at least.

We should be able to reftest this fairly easily, too.

Comment 17

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

Hmm. I'm not sure if this is the right way to do this in the style system. It might be better to explicitly probe the nsRuleNode or something like that, like nsRuleNode::HasAuthorSpecifiedRules does (or even extend nsRuleNode::HasAuthorSpecifiedRules). That'd be up to dbaron. At least I think we should be able to avoid adding this bool to nsStyleText.

Comment 18

[Jonathan Kew [:jfkthame]]

Attachment: patch, support text-shadow on ::-moz-selection, alternate version

Here's a version that adds support for text-shadow to HasAuthorSpecifiedRules; dbaron, please advise whether this is the approach you think is right, or something different (see roc's comment #17).

Comment 19

[Jonathan Kew [:jfkthame]]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #17)
> Hmm. I'm not sure if this is the right way to do this in the style system.
> It might be better to explicitly probe the nsRuleNode or something like
> that, like nsRuleNode::HasAuthorSpecifiedRules does (or even extend
> nsRuleNode::HasAuthorSpecifiedRules). That'd be up to dbaron. 

Alternate patch attached, feel free to comment, though I've tagged dbaron for review.

> At least I
> think we should be able to avoid adding this bool to nsStyleText.

This version avoids that (though FWIW, the bool shouldn't have affected memory usage, as there are currently a series of 7 PRUint8 fields, followed by a PRInt32, meaning that I'd expect there to be a byte of padding in the existing structure).

Comment 20

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

What's the behavior you're trying to implement with these patches?

(Also see http://lists.w3.org/Archives/Public/www-style/2008Oct/0268.html on how I think ::selection should work.)

Comment 21

[Jonathan Kew [:jfkthame]]

I don't really have any opinion on the deeper issues of how ::selection works (or ought to work) - the questions of nesting, cascading, etc. The idea here was just to try and make text-shadow functional within ::-moz-selection, similarly to how the already-supported properties work, so that examples like attachment 596316 [details] will work as intended.

If doing a simple fix for this is too problematic - e.g. in terms of how it would interact in more complex scenarios - then maybe we should just leave it until such time as there's more clarity on the spec side?

Comment 22

[Jonathan Kew [:jfkthame]]

Attachment: reftests for text-shadow in ::-moz-selection

Comment 23

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

So how does the behavior you're trying to implement compare to how other non-inherited properties work with ::-moz-selection?

Comment 24

[Jonathan Kew [:jfkthame]]

Attachment: examples of -moz-selection to compare behavior of color and text-shadow

It looks like with the patch here, our behavior matches Chrome on this: as soon as ::(-moz-)selection is used to set any properties for the selection, the default behavior of setting color and background is abandoned; however, any text-shadow in effect will persist unless overridden by the ::selection.

Comment 25

[Jonathan Kew [:jfkthame]]

dbaron, r? ping.... ISTR we looked at the behavior of this in SF and it seemed reasonable.

Comment 26

[Zack Weinberg (:zwol)]

I think it's unfortunate that setting _any_ property on ::-moz-selection has the side-effect of cancelling the default behavior.  If I _just_ want text-shadow:none and don't want to interfere with the user's choice (via system themes) of highlight colors I have to make use of deprecated system color names:

::-moz-selection { text-shadow: none; color: HighlightText;
                   background-color: Highlight; }

I also noticed, while experimenting with this, that if you set your own colors for color: and background-color: on ::-moz-selection, some things (notably MathML operators) are nonetheless highlighted with the system theme colors, which looks really awful.

... Maybe we should internally default to not drawing text shadows on selected text (there doesn't appear to be a ::-moz-selection default in ua.css, but I imagine equivalent behavior could be coded into the C++) and then H5BP wouldn't have to be recommending use of this rather problematic experimental pseudo.

Comment 27

[Jonathan Kew [:jfkthame]]

(In reply to Zack Weinberg (:zwol) from comment #26)
> I think it's unfortunate that setting _any_ property on ::-moz-selection has
> the side-effect of cancelling the default behavior. ...

Yes, I found that surprising and a bit disconcerting, although I note that (at least) webkit seems to share the same behavior.

> I also noticed, while experimenting with this, that if you set your own
> colors for color: and background-color: on ::-moz-selection, some things
> (notably MathML operators) are nonetheless highlighted with the system theme
> colors, which looks really awful.

Ugh. That sounds like a separate bug, though.

Comment 28

[Zack Weinberg (:zwol)]

(In reply to Jonathan Kew (:jfkthame) from comment #27)
> (In reply to Zack Weinberg (:zwol) from comment #26)
> > I think it's unfortunate that setting _any_ property on ::-moz-selection has
> > the side-effect of cancelling the default behavior. ...
> 
> Yes, I found that surprising and a bit disconcerting, although I note that
> (at least) webkit seems to share the same behavior.

I would advocate for changing it, because (a): it is inconsistent with the normal behavior of CSS (which would be: it is as if "::moz-selection { color: HighlightText; background-color: Highlight; }" appeared in ua.css, and setting any *other* property on ::moz-selection does not override the UA settings of color: or background-color:)

Perhaps more persuasively, (b): at least one widely deployed CSS boilerplate (H5BP) is using the pseudo in a way that assumes that setting text-shadow does *not* cancel the normal highlight colors.  I'm in here because I tripped over this while redesigning my personal website, using H5BP as a baseline.

> > I also noticed, while experimenting with this, that if you set your own
> > colors for color: and background-color: on ::-moz-selection, some things
> > (notably MathML operators) are nonetheless highlighted with the system theme
> > colors, which looks really awful.
> 
> Ugh. That sounds like a separate bug, though.

Filed bug 759462.

Comment 29

[jakub.g]

I totally second @Cacycle comment (#13). I see a lot of pages putting 1px text-shadow on ordinary text, which makes it look really **** when selected. When I enter that kind of page, I activate Stylish stylesheet which removes text-shadow from everything on the page (or just keep it activated for all the pages). In fact, in 95% of the cases I don't understand developers putting those small 1px shadows which almost don't change anything in normal case, and only crappify selection.

I also customize bgcolor of ::selection in Stylish (I have a couple of stylesheets, with the bgcolors e.g. light~=azure and dark=black and some others, which I activate depending on the color scheme on the page I'm on, to increase readability), and the lack of ::selection{text-shadow:} is especially painful when I want to activate azure selection bgcolor + black color, and the page uses white-ish shadow. Like I said, the easiest but brutal workaround is to have sth like html * {text-shadow:none} rule.

Comment 30

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(In reply to Jonathan Kew (:jfkthame) from comment #25)
> dbaron, r? ping.... ISTR we looked at the behavior of this in SF and it
> seemed reasonable.

At this point (or when you asked as well) I'd forgotten most of the discussion we had.  Do you remember more of it?

Comment 31

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

What behavior does this make us default to?  If the author specifies shadows, do we still draw them in the selector or not?

(And does it vary depending on whether the platform's selection behavior retains the author's colors?  Do we have any platforms that retain the author's colors in the foreground of a selection?)

Comment 32

[Jonathan Kew [:jfkthame]]

(In reply to David Baron [:dbaron] from comment #31)
> What behavior does this make us default to?  If the author specifies
> shadows, do we still draw them in the selector or not?

Yes, unless they're specifically overridden by a text-shadow property in the ::selection. (AFAICT, this matches webkit behavior, so it's what authors are coding for when they try to control this stuff.)

> (And does it vary depending on whether the platform's selection behavior
> retains the author's colors?  Do we have any platforms that retain the
> author's colors in the foreground of a selection?)

Yes, OS X retains the author's foreground color in a selection; the default behavior is to just paint a selection background color. The "standard" described in comment #13 is not in fact a standard - it's Windows behavior, but not Mac. (Not sure about Linux offhand; it might well depend on the desktop environment?)

::selection is kind of weird, in that as soon as *any* property is specified, the default selection highlighting - painting the selection background color, and changing text to white on some platforms - is abandoned, and *only* the properties specified by the selector are used. So, for example,

  ::selection { color: red }

will not only make the selected text (foreground) color red, it will also cancel the default selection background. However, it won't cancel any text-shadow that was in effect; to do that, ::selection needs to explicitly include text-shadow:none.

Comment 33

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 597132 [details] [diff] [review]
patch, support text-shadow on ::-moz-selection, alternate version

r=dbaron.  Sorry for the long delay here.

(I'm hoping that having to be compatible with this doesn't get in our way when we try to make ::selection behave more sensibly; I think it's pretty reasonable, though.)

Comment 34

[Jonathan Kew [:jfkthame]]

Pushed to inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d4084a5a23a5 (patch)
https://hg.mozilla.org/integration/mozilla-inbound/rev/8f0244595d42 (reftest)

Comment 35

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/d4084a5a23a5
https://hg.mozilla.org/mozilla-central/rev/8f0244595d42

Comment 36

[Zack Weinberg (:zwol)]

Is there already a bug on file for the problem described in comment 26, or shall I file one?

Comment 37

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Does bug 234102 cover it?  There's also some discussion in bug 509958.

Comment 38

[Zack Weinberg (:zwol)]

I think bug 234102 covers it.

Comment 39

[Frédéric Bourgeon [:FredB]]

The doc concerning ::selection has been updated with the addition of text-shadow as supported property and a note on support. https://developer.mozilla.org/en-US/docs/CSS/::selection#Summary

Please someone edit the bug and mark it as dev-doc-complete since I don't have the privs.

Comment 40

[Cacycle]

In Firefox 17.0.1 (Mozilla/5.0 (Windows NT 5.1; rv:17.0) Gecko/20100101 Firefox/17.0) the text-shadow property is now correctly applied to selections. 

However, as soon as the selection containing object becomes unfocused (i.e. a blue selection turns grey), the text-shadow property no longer applied to the selection.

This behavior feels wrong and has probably been missed in the patches (unless I miss some sophisticated CSS magic here). 

To replicate, open the first testcase, select some text (the shadow disappears). Then click into the address bar to remove the focus (shadow re-appears in the unfocused selection).

Comment 41

[Zack Weinberg (:zwol)]

We prefer to deal with issues like that as follow-up bugs.  Cacycle, please file a new bug (just copy what you said in comment 40 into the description) and mark it as depending on this bug.  If you prefer I can do it for you, but it's better if you are the official reporter.