Allow to set browserAction badge text color

VERIFIED FIXED in Firefox 63

Status

enhancement
P5
normal
VERIFIED FIXED
a year ago
7 months ago

People

(Reporter: Oriol, Assigned: Oriol)

Tracking

({dev-doc-complete})

unspecified
mozilla63

Firefox Tracking Flags

(firefox63 verified)

Details

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

Attachments

(2 attachments)

(Assignee)

Description

a year ago
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.
Severity: normal → enhancement

Updated

a year ago
Whiteboard: [design-decision-needed]

Updated

a year ago
Priority: -- → P5
(Assignee)

Comment 1

a year ago
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.
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
(Assignee)

Comment 3

a year ago
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.
I wonder if we could modify style, font styles, transform, etc. etc.
We tentatively approved this in the design-decision meeting, but wanted to run this by UX before officially approving. Philip, could you help us?
Flags: needinfo?(pwalmsley)
Apologies; we did not tentatively approve this; we explicitly made no design-decision on this bug during the meeting.
(Assignee)

Comment 7

a year ago
In comment 2 you could have mentioned that today the meeting was 1 hour earlier...
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

a year ago
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.
(Assignee)

Comment 10

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

Updated

a year ago
Flags: needinfo?(pwalmsley)
Emanuela: this question is more in your wheelhouse, could you give your guidance on this request?
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.
Flags: needinfo?(emanuela)

Updated

10 months ago
Product: Toolkit → WebExtensions

Comment 13

10 months ago
I don't have any concern at the moment. Let's move forward
Flags: needinfo?(emanuela)

Updated

10 months ago
Whiteboard: [design-decision-needed] → [design-decision-approved][browserAction]
(Assignee)

Comment 14

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

Comment 15

10 months ago
(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.
(Assignee)

Comment 16

10 months ago
(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.
(Assignee)

Comment 17

10 months ago
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
(Assignee)

Comment 18

10 months ago
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.
Assignee: nobody → oriol-bugzilla
Status: NEW → ASSIGNED
Comment hidden (mozreview-request)
(Assignee)

Comment 20

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

Updated

10 months ago
See Also: → 1474110
Comment hidden (mozreview-request)

Comment 22

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

Comment 23

9 months ago
(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

9 months ago
Pushed by ccoroiu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d5950b441d27
Add setBadgeTextColor and getBadgeTextColor methods to browserAction r=mixedpuppy
Keywords: checkin-needed

Comment 25

9 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/d5950b441d27
Status: ASSIGNED → RESOLVED
Last Resolved: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63

Comment 26

9 months ago
Posted image 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.

Updated

9 months ago
Status: RESOLVED → VERIFIED
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.
Flags: needinfo?(oriol-bugzilla)
(Assignee)

Comment 28

7 months ago
Yes, that's good, thanks
Flags: needinfo?(oriol-bugzilla)
You need to log in before you can comment on or make changes to this bug.