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

Proposal: Add support for setting the main context menu icon #592

Open
Juraj-Masiar opened this issue Apr 14, 2024 · 8 comments
Open

Proposal: Add support for setting the main context menu icon #592

Juraj-Masiar opened this issue Apr 14, 2024 · 8 comments
Labels
neutral: chrome Not opposed or supportive from Chrome neutral: firefox Not opposed or supportive from Firefox neutral: safari Not opposed or supportive from Safari

Comments

@Juraj-Masiar
Copy link

Similarly as you can change the main toolbar icon using action.setIcon, you should be able to change your main context menu icon.

The contextMenu.create already supports "icons" parameter, but only in Firefox and only for sub-menus.

Motivation

I was implementing icon change in my extension and while I was able to change the icon in the app UI, favicon and toolbar, there is still this one place that shows the default icon, which makes it look confusing and inconsistent:

See also:
Proposal: API for setting sidebar title and icon

@github-actions github-actions bot added needs-triage: chrome Chrome needs to assess this issue for the first time needs-triage: firefox Firefox needs to assess this issue for the first time needs-triage: safari Safari needs to assess this issue for the first time labels Apr 14, 2024
@xeenon xeenon added implemented: safari Implemented in Safari implemented: firefox Implemented in Firefox and removed needs-triage: safari Safari needs to assess this issue for the first time needs-triage: firefox Firefox needs to assess this issue for the first time labels Apr 15, 2024
@xeenon
Copy link
Collaborator

xeenon commented Apr 15, 2024

Safari supports menu item icons, as of Safari Technology Preview 192 with the same API as Firefox.

@xeenon xeenon added the needs-triage: firefox Firefox needs to assess this issue for the first time label Apr 15, 2024
@tophf
Copy link

tophf commented Apr 16, 2024

implemented: firefox

But Firefox doesn't implement setting the main icon i.e. for the top folder of the extension menu.

@xeenon xeenon removed implemented: safari Implemented in Safari implemented: firefox Implemented in Firefox labels Apr 16, 2024
@xeenon
Copy link
Collaborator

xeenon commented Apr 16, 2024

But Firefox doesn't implement setting the main icon i.e. for the top folder of the extension menu.

Oh! I misunderstood this issue then. Safari does allow setting icons for menu items, but the top-level auto generated menu item does not have an icon in Safari, unlike Firefox. (In general we don't use icons in context menus on Apple platforms, so we likely won't add an icon for this generated menu item.)

@xeenon xeenon added the neutral: safari Not opposed or supportive from Safari label Apr 16, 2024
@zombie
Copy link
Collaborator

zombie commented Apr 25, 2024

Using the extension icon for the top level item in context menus was a purposeful design decision on our part, because the title is also controllable by the extension, and without the icon, there would be no clues for the user to know where the menu item is coming from, opening potential for abuse.

This is different from the browser action icon, which mostly stays in the same place, and has other clues like tooltips and context menus, so is much easier for user to discern the originating extension.

@carlosjeurissen
Copy link
Contributor

carlosjeurissen commented Apr 25, 2024

To combat abuse, for the top level menu item we can use the following behaviour:

If the title property is missing or set to the extension name (for backwards compatibility):

  • allow to change the menu icon using the icons property
  • the title of the top level menu item will be set to the name of the extension

If the title property is set to anything else:

  • enforce the extension icon as specified in the manifest

This allows us to combat abuse while also not requiring any new API methods to be introduced for the feature request in this issue. @Juraj-Masiar would this cover your use case?

In general it seems like an oversight title will simply be kept empty if not provided. This case would not be present in Chrome as title is required in Chrome except for separators.

@Rob--W Rob--W added follow-up: chrome Needs a response from a Chrome representative neutral: firefox Not opposed or supportive from Firefox and removed needs-triage: firefox Firefox needs to assess this issue for the first time labels Apr 25, 2024
@Juraj-Masiar
Copy link
Author

@carlosjeurissen sadly no, that workaround would not cover the use cases I'm going for.

I agree with @zombie , that's some nice attack vector, since extension menus indeed changes a lot depending also on what user have clicked.
But I think this could be mitigated too, by limiting icons to be specified in the manifest file, using some new alt_icons key.
This would limit the ability to impersonate any extension dynamically, plus attacker would be easily identified (we could even display alt icons somewhere in the about:addons addon details page for everyone to see).

@carlosjeurissen
Copy link
Contributor

@Juraj-Masiar Alternatively you can simply just change the 16px and 32px icons in the main icons object in the manifest and make sure the action icons are set to something else.

@oliverdunk
Copy link
Member

We had a discussion on the Chrome team about this. We are opposed to supporting this for top-level menu items, given the abuse risk. We are open to supporting this for sub-level menu items although it isn't something we are likely to prioritize.

@oliverdunk oliverdunk added neutral: chrome Not opposed or supportive from Chrome and removed follow-up: chrome Needs a response from a Chrome representative needs-triage: chrome Chrome needs to assess this issue for the first time labels Jul 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
neutral: chrome Not opposed or supportive from Chrome neutral: firefox Not opposed or supportive from Firefox neutral: safari Not opposed or supportive from Safari
Projects
None yet
Development

No branches or pull requests

7 participants