519486 - Some themes are illegible with the text shadow implementation

Status: VERIFIED FIXED

(Firefox :: Theme, defect, P2)

Description

[Tony Chung [:tchung]]

Attachment: DJ World theme screenshot

With the personas uplift work, the shadow behind the text can look terrible on some themes.  For example, DJ_World will completely blur the text out on the tabs and status bar completely.

See screenshot

Repro:
1) install trunk build:
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2b1pre) Gecko/20090929 Namoroka/3.6b1pre
2) go to personas staging site; DJ World theme:  http://sm-personas01.mozilla.org/en-US/persona/8819
3) Wear that theme
4) Verify text on tab bar and status bar, find bar, are completely blurred and illegible.

Comment 1

[Tony Chung [:tchung]]

Arcane is another one with blurry text in the tabs.  http://sm-personas01.mozilla.org/en-US/persona/2400

Comment 2

[Tony Chung [:tchung]]

flagging for blocking 3.6.  if you can get to it before 3.6, great.  if not, im fine with this going to 3.next since its only a handful of themes causing this.

Comment 3

[Tony Chung [:tchung]]

Attachment: Arcane Theme with extra toolbar space screenshot

Comment 4

[Tony Chung [:tchung]]

Attachment: Spring Flowers screenshot

Also spring flowers.
http://sm-personas01.mozilla.org/en-US/persona/1734

Comment 5

[Mike Beltzner [:beltzner, not reading bugmail]]

Is this any different from how they look in the Personas Add-on? If not, then I think this is INVALID. If so, then it's a blocker. Either way marking blocking+ for now - we should be consistent with its implementation.

Comment 6

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to comment #5)
> Is this any different from how they look in the Personas Add-on? If not, then I
> think this is INVALID. If so, then it's a blocker. Either way marking blocking+
> for now - we should be consistent with its implementation.

Yes, its different - the Personas addon doesn't seem to add the text shadow, and the text is readable when using Personas.

A different text shadow is added depending on whether the text color is determined to be either light (black shadow) or dark (white shadow). In the case of the DJ World persona, a white shadow is applied - if a black shadow is applied, then the text is far more legible.

So hopefully the fix will be just some adjustment of how a light/dark text color is determined.

Comment 7

[Blair McBride [:Unfocused] (UNAVAILABLE)]

I should also point out that the text shadow will generally improve legibility of the text.

Comment 8

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Attachment: Patch v1

Time for some color theory! Instead of brightness (a physical property of color), this patch calculates luminance, which is closer to how the color is actually perceived (by weighting different color components differently). Additionally, based off testing numerous personas, I've also lowered the threshold from 127 down to 110.

All the examples given here are now legible, as are the other personas I've tested. I haven't yet been able to test all 10000+ personas.

Comment 9

[Markus Stange [:mstange]]

Where did you get these numbers from? I'm curious, because the SVG filter spec mentions uses numbers (0.2125, 0.7154 and 0.0721).

Comment 10

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to comment #9)
> Where did you get these numbers from? I'm curious, because the SVG filter spec
> mentions uses numbers (0.2125, 0.7154 and 0.0721).

There are several standards, depending on the output colorspace. Turns out the one in the patch is an older FCC standard. The one the SVG filter uses is newer (BT.709), and the adopted standard for the web - I'm more inclined to use that (thanks!). New patch coming up.

Comment 11

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Attachment: Patch v1.1

Comment 12

[Dão Gottwald [:dao]]

Comment on attachment 404151 [details] [diff] [review]
Patch v1.1

Looks good, thanks!

The status bar is still broken with <http://sm-personas01.mozilla.org/en-US/persona/2400>, but that's also the case without any text shadow.

Comment 13

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/4270c70462b5

Comment 14

[Dão Gottwald [:dao]]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/860c1b4c4454

Comment 15

[Tony Chung [:tchung]]

Attachment: Halloween Party Persona

Not sure this is quite fixed on trunk and 1.9.2 branch.  Some are still causing legible issues, like "halloween party" persona.  (http://www.getpersonas.com/en-US/persona/46739) 

See screenshot.

Tested on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.3a1pre) Gecko/20091111 Minefield/3.7a1pre and Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2b3pre) Gecko/20091112 Namoroka/3.6b3pre

Comment 16

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Hm, that is pretty bad. And it looks fine using the Personas Addon. The luminosity of that persona is 105.9.

"A Happy Halloween" has the same issue (http://www.getpersonas.com/en-US/persona/47350), with a luminosity of 97.1.

As a more extreme example, "Bright Streaming" has issues too (http://www.getpersonas.com/en-US/persona/507), with a luminosity of 67.9.


While the luminosity threshold could be lowered to accommodate these, it would introduce issues with other personas that work well at the moment. Currently, the threshold is 110 - I think 90 would be the lowest I'm comfortable lowering it to. 

Dao, any thoughts on this?



I also found some personas that will have text legibility issues regardless of what we do, as their text color is very similar to the background color. For example:
http://www.getpersonas.com/en-US/persona/26284
http://www.getpersonas.com/en-US/persona/29188

Comment 17

[Dão Gottwald [:dao]]

We could set neither dark nor bright for a certain range, although we should only do so if this helps more themes than it hurts. Somehow themes will always be able to screw up by choosing bad colors.

Comment 18

[Tony Chung [:tchung]]

(In reply to comment #17)
> We could set neither dark nor bright for a certain range, although we should
> only do so if this helps more themes than it hurts. Somehow themes will always
> be able to screw up by choosing bad colors.

Would it be helpful if we pulled a list of top persona themes used out there, and test your workaround fix to make sure these are looking right?   There will always be themes that will screw with the design, but if we can at least identify the more popular ones, we can at least get a better sampling of how bad things really are, and hopefully "fix" 90% of them.

Comment 19

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to comment #17)
> We could set neither dark nor bright for a certain range, although we should
> only do so if this helps more themes than it hurts.

Yea, I just had that thought too. I'll play around with it this weekend.

(In reply to comment #18)
> Would it be helpful if we pulled a list of top persona themes used out there,
> and test your workaround fix to make sure these are looking right?   There will
> always be themes that will screw with the design, but if we can at least
> identify the more popular ones, we can at least get a better sampling of how
> bad things really are, and hopefully "fix" 90% of them.

Yea, that'd be helpful. At the moment I'm testing on a combination of the most popular, and a random sampling (it could be that some personas aren't as popular because of this bug).

Comment 20

[Dão Gottwald [:dao]]

There seems to be a bug in GetDocumentLWTheme (<http://mxr.mozilla.org/mozilla-central/source/content/xul/document/src/nsXULDocument.cpp#4763>). :-moz-lwtheme matches only if the lwthemetextcolor attribute is either bright or dark. Instead, it should just depend on the lwtheme attribute.

Comment 21

[Suneel]

the most popular themes can be found here: http://www.getpersonas.com/en-US/gallery/All/Popular.

note that we are reorganizing the ordering of the gallery page from "popular" to "up and coming" which will likely have impact on which personas get the most usage.

Comment 22

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to comment #17)
> We could set neither dark nor bright for a certain range, although we should
> only do so if this helps more themes than it hurts. Somehow themes will always
> be able to screw up by choosing bad colors.

I had a play with this strategy using the patch from bug 528792 (described in comment 20). My comments here only apply to text that is neither obviously dark or obviously bright.

This strategy mostly works fine for themes with plain (ie, flat color) backgrounds. For themes with non-plain backgrounds, not having a textshadow is (usually) better than having the wrong textshadow, but worse than having the correct textshadow.

Of course, there's a catch: which textshadow is "correct" depends on the luminosity of the background.

Comment 23

[Blair McBride [:Unfocused] (UNAVAILABLE)]

I've added bug 529129 to add a luminosity range where lwthemetextcolor should not be set.

Comment 24

[Tony Chung [:tchung]]

Blair, is this really fixed on 1.9.2 and trunk?   If so, can you paste the changeset here?  Because i'm still seeing the illegible examples on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2b5pre) Gecko/20091202 Namoroka/3.6b5pre and Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.3a1pre) Gecko/20091202 Minefield/3.7a1pre

Comment 25

[Blair McBride [:Unfocused] (UNAVAILABLE)]

http://hg.mozilla.org/mozilla-central/rev/4270c70462b5
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/860c1b4c4454

Its fixed in that any further tweaking seems to just make more themes illegible or change which themes are illegible, rather than actually reduce the number of illegible themes. Unfortunately, there's no perfect fix for this.

Comment 26

[Tony Chung [:tchung]]

Verified fix on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2b5pre) Gecko/20091203 Namoroka/3.6b5pre, Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2b5pre) Gecko/20091204 Namoroka/3.6b5pre, and Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.3a1pre) Gecko/20091204 Minefield/3.7a1pre