Remove hack for bug 262894 and bug 28852

RESOLVED FIXED in mozilla21

Status

()

Core
Widget: Win32
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: masayuki, Assigned: masayuki)

Tracking

({inputmethod})

Trunk
mozilla21
x86_64
Windows 7
inputmethod
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

I want to remove hack for bug 28852.

I think that we should dispatch events when keyboard layout is changed or IME input mode is changed. At that time, our chrome shouldn't handle the following keyup events because if we handle them, the key input causes multiple actions.

Another possible idea is, we will add NS_EVENT_FLAG_PREVENT_MULTIPLE_ACTIONS for the keyup event at dispatching from widget. However, I think that this needs more complicated implementation because we need to manage the physical key state for all keys (I don't want to think about a system with two or more keyboards). Therefore, I think the first idea is better.
Blocks: 347010
Created attachment 711749 [details] [diff] [review]
Patch

https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=ac1eecf3709f
Comment on attachment 711749 [details] [diff] [review]
Patch

If Alt keyup event comes with WM_KEYDOWN rather than WM_SYSKEYUP, that means one or more keys are pressed during the Alt key pressed. For example, Alt+` (Alt+半角/全角 in JIS keyboard layout, legacy IME open state switching shortcut key), Alt+F or left-Alt+left-Shift (switching keyboard layout).

At that time, if we set mDefaultPrevented true, XUL menubar ignores the Alt key up event. See bug 749563. So, we should set so since OS already consumed the Alt key operation. This makes a difference with other browsers. On other browsers, the defaultPrevented attribute is false. But if web applications want to handle the keyup event, they can ignore the value (and probably, most application don't check the attribute value before handling any events). Therefore, I think that this difference doesn't matter.
Attachment #711749 - Flags: review?(jmathies)
Attachment #711749 - Flags: review?(bugs)
Depends on: 749563

Comment 3

5 years ago
Comment on attachment 711749 [details] [diff] [review]
Patch

This looks ok to me, although I can't test it directly.
Attachment #711749 - Flags: review?(jmathies) → review+
Comment on attachment 711749 [details] [diff] [review]
Patch

>+  // Set defaultPrevented of the key eventif the VK_MENU is not a system key
>+  // release, so that the menu bar does not trigger.

event if
...menu bar isn't triggered.


Not sure I really like this patch, but don't have better ideas now.
Attachment #711749 - Flags: review?(bugs) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/66b99572ab26
Blocks: 840409

Comment 6

5 years ago
This also removes the hack for bug 262894.
Blocks: 262894
Absolutely, thanks.
Summary: Remove hack for bug 28852 → Remove hack for bug 262894 and bug 28852

Comment 8

5 years ago
https://hg.mozilla.org/mozilla-central/rev/66b99572ab26
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
You need to log in before you can comment on or make changes to this bug.