New api: shortcut changed event

[hanguokai]

Use Case

Developers can use browser.commands.getAll() to display a shortcuts table for users. But at present, when the user changed a shortcut, there is no changed event for extension to know it for updating the shortcuts table.

New API

browser.commands.onRegistrationChanged.addListener( command => {} )

PS: I don't care about the api name, as long as it meets the functional requirements. Browser vendors can use other names, as long as they are consistent.

[Rob--W]

P.S. This was originally filed at #300 but split off.

This feature request looks reasonable to me.
In Chrome and Firefox, there is external browser UI to allow the user to change a shortcut.
In Firefox, there is also the commands.update API to change the shortcuts of an extension itself.

At present, extensions cannot observe shortcut changes, other than by polling (i.e. calling getAll repeatedly).

[Rob--W]

There is support from Chrome and Firefox for the ability to be notified of shortcut changes. The implementation of it is a low priority.

Safari does have a commands API, but there is no way to change the shortcut yet. Therefore the event would not be useful in Safari.

Personal note on API name, for consistency with other extension APIs, I suggest to use: `commands.onChanged

[Rob--W]

Someone opened a bug and submitted a patch to Firefox at https://bugzilla.mozilla.org/show_bug.cgi?id=1801531

I didn't find anything on Chromium's issue tracker at https://bugs.chromium.org/p/chromium/issues/list?q=component%3APlatform%3EExtensions%3EAPI%20commands&can=2.

[hanguokai]

@Rob--W Chrome bug is here https://bugs.chromium.org/p/chromium/issues/detail?id=1375790

[gregorypappas]

Hi, I'm implementing this in Firefox and would appreciate some guidance on what this should look like. Here are some ideas:

When a command's description or shortcut is changed, browser.commands.onChanged's callback will pass one argument with...

  // A single Command object with two additional properties (current patch)
  {
    "name": "foo",
    "description": "new foobar command description",
    "oldDescription": "old foobar command description",
    "shortcut": "Ctrl+Shift+V",
    "oldShortcut": "Ctrl+Shift+Y"
  }

  // A single object with two keys { oldCommand: Command, newCommand: Command }
  {
    "oldCommand": {
      "name": "foo",
      "description": "old foobar command description",
      "shortcut": "Ctrl+Shift+Y"
    },
    "newCommand": {
      "name": "foo",
      "description": "new foobar command description",
      "shortcut": "Ctrl+Shift+V"
    }
  }

Or maybe, we should pass multiple arguments? Idk. I'd just like to get a consensus on what this should look like so we can avoid breaking changes in the future.

@Rob--W Which approach does Mozilla prefer?
@dotproto @oliverdunk Which approach does Google prefer?

[Rob--W]

I wouldn't even pass the description. Anyone interested can use chrome.runtime.getManifest().command to retrieve information about the commands.

I'd fire an event with the command identifier, the old shortcut if any and the new one.

[gregorypappas]

I wouldn't even pass the description

So the idea is that we're limiting this event to only the shortcut itself and nothing else? In that case, I suppose the event shouldn't even fire if an extension calls commands.update and only changes the description of a command.

I'd fire an event with the command identifier, the old shortcut if any and the new one.

I see. In that case, should we pass an object like this:

{
  "name": "foo",
  "newShortcut": "Ctrl+Shift+Y",
  "oldShortcut": "Ctrl+Shift+V"
}

Or three arguments, like this:

(name: String, newShortcut: String, oldShortcut: String)

[Rob--W]

I wouldn't even pass the description

So the idea is that we're limiting this event to only the shortcut itself and nothing else? In that case, I suppose the event shouldn't even fire if an extension calls commands.update and only changes the description of a command.

That's right. The event is designed to help the extension with getting notified about (user-defined) shortcut changes.

One relevant note to keep in mind with this event (and the shortcuts API in general) is that being assigned the shortcut doesn't imply that the shortcut will also trigger the registered command. For example, another extension or even a browser/system-level shortcut can also be assigned the same shortcut and take precedence. For past discussions about this topic, see https://bugzilla.mozilla.org/show_bug.cgi?id=1654403

I'd fire an event with the command identifier, the old shortcut if any and the new one.

I see. In that case, should we pass an object like this:

{
  "name": "foo",
  "newShortcut": "Ctrl+Shift+Y",
  "oldShortcut": "Ctrl+Shift+V"
}

Or three arguments, like this:

(name: String, newShortcut: String, oldShortcut: String)

The object form is more common in the extension APIs.

[gregorypappas]

Alright, thanks so much! I've updated the Firefox patch to align with your feedback.

[oliverdunk]

I haven't discussed this one with the team, but it feels very similar to #346. On the Chrome side, It may be worth having some discussion on if we want to emit both the old and new object or just the changes.

[hanguokai]

Add label implemented: firefox Implemented in Firefox

Firefox implemented this feature in Firefox 115.
MDN docs see https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/commands/onChanged

@oliverdunk look forward to Chrome's implement.