826556 - Work - Change foreground texts to dark color on a light bg tile

Status: VERIFIED FIXED

(Firefox for Metro Graveyard :: Firefox Start, defect, P1)

Description

[Yuan Wang(:Yuan) – Mobile Firefox Design Lead]

Attachment: [sample] screenshot on a white tile

When FX generated a white bookmark tile bg, the foreground text didn't change to black. The readability on white tile is very poor. 

We need to identify a certain range of light colors, and match with dark foreground text.

Comment 1

[Allison Naaktgeboren :ally]

scope check, is this just for topsites tiles, or should i try for more general solution?

Comment 2

[Matt Brubeck (:mbrubeck)]

I'm fine accepting topsites-specific code for this.  If it happens to include any functions that seems more widely useful, we can add them somewhere like browser/metro/base/content/Util.js or mozIColorAnalyzer for others' enjoyment.

Comment 3

[Yuan Wang(:Yuan) – Mobile Firefox Design Lead]

Hi Stephen, do we have a definition on the value of light colors?
It would be useful to have that info. Thanks!

Comment 4

[Stephen Horlander [:shorlander]]

Plain white #fff

Comment 5

[Allison Naaktgeboren :ally]

yuan, shorlander, I think I poorly articulated what I was looking for:


let colorOfFavicon = mozIColorAnalyzer(siteFavicon);
if (colorOfFavicon > lightColorCutOffValue)
   item.textcolor = dark; // item bg color is too light, use dark text
else
   item.textcolor = #fff; //item bg is dark, use light color text

what is the value of lightColorCutOffValue ?

Comment 6

[Allison Naaktgeboren :ally]

worked out on irc. This bug will refactor the color code out of topsites, bug 848155 so that it can be used by bookmarks, fixing this issue and making our code a bit better in the process(and making it easier to test the color code)

for testing hte color code ideas, see suggestions in https://bugzilla.mozilla.org/show_bug.cgi?id=848155#c5

Comment 7

[Allison Naaktgeboren :ally]

fixed with another bug:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ecd66e8c854f

Comment 8

[Allison Naaktgeboren :ally]

wasnt picked up properly, but went in with  848155 as previously mentioned, updating this bug to match that one

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

Comment 9

[Virgil Dicu [:virgil] [QA]]

Mozilla/5.0 (Windows NT 6.2; WOW64; rv:23.0) Gecko/20130404 Firefox/23.0

Verified that for a white bookmark tile background, the foreground text is black. The readability on white tile is ok. 

Also verified that for black bookmark tile background, the text is white and ok to read.