Closed Bug 1424620 Opened 6 years ago Closed 6 years ago

Allow to set browserAction badge text color

Categories

(WebExtensions :: Frontend, enhancement, P5)

enhancement

Tracking

(firefox63 verified)

VERIFIED FIXED
mozilla63
Tracking Status
firefox63 --- verified

People

(Reporter: Oriol, Assigned: Oriol)

References

Details

(Keywords: dev-doc-complete, Whiteboard: [design-decision-approved][browserAction])

Attachments

(2 files)

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
Whiteboard: [design-decision-needed]
Priority: -- → P5
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
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.
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!
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.
Philip hasn't been active the latest months. Can somebody else from UX be asked instead?
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)
Product: Toolkit → WebExtensions
I don't have any concern at the moment. Let's move forward
Flags: needinfo?(emanuela)
Whiteboard: [design-decision-needed] → [design-decision-approved][browserAction]
Are both proposals from comment 3 approved, or just one of them?
(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.
(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.
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
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
I went ahead and added tabId and windowId, is this OK?
And not much sure if the name should be setBadgeTextColor or setBadgeColor
See Also: → 1474110
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+
(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.
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
https://hg.mozilla.org/mozilla-central/rev/d5950b441d27
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Attached 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.
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)
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.

Attachment

General

Created:
Updated:
Size: