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

RESOLVED FIXED in mozilla29

Status

()

defect
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: masayuki, Assigned: masayuki)

Tracking

Trunk
mozilla29
All
Android
Points:
---
Dependency tree / graph

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.
Posted patch PatchSplinter Review
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)
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+
https://hg.mozilla.org/mozilla-central/rev/40e241eed25e
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
You need to log in before you can comment on or make changes to this bug.