Closed Bug 887695 Opened 11 years ago Closed 5 years ago

WM_KEYDOWN and WM_SYSKEYDOWN should be handled before IME handles them

Categories

(Core :: Widget: Win32, defect, P2)

x86_64
Windows 8
defect

Tracking

()

RESOLVED WONTFIX

People

(Reporter: masayuki, Assigned: masayuki)

References

(Depends on 1 open bug)

Details

(Keywords: inputmethod, Whiteboard: tpi:+)

Attachments

(5 files)

Currently, both WM_KEYDOWN and WM_SYSKEYDOWN are passed to IME before we handle them. Therefore, if IME consumes the key messages, we don't dispatch NS_KEY_DOWN event. This is different behavior from Gecko for Mac. And also, we need to fix this for fixing bug 354358. I think that there are two approach for this bug. 1. Don't call TranslateMessage() before a call of DispatchMessage(). Then, the keydown message handler in nsWindow calls TranslateMessage() after dispatching NS_KEY_DOWN. 2. Before a call of TranslateMessage() in the message loop, call the message handler of the window directly. Then, ignore the messages via DispatchMessage(). I feel #1 is better. However, I don't know if it's a right way because I've never see such message handling code. Do you guys have any suggestions?
Flags: needinfo?(m_kato)
Flags: needinfo?(jmathies)
Flags: needinfo?(VYV03354)
I think it's better to handle the message entirely before calling TranslateMessage(). MFC has PreTranslateMessage() method for this very purpose. http://msdn.microsoft.com/ja-jp/library/cw1sb70c%28v=vs.80%29.aspx IsDialogMessage() and TranslateAccelerator() will also be called before TranslateMessage(). And if the functions return nonzero (which will correspond to preventDefault() call), TranslateMessage() and DispatchMessage() will not be called.
Flags: needinfo?(VYV03354)
Thank you, Kimura-san! I'll try to do similar approach.
Flags: needinfo?(m_kato)
Flags: needinfo?(jmathies)
No longer depends on: 888204
It seems that CoreDispatcher::ProcessEvents() calls TranslateMessage() and passes key messages to TSF automatically. This blocks to fix this bug on Metrofox. Jimm, I'd like to know your idea for this issue. If we don't use CoreDispatcher for key messages (WM_KEYFIRST - WM_KEYLAST), we can handle them in lower level. However, it might cause another bug since CoreDispatcher::ProcessEvents() might do something we don't know. Do you think that not using CoreDispatcher for key messages is too risky? If so, we'd need to mark this bug as WONTFIX on Metrofox. Although, we shouldn't decide so as far as possible for consistent behavior between platforms.
Flags: needinfo?(jmathies)
FYI: Currently, Chrome in MetroApp mode has same issue. I.e., web developer cannot prevent composition by preventing default of keydown event since keydown event isn't fired when TSF consumes native key message.
Hmm, I forgot the part.1. It seems that we need to call at least HandleKeyDownMessageBeforeIME() should be called before CoreDispatcher::ProcessEvents()...
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #3) > It seems that CoreDispatcher::ProcessEvents() calls TranslateMessage() and > passes key messages to TSF automatically. This blocks to fix this bug on > Metrofox. > > Jimm, I'd like to know your idea for this issue. If we don't use > CoreDispatcher for key messages (WM_KEYFIRST - WM_KEYLAST), we can handle > them in lower level. However, it might cause another bug since > CoreDispatcher::ProcessEvents() might do something we don't know. > > Do you think that not using CoreDispatcher for key messages is too risky? If > so, we'd need to mark this bug as WONTFIX on Metrofox. > > Although, we shouldn't decide so as far as possible for consistent behavior > between platforms. We have no idea what is inside CoreDispatcher::ProcessEvents(). You can put together a patch and we can land it and look for side effects. My concern would be that metro specific keyboard events might not trigger appropriate ui reactions. I guess it depends on where they pick those up. Generally more control over event processing in metro is probably a good thing assuming there aren't any serious regressions.
Flags: needinfo?(jmathies)
Thanks. I'll try to find the lowest risky way for Metrofox.
(In reply to Jim Mathies [:jimm] from comment #13) > Looks like we are already doing this? > > http://mxr.mozilla.org/mozilla-central/source/widget/windows/winrt/ > MetroWidget.cpp#591 Meaning, we already intercept these events before they get to the def wnd proc. So I'd say go ahead and give it a try.
No. At that time, TSF may already consume the message or ::TranslateMessage() is already called. So, perhaps, we need to handle WM_KEYDOWN at MetroAppShell.
Is this still a valid bug?
Flags: needinfo?(masayuki)
(In reply to Jim Mathies [:jimm] from comment #16) > Is this still a valid bug? Yes, it is. At least in TSF mode, we need to do this because TSF consumes WM_KEYDOWN messages. Therefore, we cannot dispatch keydown events during composition and keydown event of a key starting a composition.
Flags: needinfo?(masayuki)
Priority: -- → P2
Whiteboard: tpi:+

Current our behavior matches with Chrome so that changing our behavior (for consistency between platforms) may cause new web-compat issue. So, we should not do this.

Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: