Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inconsistency: action.setIcon() & action.setBadgeBackgroundColor() #336

Open
erosman opened this issue Dec 11, 2022 · 10 comments
Open

Inconsistency: action.setIcon() & action.setBadgeBackgroundColor() #336

erosman opened this issue Dec 11, 2022 · 10 comments
Labels
implemented: firefox Implemented in Firefox implemented: safari Implemented in Safari inconsistency Inconsistent behavior across browsers supportive: chrome Supportive from Chrome

Comments

@erosman
Copy link

erosman commented Dec 11, 2022

There are some minor inconsistencies.
N.B. Only Firefox & Chrome checked.

setIcon()

1. Resetting to the manifest icon

Firefox

browserAction.setIcon() & action.setIcon()

If each one of imageData and path is one of undefined, null or empty object:

  • If tabId is specified, and the tab has a tab-specific icon set, then the tab will inherit the icon from the window to which it belongs.
  • If windowId is specified, and the window has a window-specific icon set, then the window will inherit the global icon.
  • Otherwise, the global icon will be reset to the manifest icon.
browser.action.setIcon({});

Chrome

setIcon

Either the path or the imageData property must be specified.

chrome.action.setIcon({});
// Error: Either the path or imageData property must be specified.

setBadgeBackgroundColor()

1. Resetting to the default value

Firefox

browserAction.setBadgeBackgroundColor() & action.setBadgeBackgroundColor()

null. If a tabId is specified, it removes the tab-specific badge background color so that the tab inherits the global badge background color. Otherwise it reverts the global badge background color to the default value.

browser.action.setBadgeBackgroundColor({color: null});

Chrome

setBadgeBackgroundColor

Resetting to the default value

chrome.action.setBadgeBackgroundColor({color: null});
// Error in invocation of action.setBadgeBackgroundColor(object details, optional function callback): Error at parameter 'details': Missing required property 'color'.

2. transparent

Firefox

browser.action.setBadgeBackgroundColor({color: 'transparent'});

Chrome

chrome.action.setBadgeBackgroundColor({color: 'transparent'});
// Error: The color specification could not be parsed.

Footnote

See also: #216

@hanguokai
Copy link
Member

Yes, action.setBadgeTextColor() also has this minor inconsistency. Since it can be worked around, it is generally not a problem.

@xeenon
Copy link
Collaborator

xeenon commented Oct 3, 2023

Safari supports null resetting the action properties. We also plan to support windowId.

@xeenon xeenon added the supportive: safari Supportive from Safari label Oct 3, 2023
@carlosjeurissen
Copy link
Contributor

carlosjeurissen commented Oct 18, 2023

So basically the inconsistency is Chrome does not allow resetting the properties? Potentially we can just upgrade this issue to propose allowing to reset the action properties.

@oliverdunk is this something Chrome would want to implement? Seems an open issue has been made already: https://bugs.chromium.org/p/chromium/issues/detail?id=1488303

Safari support null resetting the action properties.

@xeenon Wonderful! Is this something recently implemented?

@oliverdunk oliverdunk added the follow-up: chrome Needs a response from a Chrome representative label Oct 24, 2023
@xeenon
Copy link
Collaborator

xeenon commented Nov 9, 2023

Safari has always supported null reseting, IIRC.

@xeenon xeenon added implemented: safari Implemented in Safari and removed supportive: safari Supportive from Safari labels Dec 7, 2023
@zombie zombie added the implemented: firefox Implemented in Firefox label Dec 7, 2023
@zombie
Copy link
Collaborator

zombie commented Dec 7, 2023

Similar to Safari, I think this was supported from the get go.

@Rob--W Rob--W added supportive: chrome Supportive from Chrome and removed follow-up: chrome Needs a response from a Chrome representative labels Mar 18, 2024
@Rob--W
Copy link
Member

Rob--W commented Mar 18, 2024

Devlin expressed support for resetting to the default. The crbug to track is at https://issues.chromium.org/issues/40073862

@carlosjeurissen
Copy link
Contributor

carlosjeurissen commented May 16, 2024

The same inconsistency applies to action.setPopup. Setting the popup to null resets it in Firefox, while in Chrome it throws an exception.

@xeenon Just tested setting popup to null in Safari 17.4.1 and contrary to what is stated on MDN, it behaves as if it is set to an empty string.
https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/browserAction/setPopup

@xeenon
Copy link
Collaborator

xeenon commented May 20, 2024

@carlosjeurissen Hmm thanks. What about Safari Technology Preview?

@carlosjeurissen
Copy link
Contributor

@xeenon Seems it has the correct behaviour in the Technology Preview Release 194 (Safari 17.4, WebKit 19619.1.11.111.2)

@xeenon
Copy link
Collaborator

xeenon commented May 20, 2024

@carlosjeurissen Thanks for checking! I'll make a note to update MDN.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
implemented: firefox Implemented in Firefox implemented: safari Implemented in Safari inconsistency Inconsistent behavior across browsers supportive: chrome Supportive from Chrome
Projects
None yet
Development

No branches or pull requests

7 participants