951021 - nsWindow of Android should set modifier state at dispatching events derived from WidgetInputEvent

Status: RESOLVED FIXED

(Core Graveyard :: Widget: Android, defect)

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away 4/27 - 5/6)]

nsWindow of Android doesn't set modifiers correctly. Let's set them as far as possible.

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away 4/27 - 5/6)]

Attachment: Patch

This only fixes the events dispatched from nsWindow. Looks like there are other paths, though. Chrome JS generates some keyboard/mouse events? Then, we need more work for them, but it should be out of scope of this bug.

On Android, I'm not sure how to know if AltGr key is pressed. I tested on Android 4.4, Alt modifier isn't set at native key press event which causes text input. So, for such key events, we cannot know if right Alt key is pressed. Additionally, we cannot set AltGr flag even if right Alt key is pressed at other key events because right Alt key isn't AltGr on a lot of keyboard layouts.

tryserver build:
https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=abc25dea3d3d

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away 4/27 - 5/6)]

test pages:
https://dvcs.w3.org/hg/d4e/raw-file/tip/key-event-test.html
https://bugzilla.mozilla.org/attachment.cgi?id=640488

Comment 3

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8348505 [details] [diff] [review]
Patch

Review of attachment 8348505 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM

::: widget/android/nsWindow.cpp
@@ +1617,5 @@
> +    // don't specify extra modifiers. If UnicodeChar() != BaseUnicodeChar()
> +    // it means UnicodeChar() already has modifiers applied.
> +    // XXX On Android 4.4, Alt modifier isn't set when the key input causes
> +    //     text input even while right Alt key is pressed.  Do we really need
> +    //     this code?

This code is for Android 2.3 (Gingerbread) compatibility. Can you change the comment to say "for Android 2.3 compatibility"?

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away 4/27 - 5/6)]

Thank you for the information and your review!
https://hg.mozilla.org/integration/mozilla-inbound/rev/40e241eed25e

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/40e241eed25e