1527979 - Let browser.menus.create return a Promise

Status: NEW

(WebExtensions :: Frontend, enhancement, P3)

Description

[Rob Wu [:robwu]]

Currently, the browser.menus.create API returns an integer or string, which is used as the menuId parameter in the menus.update/menus.remove API. It also appears in the event details (e.g. onClicked event).

This API design was inherited from Chromium's chrome.contextMenus.create method, to allow code like this to work:

menuId = chrome.contextMenus.create(...);
chrome.contextMenus.update(menuId);

The problem with this is that the menu registration is actually asynchronous, and may potentially fail (e.g. when a duplicate ID is used). In order to catch such, extension developers are currently forced to write code like this:

await new Promise(resolve => {
  menuId = browser.menus.create(..., () => {
    if (chrome.runtime.lastError) { /* handle error */ }
    resolve();
  });
});

The menus.create API could be much more dev-friendly (and consistent with other extension APIs) if it returned a promise, so that the usage can be simplified to:

try {
  menuId = await browser.menus.create(...);
} catch (e) {
  /* handle error */
}

Because of backwards-compatibility, we cannot just turn the method in an async function. At the least, we would have to update the API to continue accepting the return value of the browser.menus.create API, i.e. that both of the following snippets work:

menuIdOld = browser.menus.create(...);      // Would be a Promise
menuIdNew = await browser.menus.create(...);// Would be string or integer
browser.menus.update(menuIdOld, ...)
browser.menus.update(menuIdNew, ...);

Comment 1

[Mike Conca [:mconca]]

I love this idea. I'm leaving the priority unset so that it goes through triage, but unless there is some reason to not do this, we should add this to the backlog.

Comment 2

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

This may be still tricky from a compatibility point of view even with the approach described in comment 0, e.g. the approach described may not work correctly if the extension tries to do something with the return value that would only work if it is a string or an integer (e.g. if the extension may try to store the return value somewhere as it is, e.g. in the storage.local API).

Comment 3

[Rob Wu [:robwu]]

We can implement this change without compatibility layer as a part of manifest v3, where we are allowed to make backwards-incompatible changes.

To minimize the compatibility risk, I was previously thinking of returning an object of the following form:

retval = {
  // Promise
  then(func?, func?) {},
  catch(func) {},
  finally(func) {},

  [Symbol.toPrimitive]() { return menuId; },
  toJSON() { return menuId; },
};

This would cover most cases, including scenarios where extensions serialize menuId:

retval == 'menuid'
browser.menus.update(retval, ...);
browser.menus.update(await retval);

It is however not perfact, as this would not work:

typeof retval == 'string'
retval === 'menuid'

Boolean coercion such as !retval would also fail, but there is no meaningful scenario where the browser.menus.create did not throw and returned a falsey value before.