Thunderbird traditionally includes the text for toolbar buttons because there are so many and it might be difficult to keep track what icon means what. browerAction toolbar buttons are traditionally without text.

I think we should consider one of the following paths:

1. Make browser action buttons not have text, even if the rest of the toolbar has it
2. Allow setting an empty toolbar text (omitting default_title or using default_title: "" currently makes it the extension name)

For the second case this should also be reflected in documentation somewhere.
Current workaround: default_title: " "

I'm leaning towards (1) because with (2) the tooltip would be empty. A tooltip would still be nice.

A third and simple to implement option would be, to default to the current implementation only, if no icon is specified. As soon as an icon is specified, the text could be left out, as the icon itself will be enough to identify the button.

But in general, I think it is not a good idea to miss-use default_title (which is specified to set the tool-tip). In an ideal world, there should be a dedicated manifest entry for the label of the button.

As this seems to be burried in ToolbarButtonAPI, a change here will probably affect

• browserAction
• composerAction
• messageDisplayAction

It does not look to complicated, but it has to be decided, how this shall be fixed:

(1) Make browser action buttons not have text, even if the rest of the toolbar has it
(2) Allow setting an empty toolbar text
(3) Introduce default_label and use that for the button label

Looking at the code, using the image as a switch is harder, so I no longer suggest that.

Comment on attachment 9189456 [details] [diff] [review]
bug1583478_add_independent_label_to_toolbar_actions

This looks fine to me, thank you! A few things to follow up on:

• Can you check what happens if you add this property to a Firefox extension? I'm wondering if it will just warn or error out.
• Let's make sure we have documentation about this, noting that this is specific to Thunderbird.

Thanks for that fast review!

For documentation, we are independent now. We have this:
https://developer.thunderbird.net/add-ons/mailextensions/supported-manifest-keys

And that links browser_action to our own API doc, which is generated from the source. So the new thing will show up there as soon as it hits c-c and Geoff re-runs the source-to-doc script.

In Firefox, this produces warnings about unsupported entries, but runs.

• Do you want me to add a "only supported in Thunderbird" remark to the scheme? We have not done that for any of our own APIs. But this is special, as we override an existing one...
• I would like to see this in Beta and TB78. Are you ok with that?

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/72756ab0bf5c
Add independent label to toolbar actions. r=Fallen

(In reply to John Bieling (:TbSync) from comment #6)

Thanks for that fast review!

For documentation, we are independent now. We have this:
https://developer.thunderbird.net/add-ons/mailextensions/supported-manifest-keys

And that links browser_action to our own API doc, which is generated from the source. So the new thing will show up there as soon as it hits c-c and Geoff re-runs the source-to-doc script.

In Firefox, this produces warnings about unsupported entries, but runs.

• Do you want me to add a "only supported in Thunderbird" remark to the scheme? We have not done that for any of our own APIs. But this is special, as we override an existing one...

Yeah some sort of tag would be good.

• I would like to see this in Beta and TB78. Are you ok with that?

Uplifts are usually not my decision, but the impact of this patch may be minimal enough that we can backport so I'd be ok with it.

C-B: https://treeherder.mozilla.org/jobs?repo=try-comm-central&revision=a23b810e79e2151d6427905e3eb9b13bde3bed92

Comment on attachment 9189456 [details] [diff] [review]
bug1583478_add_independent_label_to_toolbar_actions

[Approval Request Comment]
User impact if declined:
ESR will not benefit from this new feature

Testing completed (on c-c, etc.):
B-C: https://treeherder.mozilla.org/jobs?repo=try-comm-central&revision=a23b810e79e2151d6427905e3eb9b13bde3bed92

Risk to taking this patch (and alternatives if risky):
I hope none.

Comment on attachment 9189456 [details] [diff] [review]
bug1583478_add_independent_label_to_toolbar_actions

[Triage Comment]
Approved for beta

Thunderbird 84.0b3
https://hg.mozilla.org/releases/comm-beta/rev/ed1e6258e0f5

Comment on attachment 9189456 [details] [diff] [review]
bug1583478_add_independent_label_to_toolbar_actions

[Triage Comment]
Approved for esr78

Comment on attachment 9189456 [details] [diff] [review]
bug1583478_add_independent_label_to_toolbar_actions

This doesn't apply on c-esr78. The toolbar rules that are being changed in messenger.css are not even in that file.

Try run with updated patch for esr78:
https://treeherder.mozilla.org/jobs?repo=try-comm-central&revision=9acd1a56251a5755bd12a87276bbe174ffe59c30

Comment on attachment 9192937 [details] [diff] [review]
bug1583478_add_independent_label_to_toolbar_actions_esr78.patch

Patch for ESR78 looks good on try. Too late?

(In reply to John Bieling (:TbSync) from comment #17)

Comment on attachment 9192937 [details] [diff] [review]
bug1583478_add_independent_label_to_toolbar_actions_esr78.patch

Patch for ESR78 looks good on try. Too late?

I've got no problem with running another build to pick this up. Wayne, any objection?

tl;dr I would rather ask this question, is uplifting in 78.6.1 in January reasonable?

Building is cheap. So I ask for other reasons: foremost, as a general rule we want to rebuild/take late uplifts only for items that cannot reasonably be delayed (unless it is a ride along), we'd rather not have to retest, bug is not a regression, my impression is this is not a serious problem, and ...

(In reply to John Bieling (:TbSync) from comment #10)

Comment on attachment 9189456 [details] [diff] [review]
bug1583478_add_independent_label_to_toolbar_actions

[Approval Request Comment]
User impact if declined:
ESR will not benefit from this new feature

Will specific, important add-ons be blocked and users suffer if the uplift doesn't happen until 78.6.1?

Testing completed (on c-c, etc.):
B-C: https://treeherder.mozilla.org/jobs?repo=try-comm-central&revision=a23b810e79e2151d6427905e3eb9b13bde3bed92

Risk to taking this patch (and alternatives if risky):
I hope none.

Agreed. But this touches themes, in a trivial way perhaps, but themes have been problematic for us.

Welcome feedback.

Wayne is probably right. I was just giving it a shoot, as I of course want this in ESR as fast as possible. But I do not want to cause an extra test round. It was me who was too late and I signaled Wayne already, that delaying the uplift is fine. It was just too tempting to request it getting uplifted after I was able to fix the patch before shipping.

As this is a new feature, no add-on is using it (have not seen this in review) and so no add-on is blocked.

Let's postpone this. Sorry for the noise. Will do better next time.

use async: "callback" instead of async: true

Comment on attachment 9194386 [details] [diff] [review]
bug1583478_add_independent_label_to_toolbar_actions_esr78.patch

[Triage Comment]
Approved for esr78

Thunderbird 78.6.1:
https://hg.mozilla.org/releases/comm-esr78/rev/346c441c3a55