1424620 - Allow to set browserAction badge text color

Status: VERIFIED FIXED

(WebExtensions :: Frontend, enhancement, P5)

Description

[Oriol Brufau [:Oriol]]

I just want to display a badge with black colored text and no background, something like https://addons.mozilla.org/en-US/firefox/addon/tab-counter/

I can set the badge background to transparent but then the text is white, and this is not visible in light themes.

So please allow me to set the text color to black.

Comment 1

[Oriol Brufau [:Oriol]]

One approach would be adding a setBadgeTextColor API, allowing authors to set arbitrary colors.

A simpler approach would be just choosing between white and black depending on the background color. White would be used for dark backgrounds, and black for light backgrounds, maximizing contrast. Opacity would be ignored, i.e. [0, 0, 0, 0] and [255, 255, 255, 0] are both fully transparent backgrounds but the former would have white text and the latter black text.

Comment 2

[Caitlin Neiman [:caitmuenster]]

Hi Oriol, this has been added to the agenda for the WebExtensions APIs triage meeting on March 13, 2018. Would you be able to join us? 

Here’s a quick overview of what to expect at the triage: 

* We normally spend 5 minutes per bug
* The more information in the bug, the better
* The goal of the triage is to give a general thumbs up or thumbs down on a proposal; we won't be going deep into implementation details

Relevant Links: 

* Wiki for the meeting: https://wiki.mozilla.org/WebExtensions/Triage#Next_Meeting
* Meeting agenda: https://docs.google.com/document/d/1b4r8z964_Est_mbSYUx9jtRt-HTtXgu-EAzM_3yr7ww/edit
* Vision doc for WebExtensions: https://wiki.mozilla.org/WebExtensions/Vision

Comment 3

[Oriol Brufau [:Oriol]]

To clarify, I have two proposals. You may approve both or only one of them.

 - 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.

 - Additionally, a new browser.browserAction.setBadgeTextColor can be added.
   It would override the behavior above and use the provided color instead.

   Its signature would be like setBadgeBackgroundColor, i.e.

   browser.browserAction.setBadgeTextColor(details), where details has
    - tabId (optional): changes the color only for that tab. If omitted,
      the color is changed globally.
    - color: one of a string, a browserAction.ColorArray or null.

Comment 4

[Hiroyuki Ikezoe (:hiro)]

I wonder if we could modify style, font styles, transform, etc. etc.

Comment 5

[Caitlin Neiman [:caitmuenster]]

We tentatively approved this in the design-decision meeting, but wanted to run this by UX before officially approving. Philip, could you help us?

Comment 6

[Caitlin Neiman [:caitmuenster]]

Apologies; we did not tentatively approve this; we explicitly made no design-decision on this bug during the meeting.

Comment 7

[Oriol Brufau [:Oriol]]

In comment 2 you could have mentioned that today the meeting was 1 hour earlier...

Comment 8

[Caitlin Neiman [:caitmuenster]]

Sorry, Oriol -- I made a general announcement at the Project Call meeting yesterday and updated the wiki about North America going into Daylight Saving Time, but didn't update the agenda or say anything in the bug. :( So sorry about that!

Comment 9

[keeganwitt]

FYI, this came up in the context of another (https://addons.mozilla.org/en-US/firefox/addon/tab-counter-webext/) tab count extension as well: https://github.com/DaAwesomeP/tab-counter/issues/13.  As a user of the extension, +1 on this idea from me.

Comment 10

[Oriol Brufau [:Oriol]]

Philip hasn't been active the latest months. Can somebody else from UX be asked instead?

Comment 11

[Philip [:pwalm]]

Emanuela: this question is more in your wheelhouse, could you give your guidance on this request?

Comment 12

[Mike Conca [:mconca]]

Emanuela, we're looking for input on if it makes sense to allow extensions to control the text color of badges for browserActions (toolbar buttons).  Thanks.

Comment 13

[emanuela [ux]]

I don't have any concern at the moment. Let's move forward

Comment 14

[Oriol Brufau [:Oriol]]

Are both proposals from comment 3 approved, or just one of them?

Comment 15

[emanuela [ux]]

(In reply to Oriol Brufau [:Oriol] from comment #14)
> Are both proposals from comment 3 approved, or just one of them?

Yes, except for the `tabId (optional)`. I don't see how such graininess can help. Let's keep it global.

Comment 16

[Oriol Brufau [:Oriol]]

(In reply to emanuela [ux team] from comment #15)
> Yes, except for the `tabId (optional)`. I don't see how such graininess can
> help. Let's keep it global.

I don't need it for my use cases, but setBadgeBackgroundColor can be set per tab.
So I can imagine an extension using different colors for different kinds of notifications in different tabs.
Then for lighter backgrounds they want to specify a darker text color, and for darker backgrounds they want a lighter text color.

So I would allow tabId and windowId.

Comment 17

[Oriol Brufau [:Oriol]]

I don't know XBL. Is there a better way to set the badge text color?

  document.getAnonymousElementByAttribute(node, "class", "toolbarbutton-badge").style.color = textColor

Comment 18

[Oriol Brufau [:Oriol]]

I didn't read properly, I thought the badgeStyle value was just the bg color but it's a CSS declaration, then adding color should be easy.

Comment 19

[Oriol Brufau [:Oriol]]

Attachment: Bug 1424620 - Add setBadgeTextColor and getBadgeTextColor methods to browserAction

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

Comment 20

[Oriol Brufau [:Oriol]]

I went ahead and added tabId and windowId, is this OK?
And not much sure if the name should be setBadgeTextColor or setBadgeColor

Comment 21

[Oriol Brufau [:Oriol]]

Comment on attachment 8990508 [details]
Bug 1424620 - Add setBadgeTextColor and getBadgeTextColor methods to browserAction

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

Comment 22

[Shane Caraveo (:mixedpuppy)]

Comment on attachment 8990508 [details]
Bug 1424620 - Add setBadgeTextColor and getBadgeTextColor methods to browserAction

https://reviewboard.mozilla.org/r/255582/#review262564

::: browser/components/extensions/parent/ext-browserAction.js:735
(Diff revision 2)
> +          browserAction.setProperty(details, "badgeTextColor", color);
> +        },
> +
> +        getBadgeTextColor: function(details, callback) {
> +          let color = browserAction.getProperty(details, "badgeTextColor");
> +          return color || [255, 255, 255, 255];

Please file a followup bug to get these default colors (inc. background) from the current theme.

Comment 23

[Oriol Brufau [:Oriol]]

(In reply to Shane Caraveo (:mixedpuppy) from comment #22)
> Please file a followup bug to get these default colors (inc. background)
> from the current theme.

You already filed bug 1462416.

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d5950b441d27
Add setBadgeTextColor and getBadgeTextColor methods to browserAction r=mixedpuppy

Comment 25

[Andreea Pavel [:apavel]]

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

Comment 26

[CosminB]

Attachment: Bug1424620.png

This issue is verified as fixed on Firefox 63.0a1 (20180801223951) under Win 7 64-bit and Mac OS X 10.13.3.

browser.browserAction.setBadgeTextColor({ color: '#de0b08 ' }); 
browser.browserAction.setBadgeBackgroundColor({ color: '#00FF00' });

were used in the testing extensions.

Please see the attached screenshot.

Comment 27

[Will Bamberg [:wbamberg]]

I've added:
https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/browserAction/getBadgeTextColor
https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/browserAction/setBadgeTextColor

I've updated the compat data too, but it's not deployed yet.

Please let me know if this covers it or if we need anything else.

Comment 28

[Oriol Brufau [:Oriol]]

Yes, that's good, thanks