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

RESOLVED FIXED in mozilla29

Status

()

Core
Widget: Android
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: masayuki, Assigned: masayuki)

Tracking

Trunk
mozilla29
All
Android
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

nsWindow of Android doesn't set modifiers correctly. Let's set them as far as possible.
Created attachment 8348505 [details] [diff] [review]
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
Attachment #8348505 - Flags: review?(nchen)
test pages:
https://dvcs.w3.org/hg/d4e/raw-file/tip/key-event-test.html
https://bugzilla.mozilla.org/attachment.cgi?id=640488
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"?
Attachment #8348505 - Flags: review?(nchen) → review+
Thank you for the information and your review!
https://hg.mozilla.org/integration/mozilla-inbound/rev/40e241eed25e
https://hg.mozilla.org/mozilla-central/rev/40e241eed25e
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
You need to log in before you can comment on or make changes to this bug.