1618580 - Add devtools.panels.ExtensionPanel.setIcon to allow changing panel's icon after creation

Status: NEW

(WebExtensions :: Developer Tools, enhancement, P3)

Description

[Tooru Fujisawa [:arai]]

Attachment: Example patch that adds devtools.panels.ExtensionPanel.setIcon

This is a solution for bug 1617555

Currently DevTools panel's icon can be specified only when creating it (iconPath parameter for devtools.panels.create.
this causes bug 1617555 (cannot immediately reflect theme switch to icon).

one possible solution is to add an API to change the icon.

Proposal

Add a new API setIcon(iconPath) to devtools.panels.ExtensionPanel, that allows changing the panel's icon after creating the panel.

Basic Usage

// Create a panel with `icon-1.png` icon
const panel = await browser.devtools.panels.create("Test Panel", "/icon-1.png", "/test.html");

// Change the panel's icon to `icon-2.png`
panel.setIcon("/icon-2.png");

Solution for bug 1617555

// Create with the icon for the current theme.
const panel = await browser.devtools.panels.create("Test Panel", iconForCurrentTheme(), "/test.html");

browser.devtools.panels.onThemeChanged.addListener(() => {
  // Change the panel's icon for the current theme.
  panel.setIcon(iconForCurrentTheme());
});

function iconForCurrentTheme() {
  if (browser.devtools.panels.themeName == "dark") {
    return "/icon-dark.png";
  }
  return "/icon-light.png";
}

Another solution for bug 1617555

Just to be clear, bug 1617555 can be solved in 2 ways:

• add an API to change the icon (this bug)
• standardize SVG context fill and use it (bug 1617554, bug 1553996, bug 1351236)

but, to my understanding, the latter seems to be much hard work (needs both spec change and implementation change),
so that I'm proposing this API.

Implementation example

In the attached patch.

Other possible designs for this API

Allow specifying multiple icon sizes

Related to bug 1418299.

There are setIcon API for some interfaces [1][2][3], and all of them accept an object instead of a string.
With the object, icon can be specified for multiple sizes, with either ImageData or path.

Allowing it might be useful, but it doesn't align with devtools.panels.create's iconPath parameter.
So, if we go this way, solving bug 1418299 at the same time would be better.

Allow updating title at the same time

If there's demand for changing title as well (I'm not sure), we could have something like update(title, iconPath) method instead.
This seems to align with devtools.panels.Button.update(iconPath, tooltipText, disabled) for devtools.panels.ExtensionPanel.createStatusBarButton(iconPath, tooltipText, disabled).

[1] https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/sidebarAction/setIcon
[2] https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/browserAction/setIcon
[3] https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/pageAction/setIcon

Comment 1

[Luca Greco [:rpl] [:luca] [:lgreco]]

Needinfo-ed myself to come back to this and look into the proposed API (also linking this to Bug 1617554 as a see also).

Comment 2

[Luca Greco [:rpl] [:luca] [:lgreco]]

It looks that in Chrome the extension devtools panel icons is adapted automatically when the devtools panel settings are changed to set the dark theme (and so it would be nice if Bug 1553996 could get fixed at some point and provide a default behavior that is shared by both Firefox and Chrome), but fixing Bug 1553996 would not help with Bug 1418299, and so in the end I agree that providing a new setIcon method on the panel API object could still be a valuable addition.

Personally I think that for consistency it would be good if the panel.setIcon method could provide a type signature similar to the one used for the other existing setIcon methods (or to aim for that, e.g. an initial version could only support icon paths, and in a follow up we may evaluate if it could be useful to also allow the extension to pass an icon in form of image data), besides tabId and windowId which do not make sense for the panel API object (given that the panel object is already related to a specific devtools toolbox).

Providing a panel.setTitle may also make sense if the extension wants to change the title for localization reasons, but I would defer that to a follow up and separate bugzilla issue.

I'm needinfo-ing Philipp to get his feedback from a product perspective (and for his sign-off, if the feedback is positive from his side too).

As a side note:

• I have closed Bug 1617554 as a duplicate of Bug 1553996
• I have not closed yet Bug 1617555 and Bug 1418299 as a duplicates of this one (but I think that they should be closed as duplicate of this one if proceed to implementing the approach proposed in this issue)