Closed Bug 721750 Opened 13 years ago Closed 12 years ago

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

Categories

(Core :: CSS Parsing and Computation, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla17

People

(Reporter: matvey, Assigned: jfkthame)

References

Details

(Keywords: dev-doc-complete, regression, testcase)

Attachments

(5 files, 1 obsolete file)

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.
works for me with Mozilla/5.0 (Windows NT 6.1; rv:12.0a1) Gecko/20120125 Firefox/12.0a1 SeaMonkey/2.9a1
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
Keywords: regression
OS: Windows 7 → All
Hardware: x86 → All
Version: 10 Branch → Trunk
Component: Untriaged → Layout
Product: Firefox → Core
QA Contact: untriaged → layout
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
The first bad revision is: changeset: 78399:b87e8e7e7916 user: Jonathan Kew <jfkthame@gmail.com> 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
Blocks: 692752
Status: UNCONFIRMED → NEW
Ever confirmed: true
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
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → INVALID
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.
Status: RESOLVED → REOPENED
Component: Layout → Style System (CSS)
QA Contact: layout → style-system
Resolution: INVALID → ---
Summary: text-shadow doesn't disappear in selection → Shoud "text-shadow" apply to "-moz-selextion" now due to fix of bug 692752? (text-shadow doesn't disappear in selection)
Attached file testcase (deleted) —
Keywords: testcase
Summary: Shoud "text-shadow" apply to "-moz-selextion" now due to fix of bug 692752? (text-shadow doesn't disappear in selection) → Shoud "text-shadow" apply to "-moz-selection" now due to fix of bug 692752? (text-shadow doesn't disappear in selection)
(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.
Summary: Shoud "text-shadow" apply to "-moz-selection" now due to fix of bug 692752? (text-shadow doesn't disappear in selection) → Should "text-shadow" apply to "-moz-selection" now due to fix of bug 692752? (text-shadow doesn't disappear in selection)
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.
Attachment #596316 - Attachment mime type: text/plain → text/html
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.
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.
(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.
I think we should probably support text-shadow here.
Attached patch patch, support text-shadow on ::-moz-selection (obsolete) (deleted) — Splinter Review
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.
Attachment #596767 - Flags: review?(roc)
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.
Keywords: dev-doc-needed
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).
Attachment #597132 - Flags: review?(dbaron)
(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).
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.)
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?
Attachment #597187 - Flags: review?(dbaron)
Assignee: nobody → jfkthame
So how does the behavior you're trying to implement compare to how other non-inherited properties work with ::-moz-selection?
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.
dbaron, r? ping.... ISTR we looked at the behavior of this in SF and it seemed reasonable.
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.
(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.
(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.
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.
(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?
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?)
(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 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.)
Attachment #597132 - Flags: review?(dbaron) → review+
Attachment #597187 - Flags: review?(dbaron) → review+
Attachment #596767 - Attachment is obsolete: true
Attachment #596767 - Flags: review?(roc)
Status: REOPENED → RESOLVED
Closed: 13 years ago12 years ago
Resolution: --- → FIXED
Is there already a bug on file for the problem described in comment 26, or shall I file one?
Does bug 234102 cover it? There's also some discussion in bug 509958.
I think bug 234102 covers it.
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.
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).
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.
Blocks: 826006
No longer blocks: 826006
Depends on: 826006
No longer depends on: 826006
Depends on: 1363088
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: