1474110 - Choose browserAction text color among white and black, maximizing contrast

Status: RESOLVED FIXED

(WebExtensions :: Frontend, enhancement)

Description

[Oriol Brufau [:Oriol]]

This was one of the proposals in bug 1424620 comment 3:

   By default, the text badge color should be chosen among white and black
   in order to maximize contrast.

   So if I use browser.browserAction.setBadgeBackgroundColor({color: "black"})
   then the text color should be white.
   And if I use browser.browserAction.setBadgeBackgroundColor({color: "white"})
   then the text color should be black.

   I don't think transparency can be properly taken into account, so I would
   just ignore the alpha channel and assume the background is fully opaque.

   https://www.w3.org/TR/WCAG20-TECHS/G18.html#G18-procedure can be used to
   choose between white and black. If L is the background luminance according
   to that algorithm, if 1.05*0.05 < (L+0.05)**2 then we should prefer black.

Was already approved in bug 1424620 comment 13.

Comment 1

[Oriol Brufau [:Oriol]]

Attachment: Bug 1474110 - Choose browserAction text color among white and black, maximizing contrast

Review commit: https://reviewboard.mozilla.org/r/257960/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/257960/

Comment 2

[Shane Caraveo (:mixedpuppy)]

Comment on attachment 8993146 [details]
Bug 1474110 - Choose browserAction text color among white and black, maximizing contrast

https://reviewboard.mozilla.org/r/257960/#review266346

Given I'm understanding the calculation I pointed out below, the addition of a comment, and some instruction for QA to give some visual verfication, I'm good with this.

::: browser/components/extensions/parent/ext-browserAction.js:637
(Diff revision 1)
> +        return channel / 12.92;
> +      }
> +      return ((channel + 0.055) / 1.055) ** 2.4;
> +    });
> +    let luminance = 0.2126 * r + 0.7152 * g + 0.0722 * b;
> +    let channel = 1.05 * 0.05 < (luminance + 0.05) ** 2 ? 0 : 255;

I'm getting a touch lost right here as this isn't (or doesn't seem to be) part of the explanation in the referenced document.

Specifically, what does "1.05 \* 0.05" represent in terms of luminance?

I take it as a luminance midpoint, and if the luminance of the color "(luminance + 0.05) \*\* 2" is higher than midpoint, the color is lighter, so we choose black for text.

Add a code comment to explain.

Comment 3

[Oriol Brufau [:Oriol]]

Comment on attachment 8993146 [details]
Bug 1474110 - Choose browserAction text color among white and black, maximizing contrast

https://reviewboard.mozilla.org/r/257960/#review266356

::: browser/components/extensions/parent/ext-browserAction.js:637
(Diff revision 1)
> +        return channel / 12.92;
> +      }
> +      return ((channel + 0.055) / 1.055) ** 2.4;
> +    });
> +    let luminance = 0.2126 * r + 0.7152 * g + 0.0722 * b;
> +    let channel = 1.05 * 0.05 < (luminance + 0.05) ** 2 ? 0 : 255;

The luminance of black is 0, the luminance of white is 1, let `L` be the luminance of the background. Black will never be lighter than the background, so its contrast is `(L + 0.05) / 0.05`. White will never be darker than the background, so its contrast is `1.05 / (L + 0.05)`. That's all according to the linked algorithm. Then we want to maximize contrast, so black is chosen if `(L + 0.05) / 0.05 > 1.05 / (L + 0.05)`, i.e. `(L + 0.05)**2 > 1.05 * 0.05`.

Comment 4

[Oriol Brufau [:Oriol]]

Comment on attachment 8993146 [details]
Bug 1474110 - Choose browserAction text color among white and black, maximizing contrast

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/257960/diff/1-2/

Comment 5

[Oriol Brufau [:Oriol]]

Comment on attachment 8993146 [details]
Bug 1474110 - Choose browserAction text color among white and black, maximizing contrast

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/257960/diff/2-3/

Comment 6

[Oriol Brufau [:Oriol]]

Also cached white as the default text color for the default [0xd9, 0, 0, 255] background.
This avoids contrast calculations for add-ons that don't use setBadgeBackgroundColor.

Comment 7

[Shane Caraveo (:mixedpuppy)]

Comment on attachment 8993146 [details]
Bug 1474110 - Choose browserAction text color among white and black, maximizing contrast

https://reviewboard.mozilla.org/r/257960/#review266388

Thanks!

Comment 8

[Mozilla Autoland]

(255, 'comparing with ssh://hg.mozilla.org/integration/autoland\nremote: Permission denied (publickey).', 'abort: no suitable response from remote hg!')

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b8bdb82da4ac
Choose browserAction text color among white and black, maximizing contrast r=mixedpuppy

Comment 10

[Stefan Hindli [:stefan_hindli]]

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

Comment 11

[Vlad Jiman]

Is manual testing required on this bug? If yes, please provide some STR and the proper extension(if required) or set the “qe-verify -“ flag.

Thanks!

Comment 12

[Oriol Brufau [:Oriol]]

Covered by automated tests

Comment 13

[Will Bamberg [:wbamberg]]

I've added a note in https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/browserAction/setBadgeBackgroundColor, which seemed to be the best place to mention this. I didn't think it was really appropriate to make a change in the compat data here, but please let me know if you do!

Comment 14

[Oriol Brufau [:Oriol]]

I would also mention this in getBadgeTextColor once it is documented.
The compat data for setBadgeBackgroundColor doesn't seem necessary with the note. But maybe I would include it in the compat data for getBadgeTextColor

Comment 15

[Will Bamberg [:wbamberg]]

Thanks! I've mentioned this in https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/browserAction/getBadgeTextColor. I haven't added it the compat data, since I think the notes in the page should be enough.