Choose browserAction text color among white and black, maximizing contrast

RESOLVED FIXED in Firefox 63

Status

--
enhancement
RESOLVED FIXED
9 months ago
6 months ago

People

(Reporter: Oriol, Assigned: Oriol)

Tracking

({dev-doc-complete})

unspecified
mozilla63
dev-doc-complete
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox63 fixed)

Details

(Whiteboard: [design-decision-approved][browserAction])

Attachments

(1 attachment)

(Assignee)

Description

9 months ago
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 hidden (mozreview-request)

Comment 2

8 months ago
mozreview-review
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.
Attachment #8993146 - Flags: review?(mixedpuppy) → review+
(Assignee)

Comment 3

8 months ago
mozreview-review
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 hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 6

8 months ago
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

8 months ago
mozreview-review
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!
(Assignee)

Updated

8 months ago
Keywords: checkin-needed, dev-doc-needed

Comment 8

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

Comment 9

8 months ago
Pushed by csabou@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b8bdb82da4ac
Choose browserAction text color among white and black, maximizing contrast r=mixedpuppy
Keywords: checkin-needed

Comment 10

8 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/b8bdb82da4ac
Status: ASSIGNED → RESOLVED
Last Resolved: 8 months ago
status-firefox63: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla63

Comment 11

7 months ago
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!
Flags: needinfo?(oriol-bugzilla)
(Assignee)

Comment 12

7 months ago
Covered by automated tests
Flags: needinfo?(oriol-bugzilla) → qe-verify-
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!
Flags: needinfo?(oriol-bugzilla)
(Assignee)

Comment 14

7 months ago
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
Flags: needinfo?(oriol-bugzilla)
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.
Keywords: dev-doc-needed → dev-doc-complete
You need to log in before you can comment on or make changes to this bug.