1749667 - Keyboard commands broken by bug 371900

Status: RESOLVED FIXED

(Thunderbird :: Upstream Synchronization, defect)

Description

[Geoff Lankow (:darktrojan)]

We've lost a bunch of functionality. The following tests are failing:

• comm/mail/components/addrbook/test/browser/new/browser_edit_photo.js which involves pasting on an element that has focus but isn't an input element (I'm not entirely sure if this is supposed to work, but it used to)
• comm/mail/components/extensions/test/browser/browser_ext_commands_onCommand.js which assigns Ctrl+Alt+V to an extension
• comm/mail/test/browser/attachment/browser_attachment.js which uses Ctrl+A
• comm/mail/test/browser/composition/browser_contactsSidebar.js which presses Delete on a XUL tree
• possibly some of the other tests in the same folder as the previous one

I've done a local backout of bug 371900 and the tests pass again. I'm not sure where to go from here.

Comment 1

[Geoff Lankow (:darktrojan)]

I've managed to trick some of the things into working again by removing XUL keys such as <key id="key_delete" keycode="VK_DELETE"/> in the contacts sidebar. This seems less than ideal.

Comment 2

[Neil Deakin]

Some commands -- the ones in editMenuOverlay.js that were changed by bug 371900 -- are special in that they have a key/menu defined in the <keyset> or menu, but the actual key handling behaviour is done via the ones in ShortcutKeyDefinitions.cpp. To accommodate this before patch 371900 there was a special-case that would ignore keys that didn't have a command or oncommand associated with it, however there are cases where you want to use an event listener defined separately. So, now, post-371900, those key elements are not ignored so nothing will happen.

Instead, you need to add internal="true" to those keys. For example, I tried adding internal="true" to the key "key_selectAll" that is defined in mainKeySet.inc.xhtml and this allowed the test browser_attachment.js to now pass.

There seems to be quite a few different files where this key and others are defined with no associated command handler, so I assume fixing all those up would fix these tests.

Comment 3

[Alessandro Castellani [:aleca]]

Working on a patch for this.

Comment 4

[Alessandro Castellani [:aleca]]

It seems the Geoff beat me to it.

Comment 5

[Geoff Lankow (:darktrojan)]

Attachment: Bug 1749667 - Add internal="true" to XUL keys without command or oncommand attributes. r=aleca

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/6417eea12df0
Add internal="true" to XUL keys without command or oncommand attributes. r=aleca

Comment 7

[Geoff Lankow (:darktrojan)]

I hope that's the end of it. It fixed the broken tests locally. Has it broken any others? Only time will tell!