WM_APPCOMMAND may be caused by key press, then, we should dispatch key event with D3E KeyboardEvent.key (only when the value isn't "Unidentified").

As I commented in NativeKey::HandleAppCommandMessage(), Windows sends WM_APPCOMMAND to the foreground window first. Then, if it is caused by a keypress and the key can be mapped to a virtual keycode, the DefaultWndProc posts WM_KEYDOWN and WM_KEYUP. I believe that WM_APPCOMMAND's action should be a default action of its keydown event. Therefore, this patch dispatches a keydown event before handling the command. Then, if the keydown event is not consumed, the command is performed by our chrome. Then, if WM_KEYUP won't come, the method dispatches a keyup event. Otherwise, we should wait following WM_KEYUP event. This patch makes the WM_APPCOMMAND handler completely move to NativeKey class. This may be odd if WM_APPCOMMAND isn't caused by a key press (e.g., mouse button press). However, calling a part of handler from NativeKey is complicated. And also I don't want to duplicate the code. Therefore, I take this approach.

Comment on attachment 8446451 [details] [diff] [review] Dispatch key events when WM_APPCOMMAND is fired for a keypress Review of attachment 8446451 [details] [diff] [review]: ----------------------------------------------------------------- looks flawless to me, nice code cleanup too.

Comment on attachment 8446451 [details] [diff] [review] Dispatch key events when WM_APPCOMMAND is fired for a keypress So I assume it isn't possible to get first appcommand for X, then some random key events, and then some key events which have the same virtualkeycode as what X had? Also, what happens if one presses for example browser_forward button for long time? Do we get multiple command events or multiple key events or both or just one commant + keydown/up ? But guess this is ok, though it would be perhaps nice to make FF to use those key events, and not use Gecko-only AppCommand DOM event.

(In reply to Olli Pettay [:smaug] from comment #3) > Comment on attachment 8446451 [details] [diff] [review] > Dispatch key events when WM_APPCOMMAND is fired for a keypress > > So I assume it isn't possible to get first appcommand for X, then > some random key events, and then some key events which have the > same virtualkeycode as what X had? First, Windows (OS) handles raw keyboard event, and then, sends WM_APPCOMMAND message to the focused window. This is the first time when we can know a multimedia key is pressed. Then, if we don't consume the message, the DefaultWndProc will post keydown message and keyup message (only when the key is released). Therefore, other messages might be inserted between WM_APPCOMMAND and WM_KEYDOWN and WM_KEYUP. > Also, what happens if one presses for example browser_forward button for > long time? Do we get multiple command events or multiple key events or both > or just one commant + keydown/up ? The former like this: > <00001> 00081670 S WM_APPCOMMAND > <00002> 00081670 R WM_APPCOMMAND > <00003> 00081670 P WM_KEYDOWN nVirtKey:VK_BROWSER_BACK cRepeat:1 ScanCode:00 fExtended:1 fAltDown:0 fRepeat:0 fUp:0 > <00004> 00081670 S WM_APPCOMMAND > <00005> 00081670 R WM_APPCOMMAND > <00006> 00081670 P WM_KEYDOWN nVirtKey:VK_BROWSER_BACK cRepeat:1 ScanCode:00 fExtended:1 fAltDown:0 fRepeat:1 fUp:0 > <00007> 00081670 S WM_APPCOMMAND > <00008> 00081670 R WM_APPCOMMAND > <00009> 00081670 P WM_KEYDOWN nVirtKey:VK_BROWSER_BACK cRepeat:1 ScanCode:00 fExtended:1 fAltDown:0 fRepeat:1 fUp:0 > <00010> 00081670 S WM_APPCOMMAND > <00011> 00081670 R WM_APPCOMMAND > <00012> 00081670 P WM_KEYDOWN nVirtKey:VK_BROWSER_BACK cRepeat:1 ScanCode:00 fExtended:1 fAltDown:0 fRepeat:1 fUp:0 > <00013> 00081670 S WM_APPCOMMAND > <00014> 00081670 R WM_APPCOMMAND > <00015> 00081670 P WM_KEYDOWN nVirtKey:VK_BROWSER_BACK cRepeat:1 ScanCode:00 fExtended:1 fAltDown:0 fRepeat:1 fUp:0 > <00016> 00081670 P WM_KEYUP nVirtKey:VK_BROWSER_BACK cRepeat:1 ScanCode:00 fExtended:1 fAltDown:0 fRepeat:1 fUp:1

FYI: I merged a fix of bug 1091025 before landing.

Hello, you have a non-unified bustage from this patch. Can you please take a look?

(In reply to nigelbabu@gmail.com [:nigelb] from comment #7) > Hello, you have a non-unified bustage from this patch. Can you please take a > look? Wow, sorry for the bustate. I start to check it... Wait a moment.

https://hg.mozilla.org/mozilla-central/rev/facf85b61c37 https://hg.mozilla.org/mozilla-central/rev/2f51262d2cab