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

[contextMenus] create or update contextMenus in a single call #312

Open
hanguokai opened this issue Oct 30, 2022 · 3 comments
Open

[contextMenus] create or update contextMenus in a single call #312

hanguokai opened this issue Oct 30, 2022 · 3 comments
Labels
enhancement Enhancement or change to an existing feature 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

@hanguokai
Copy link
Member

The Problem

Current situation, contextMenus.create() or contextMenus.update() needs to specify an ID parameter. If the contextMenus with the ID already exist for create() or doesn't exist for update(), the method will fail (no effect).

Note: there is no contextMenus.get() method. So to ensure that the create or update is executed successfully, developers need to do in this way:

contextMenus.remove/removeAll(() => contextMenus.create());

The Solution

I suggest improving this API to support create or update in a single call. For example, add an override option in createProperties and updateProperties object, or add a new method contextMenus.createOrUpdate().

@oliverdunk
Copy link
Member

I think in some situations (Chrome at least) this results in a chrome.runtime.lastError which is impossible to avoid. Usually you can check that property to hide the error from the console but I haven't been able to get that to play nicely with the promises in MV3.

@xeenon xeenon added the neutral: safari Not opposed or supportive from Safari label Dec 8, 2022
@Rob--W Rob--W added neutral: chrome Not opposed or supportive from Chrome neutral: firefox Not opposed or supportive from Firefox and removed agenda Discuss in future meetings labels Dec 8, 2022
@hanguokai
Copy link
Member Author

I think in some situations (Chrome at least) this results in a chrome.runtime.lastError which is impossible to avoid. Usually you can check that property to hide the error from the console

Note that:

  1. if you never change your context menu, you can just ignore the runtime error.
  2. if you want to update your context menu, you can't ignore the runtime error. For example, update title from 'menu-title-1' to 'menu-title-2'. In this case, you need to catch the error and remove the menu then create it.

Current contextMenus api is not consistent with other extension apis. For example, alarms.create() and notifications.create() allow override(update) current alarm/notification if alarm name or notificationId is already exist.

If browser vendors don't want to create new api, I suggest change current behavior of contextMenus.create() and contextMenus.update(), don't throw runtime error, instead, if no menu then create it, if have existing menu then update it.

@hanguokai
Copy link
Member Author

Another problem of contextMenus api is that you can't control a single menu order. If you care about the menus order, you have to removeAll() first then create all menus when you update them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement or change to an existing feature 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

4 participants