Closed
Bug 503188
Opened 16 years ago
Closed 16 years ago
text-shadow uses page colors even when page colors are disabled
Categories
(Core :: CSS Parsing and Computation, defect, P3)
Tracking
()
RESOLVED
FIXED
mozilla1.9.2a1
People
(Reporter: neady, Assigned: dbaron)
References
()
Details
Attachments
(4 files)
24.82 KB,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
6.58 KB,
patch
|
bzbarsky
:
review+
roc
:
review+
|
Details | Diff | Splinter Review |
4.70 KB,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
849 bytes,
text/html
|
Details |
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.
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Linux → All
Blocks: text-shadow
Assignee | ||
Comment 2•16 years ago
|
||
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
![]() |
||
Comment 3•16 years ago
|
||
That all sounds reasonable to me.
Assignee | ||
Updated•16 years ago
|
Assignee: nobody → dbaron
Status: NEW → ASSIGNED
Priority: -- → P3
Hardware: x86 → All
Target Milestone: --- → mozilla1.9.2a1
Assignee | ||
Comment 4•16 years ago
|
||
Attachment #387765 -
Flags: superreview?(bzbarsky)
Attachment #387765 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 5•16 years ago
|
||
Attachment #387766 -
Flags: review?(bzbarsky)
Assignee | ||
Updated•16 years ago
|
Attachment #387765 -
Flags: superreview?(bzbarsky)
Assignee | ||
Updated•16 years ago
|
Attachment #387766 -
Flags: review?(roc)
![]() |
||
Comment 6•16 years ago
|
||
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+
![]() |
||
Updated•16 years ago
|
Attachment #387766 -
Flags: review?(bzbarsky) → review+
Attachment #387766 -
Flags: review?(roc) → review+
Does this mean people that have page colors being ignored will miss out on the glory of effects like spotlight, flames, and smoke?
http://hacks.mozilla.org/2009/06/text-shadow-spotlight/
http://m8y.org/tmp/testcase63.xhtml
http://m8y.org/tmp/testcase67.xhtml
![]() |
||
Comment 8•16 years ago
|
||
Yes, absolutely. It's the least of what they'll miss out on in terms of "effects"...
Assignee | ||
Comment 9•16 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/afac199a8153
http://hg.mozilla.org/mozilla-central/rev/a23d948042c9
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Comment 10•16 years ago
|
||
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?
![]() |
||
Comment 11•16 years ago
|
||
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.
Assignee | ||
Comment 12•16 years ago
|
||
I think we shouldn't change this on a security release branch.
Comment 13•16 years ago
|
||
The same applies to -moz-box-shadow, no?
Assignee | ||
Comment 14•16 years ago
|
||
Think we should also do this for -moz-box-shadow ?
Attachment #389071 -
Flags: review?(bzbarsky)
![]() |
||
Updated•16 years ago
|
Attachment #389071 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Comment 15•16 years ago
|
||
Comment 16•16 years ago
|
||
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)
Comment 17•16 years ago
|
||
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.
Description
•