w3c / webextensions

Charter and administrivia for the WebExtensions Community Group (WECG)
Other
575 stars 50 forks source link

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

Open Juraj-Masiar opened 2 months ago

Juraj-Masiar commented 2 months ago

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

xeenon commented 2 months ago

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

tophf commented 2 months ago

implemented: firefox

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

xeenon commented 2 months ago

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.)

zombie commented 2 months ago

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 commented 2 months ago

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):

If the title property is set to anything else:

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.

Juraj-Masiar commented 2 months ago

@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 commented 1 month ago

@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.