See https://bugzilla.mozilla.org/show_bug.cgi?id=1203059#c52 TL;DR we're reserving certain keybindings for the browser only and need some Firefox code changed to fully support this.

<key id="key_quitApplication" key="&quitApplicationCmdUnix.key;" modifiers="accel"/> has no command attribute, so even if adding reserved="true", it doesn't check this attribute.

(In reply to Makoto Kato [:m_kato] from comment #1) > <key id="key_quitApplication" key="&quitApplicationCmdUnix.key;" > modifiers="accel"/> has no command attribute, so even if adding > reserved="true", it doesn't check this attribute. Is that really not checking? nsXBLWindowKeyHandler checks the reserved event here: https://dxr.mozilla.org/mozilla-central/rev/fc15477ce628599519cb0055f52cc195d640dc94/dom/xbl/nsXBLWindowKeyHandler.cpp#604,607-608,613,618-620 But GetElementForHandler() looks like returning key element if there is no proper command element: https://dxr.mozilla.org/mozilla-central/source/dom/xbl/nsXBLWindowKeyHandler.cpp#717

Review commit: https://reviewboard.mozilla.org/r/51053/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/51053/

(In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) (offline: 4/29-5/8) from comment #2) > (In reply to Makoto Kato [:m_kato] from comment #1) > > <key id="key_quitApplication" key="&quitApplicationCmdUnix.key;" > > modifiers="accel"/> has no command attribute, so even if adding > > reserved="true", it doesn't check this attribute. > > Is that really not checking? > > nsXBLWindowKeyHandler checks the reserved event here: > https://dxr.mozilla.org/mozilla-central/rev/ > fc15477ce628599519cb0055f52cc195d640dc94/dom/xbl/nsXBLWindowKeyHandler. > cpp#604,607-608,613,618-620 > > But GetElementForHandler() looks like returning key element if there is no > proper command element: > https://dxr.mozilla.org/mozilla-central/source/dom/xbl/nsXBLWindowKeyHandler. > cpp#717 In WalkHandlersAndExecute(), IsExecutableElement() returns false due to no command attribute. So HasHandlerForEvent returns false. Then mIsReserved isn't set to true.

(In reply to Makoto Kato [:m_kato] from comment #3) > Created attachment 8749587 [details] > MozReview Request: Bug 1253284 - Allow reserved attribute without command > attribute > > Review commit: https://reviewboard.mozilla.org/r/51053/diff/#index_header > See other reviews: https://reviewboard.mozilla.org/r/51053/ Looks good, should I review this?

(In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) (offline: 4/29-5/8) from comment #5) > (In reply to Makoto Kato [:m_kato] from comment #3) > > Created attachment 8749587 [details] > > MozReview Request: Bug 1253284 - Allow reserved attribute without command > > attribute > > > > Review commit: https://reviewboard.mozilla.org/r/51053/diff/#index_header > > See other reviews: https://reviewboard.mozilla.org/r/51053/ > > Looks good, should I review this? I am investigating whether there is another way...

Any update on this? Should we proceed with the current patch?

humm, although I investigated another way like reading attribute or hooking on global menu bar, but no way to hook it.... So we should handle it on nsXBLWindowKeyHandler like my first idea.

Comment on attachment 8749587 [details] MozReview Request: Bug 1253284 - Allow reserved attribute without command attribute https://reviewboard.mozilla.org/r/51053/#review50224