Toolbar buttons WebExtensions API

RESOLVED FIXED in Thunderbird 64.0

Status

enhancement
RESOLVED FIXED
9 months ago
8 months ago

People

(Reporter: darktrojan, Assigned: darktrojan)

Tracking

(Blocks 1 bug)

Trunk
Thunderbird 64.0
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

9 months ago
I've begun working on an API so extensions can add buttons to toolbars. It'd be the Thunderbird equivalent of browserAction. We have a lot of toolbars, which makes life difficult. Currently what I have is a different name for each different button: messengerAction, composeAction, etc., but I think it'd work better to be more like Firefox's contextMenus API, where in code you create any number of menu items and specify which menu(s) to add them to.
(Assignee)

Updated

9 months ago
No longer depends on: 1469238, 1482040
I think we should definitely support browserAction to the main toolbar, for compatibility. I'd be carefully with allowing any number of items, in Firefox/Chrome I suspect this was done to avoid an overflow of buttons by a single add-on. I can imagine cases where more than one per toolbar would make sense, but we shouldn't allow an unlimited number of buttons.

This would indeed be a great API to add, it would get us a step closer to making the gecko profiler add-on work.
Component: General → Add-Ons: Extensions API
(Assignee)

Comment 2

8 months ago
Posted patch 1487008-webext-actions-1.diff (obsolete) β€” β€” Splinter Review
I've created "browser"_action for the 3-pane tab and compose_action for the compose window. It should be easy from here to add more actions for other toolbars like the message display toolbar.

ExtensionPopups.jsm is more or less the same as the one in browser/components/extensions, but we don't have CustomizableUI so I've had to hack it a bit. There's one or two bits still missing, like the browser_style stylesheets.
Attachment #9008612 - Flags: feedback?(philipp)
Comment on attachment 9008612 [details] [diff] [review]
1487008-webext-actions-1.diff

Review of attachment 9008612 [details] [diff] [review]:
-----------------------------------------------------------------

This looks great overall, I like that it is so easy to add new _actions.

::: mail/themes/shared/mail/messengercompose.css
@@ +10,5 @@
>    --lwt-background-tiling: no-repeat;
>  }
>  
> +toolbarbutton[data-extensionid] > .toolbarbutton-icon {
> +  list-style-image: var(--webextension-toolbar-image);

These styles probably need some additions for the default icon, similar to how it is in the m-c code. You might have that planned though.
Attachment #9008612 - Flags: feedback?(philipp) → feedback+
(Assignee)

Comment 4

8 months ago
Posted patch 1487008-webext-actions-2.diff (obsolete) β€” β€” Splinter Review
I'm not convinced this is perfect, but it's good enough to use.
Attachment #9008612 - Attachment is obsolete: true
Attachment #9009802 - Flags: review?(philipp)
Comment on attachment 9009802 [details] [diff] [review]
1487008-webext-actions-2.diff

Review of attachment 9009802 [details] [diff] [review]:
-----------------------------------------------------------------

Fabulous, looking forward to getting this one in. Some minor comments:

::: mail/components/extensions/ExtensionPopups.jsm
@@ +50,5 @@
> +//   if (AppConstants.platform === "win") {
> +//     stylesheets.push("chrome://browser/content/extension-win-panel.css");
> +//   }
> +//   return stylesheets;
> +// });

Can you remove the commented out code? If we need this later we can still add it in, by then we'd probably have to sync with browser's ExtensionPopups.jsm anyway.

::: mail/components/extensions/ExtensionToolbarButtons.jsm
@@ +195,5 @@
> +  }
> +
> +  // Update the toolbar button |node| with the tab context data
> +  // in |tabData|.
> +  updateButton(node, tabData, sync = false) {

This file contains a mix of undocumented functions, fully documented functions, and such where there is just a simple comment. Can you consider making them all full jsdoc comments?
Attachment #9009802 - Flags: review?(philipp) → review+
Comment on attachment 9009802 [details] [diff] [review]
1487008-webext-actions-2.diff

Review of attachment 9009802 [details] [diff] [review]:
-----------------------------------------------------------------

::: mail/components/extensions/ExtensionPopups.jsm
@@ +25,5 @@
> +const {
> +  makeWidgetId,
> +} = ExtensionCommon;
> +
> +

nit: double empty line

@@ +513,5 @@
> +    }
> +
> +    this.attached = true;
> +
> +

nir: here too double empty line

::: mail/components/extensions/schemas/browserAction.json
@@ +50,5 @@
> +    ]
> +  },
> +  {
> +    "namespace": "browserAction",
> +    "description": "Use browser actions to put icons in the main browser toolbar, to the right of the address bar. In addition to its icon, a browser action can also have a tooltip, a badge, and a popup.",

Hmm. Might need some updates :)
(Assignee)

Comment 7

8 months ago
Posted patch 1487008-webext-actions-3.diff (obsolete) β€” β€” Splinter Review
This is more or less what I'd intended to post last time (but didn't for some reason). Asking for review again because of the large number of changes.
Attachment #9009802 - Attachment is obsolete: true
Attachment #9011726 - Flags: review?(philipp)
Comment on attachment 9011726 [details] [diff] [review]
1487008-webext-actions-3.diff

Review of attachment 9011726 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me, thanks for the update!
Attachment #9011726 - Flags: review?(philipp) → review+
(Assignee)

Comment 9

8 months ago
Rebased to avoid dependency on bug 1469238.
Attachment #9011726 - Attachment is obsolete: true
Attachment #9013178 - Flags: review+
(Assignee)

Updated

8 months ago
Keywords: checkin-needed

Comment 10

8 months ago
Sorry, I have no indication whether this could possibly break anything:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=ddfa17b95d8d90d7fcbabcd61c1f185881181f48

Comment 11

8 months ago
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/630fc01f8e33
Toolbar buttons WebExtensions API. r=philipp
Status: ASSIGNED → RESOLVED
Last Resolved: 8 months ago
Keywords: checkin-needed
Resolution: --- → FIXED

Updated

8 months ago
Target Milestone: --- → Thunderbird 64.0
You need to log in before you can comment on or make changes to this bug.