XBL and XUL key handling implementation is currently shared. This is being reworked in Bug 1550058 to allow for disabling XBL.

<key> has some issues like Bug 371900 (which is in turn annoying because it stops us removing inline event handlers in browser.xul, which in turn blocks further improving CSP and eval() defense-in-depth security improvements).

Two main ideas so far:

1: Build a chrome custom element that does pretty much the same thing. The module would add a single event listener then as elements are connected/disconnected they would get added in a map that's consulted in addEventListener.

Pros:

• Would be able to continue using dtd values for keys until they are removed
• Would help identify timing / event order related issues with less effort than (2) - i.e. does <key> handling happen before/after addEventListener handlers registered in chrome?
• May continue to allow platform and frontend integrations with key elements (usually wired up through a [key=foo] attribute (see https://searchfox.org/mozilla-central/search?q=nsGkAtoms%3A%3Akey&path= and https://searchfox.org/mozilla-central/source/toolkit/modules/ShortcutUtils.jsm)

Cons:

• Remains DOM-based for something that doesn't have a DOM representation

2: Use an existing or write a new key shortcut handling JS module. For example, https://searchfox.org/mozilla-central/source/devtools/client/shared/key-shortcuts.js

Pros:

• JS-based API would be generally easier to reuse with other JS, write tests against, extend, etc

Cons:

• More work - see "Pros" list of (1)

At this point further investigation would be helpful. If we could quickly put together a prototype of (1) and it was relatively green, it could make sense to split this up into two separate steps.

Julian and Alex, I'd like to get some feedback about how the KeyShortcuts API has been serving DevTools so far. Specifically:

1. The Electron string syntax for defining keys
2. The API itself

Just exploring options here and would be great to hear from folks who have worked with it closely (and you've both done conversions from existing code to start using it in Bug 1277501 and Bug 1268450).

Sorry for the very slow answer Brian!

I don't have a strong opinion about the Electron key shortcut API. It works fine for us but I would not go ahead and force the rest of m-c to adopt it.
I think having a 1:1 replacement for key will be easier. Migrating out of XUL/XBL and changing the paradigm at the same time feels scary to me :)

Maybe while doing this we can identify some UIs that might be easy to migrate to a KeyShortcut-like API, and file follow ups?

Hope this helps, happy to discuss on IRC/slack if it can help (also note that Alex will be on PTO for 3 weeks starting monday)

Thanks Julian, that's helpful

I pushed up a work in progress patch that uses a plain HTML Custom Element (<moz-key>) and it seems to work for most cases. Some notes:

• There are still some mochitest failures (https://treeherder.mozilla.org/#/jobs?repo=try&revision=c2e281037d1f3a60adf0d4e6d03d29f8ee9f6c06). I think some of those are related to [reserved] attribute which I don't fully understand yet.
• I switched inline [oncommand] handlers to a different HTML attribute so that the attribute gets wired up to a listener. I picked "onselect" instead but any event could do - or we could switch back to a XUL element and continue using oncommand.
• This drops the requirement to have the weird oncommand="//" / oncommand=";" (Bug 371900).

(In reply to Brian Grinstead [:bgrins] from comment #5)

I pushed up a work in progress patch that uses a plain HTML Custom Element (<moz-key>) and it seems to work for most cases. Some notes:

• There are still some mochitest failures (https://treeherder.mozilla.org/#/jobs?repo=try&revision=c2e281037d1f3a60adf0d4e6d03d29f8ee9f6c06). I think some of those are related to [reserved] attribute which I don't fully understand yet.

Mossop, if we were able to get this over the finish line and fully drop the need for <xul:key> outside of XBL, would that change/simplify the approach you are thinking about in Bug 1550058? Wondering if this should be prioritized to support that work as we get closer to removing XBL consumers.

(In reply to Brian Grinstead [:bgrins] from comment #6)

(In reply to Brian Grinstead [:bgrins] from comment #5)

I pushed up a work in progress patch that uses a plain HTML Custom Element (<moz-key>) and it seems to work for most cases. Some notes:

• There are still some mochitest failures (https://treeherder.mozilla.org/#/jobs?repo=try&revision=c2e281037d1f3a60adf0d4e6d03d29f8ee9f6c06). I think some of those are related to [reserved] attribute which I don't fully understand yet.

Mossop, if we were able to get this over the finish line and fully drop the need for <xul:key> outside of XBL, would that change/simplify the approach you are thinking about in Bug 1550058? Wondering if this should be prioritized to support that work as we get closer to removing XBL consumers.

I think it would make a difference, not quite sure how much. Happily I have a bunch more time available this week and I was going to dig back into it and I can see how much it would help.

See part 2 of https://docs.google.com/document/d/1_Z2P5LeISwXt7JKuS9rKJkJg-ubWbMWUIMXPnbBrywg/

Sounds like we'll ultimately need new APIs exposed to JS to properly implement some of the attributes programmatically. As per the doc in Comment 8, this will depend on Bug 1550058.

What's the current status of this project? I was reminded of the annoyance that bug 371900 causes in https://phabricator.services.mozilla.com/D44474 and was wondering when we're switching to something nicer. :-)

(In reply to :Gijs (he/him) from comment #10)

What's the current status of this project? I was reminded of the annoyance that bug 371900 causes in https://phabricator.services.mozilla.com/D44474 and was wondering when we're switching to something nicer. :-)

See https://docs.google.com/document/d/1_Z2P5LeISwXt7JKuS9rKJkJg-ubWbMWUIMXPnbBrywg/edit

Part one landed in bug 1550058 and I am attempting to get round to part two which is effectively this bug, ... but if someone were eager to make this move faster I could help mentor them through this!

The following patch is waiting for review from an inactive reviewer:

:bgrins, could you please find another reviewer or abandon the patch if it is no longer relevant?

For more information, please visit auto_nag documentation.