Closed Bug 708936 Opened 10 years ago Closed 9 years ago

Remove hack for bug 262894 and bug 28852


(Core :: Widget: Win32, defect)

Windows 7
Not set





(Reporter: masayuki, Assigned: masayuki)



(Keywords: inputmethod)


(1 file)

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.
Comment on attachment 711749 [details] [diff] [review]

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)
Comment on attachment 711749 [details] [diff] [review]

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]

>+  // 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 bar isn't triggered.

Not sure I really like this patch, but don't have better ideas now.
Attachment #711749 - Flags: review?(bugs) → review+
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
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
You need to log in before you can comment on or make changes to this bug.