Closed Bug 503188 Opened 14 years ago Closed 14 years ago

text-shadow uses page colors even when page colors are disabled


(Core :: CSS Parsing and Computation, defect, P3)

1.9.1 Branch





(Reporter: neady, Assigned: dbaron)





(4 files)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1) Gecko/20090624 Firefox/3.5
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1) Gecko/20090624 Firefox/3.5

If a stylesheet for a web page specifies a text-shadow property with a specific color, Firefox (and probably any other Gecko product, I imagine) uses that color, even if page colors are disabled.

Reproducible: Always

Steps to Reproduce:
1. Edit->Preferences->Content->Colors
2. Make sure "Allow pages to choose their own colors, instead of my selections above" is *unchecked*.  Close the prefs.
3. Visit any page that has text styled with a text-shadow in a color that doesn't go with your colors.
Actual Results:  
The shadow is displayed in the color the stylesheet specified, even though all other page elements use the user's colors.

Expected Results:  
The shadow should use a color derived from the text and background colors in some fashion.  (My vote would be, just re-use the text color, maybe with a hidden about:config pref to change it.)

I normally browse with page colors disabled, system colors set to #FFE6BC foreground on #294D4A background, so something like this really sticks out like a sore thumb.  I know, I'm weird.  But still, the software should be consistent, and if page-specified colors are disabled by the user, that means colors specified by the page should be ignored.  All of them.
Component: General → Layout: Text
Product: Firefox → Core
QA Contact: general → layout.fonts-and-text
Version: unspecified → 1.9.1 Branch
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1) Gecko/20090624 Firefox/3.5
Confirming. In the URL, the first line is not given a colour for text, background or text-shadow, and text-shadow is the colour chosen in prefs. In all the following lines text color and background colour from page is overridden by prefs but text-colour is not.
Ever confirmed: true
OS: Linux → All
Blocks: text-shadow
I suspect the right thing to do for text-shadow when the page colors pref is turned off is to disable text shadows completely; drawing them all in the default color is likely to lead to illegibility.

That should be a relatively straightforward change to nsCSSCompressedDataBlock::MapRuleInfoInto.  (Though we should really move the property lists to the flags field in nsCSSPropList.h, and just special-case background-color... or maybe get better about preserving alpha for backgrounds and borders more generally, and have two flags.)
Component: Layout: Text → Style System (CSS)
QA Contact: layout.fonts-and-text → style-system
That all sounds reasonable to me.
Assignee: nobody → dbaron
Priority: -- → P3
Hardware: x86 → All
Target Milestone: --- → mozilla1.9.2a1
Attachment #387765 - Flags: superreview?(bzbarsky)
Attachment #387765 - Flags: review?(bzbarsky)
Attachment #387765 - Flags: superreview?(bzbarsky)
Attachment #387766 - Flags: review?(roc)
Comment on attachment 387765 [details] [diff] [review]
patch 1: put ignoring colors in the property flags

Looks ok, but with the simpler if condition can we use && instead of nested ifs and outdent the body?
Attachment #387765 - Flags: review?(bzbarsky) → review+
Attachment #387766 - Flags: review?(bzbarsky) → review+
Does this mean people that have page colors being ignored will miss out on the glory of effects like spotlight, flames, and smoke?
Yes, absolutely.  It's the least of what they'll miss out on in terms of "effects"...
Closed: 14 years ago
Resolution: --- → FIXED
Version field is 1.9.1 branch, but there is no wanted1.9.1.2 flag to set. Will this land in 3.5.x at all?
If you want to request branch triage, set the "status1.9.1" flag to "needstriage".

If you're trying to ask for approval to check in on the branch, request approval on the patches.

We're no longer doing per-dot-release wanted flags. See the "Changing how we use flags for branch management in Bugzilla" thread in m.d.planning.
I think we shouldn't change this on a security release branch.
The same applies to -moz-box-shadow, no?
Think we should also do this for -moz-box-shadow ?
Attachment #389071 - Flags: review?(bzbarsky)
Attachment #389071 - Flags: review?(bzbarsky) → review+
This should land on 1.9.1.x to avoid ugly or illegible text.

(Use of text-shadow and box-shadow may increase quickly since support of Fx 3.5)
Attached file testcase
Test with Fx 3.5 and your own colors (black on white) enabled.
You need to log in before you can comment on or make changes to this bug.