In bug 1454323 we hit a scenario when due to l10n fallback XUL registered the <key> element before its localization was populated in DOM. That results in the registration of <key> which seems to capture all command keys taking over cmd+t, cmd+q etc. But when we set it to `<key key="" data-l10n-id="...">` then the key doesn't takeover all the other command keys. I'd like to switch the default behavior to the same as for `key=""` to avoid having to populate empty `key=""` in each <key> controlled by Fluent. Now, here come famous last words: I hope there's no way that any code depends on that behavior, right?

Funny thing, this code seems to actually live in XBL. :-) I think I have a patch, just doing a sanity check...

I tested the patch by removing the data-l10n-id attribute from the key in preferences.xul. Pre-patch, this broke using ctrl-t to open a tab on my windows machine when the prefs were open. Post-patch, this worked correctly. Note that some shortcuts did continue working even when the prefs were open. Not really clear on why that would be. On a more technical note in the patch, I wonder if I need to initialize the nsAutoString before using? The attached patch compiles and works fine for me on Windows, but I'm paranoid and got lost in string headers trying to find a definition to resolve the question...

Comment on attachment 8969627 [details] Bug 1455379 - ignore key elements without a key attribute, https://reviewboard.mozilla.org/r/238420/#review244248 that looks great! Thank you Gijs!

Curious - why is this implemented in xbl/ instead of xul/? And would removing platformHTMLBindings.xml (as in Bug 1419091) make it easier to decouple this from XBL?

(In reply to Brian Grinstead [:bgrins] from comment #5) > Curious - why is this implemented in xbl/ instead of xul/? I don't know, I just noticed it was... > And would > removing platformHTMLBindings.xml (as in Bug 1419091) make it easier to > decouple this from XBL? I don't know that, either. Sorry!

Comment on attachment 8969627 [details] Bug 1455379 - ignore key elements without a key attribute, https://reviewboard.mozilla.org/r/238420/#review244274 ::: dom/xbl/nsXBLWindowKeyHandler.cpp:214 (Diff revision 1) > - bool attrExists = > + // Hopefully at least one of the attributes is set: > - keyElement->GetAttr(kNameSpaceID_None, nsGkAtoms::key, valKey) || > + keyElement->GetAttr(kNameSpaceID_None, nsGkAtoms::key, valKey) || > keyElement->GetAttr(kNameSpaceID_None, nsGkAtoms::charcode, valCharCode) || > keyElement->GetAttr(kNameSpaceID_None, nsGkAtoms::keycode, valKeyCode); > - if (attrExists && > - valKey.IsEmpty() && valCharCode.IsEmpty() && valKeyCode.IsEmpty()) > + // If not, ignore this key element. > + if (valKey.IsEmpty() && valCharCode.IsEmpty() && valKeyCode.IsEmpty()) Please brace the body (I know this was preexisting).

Pushed by gijskruitbosch@gmail.com: https://hg.mozilla.org/integration/autoland/rev/edde2964d8bc ignore key elements without a key attribute, r=bz,gandalf