Skip to content

Commit

Permalink
Bug 1303273 part.3 Dispatch eKeyPress events without NativeKey::Handl…
Browse files Browse the repository at this point in the history 
…eCharMessage() when it handles WM_(SYS)KEYDOWN message and there are following WM_(SYS)CHAR messages which includes non-control character r=m_kato

This patch creates NativeKey::DispatchKeyPressEventsWithRetrievedCharMessages() for dispatching eKeyPress event with mCommittedCharsAndModifiers when it stores following printable WM_(SYS)CHAR messages.

Using loop for dispatching eKeyPress event for every WM_(SYS)CHAR message is wrong because WidgetKeyboardEvent::mKeyValue is initialized with mCommittedCharsAndModifiers and it causes TextEventDispatcher dispatching multiple eKeyPress events at every call of MaybeDispatchKeypressEvents().  Therefore, if mKeyValue is "^^", eKeyPress event is dispatched 4 times --for the first message, eKeyPress events are fired for each "^" and for the second message, eKeyPress events are fired again for each "^"--.  Therefore, when it handles WM_(SYS)KEYDOWN and it causes inputting one or more printable characters, it's the easiest way not to use HandleCharMessage().

The new method calls TextEventDispatcher::MaybeDispatchKeypressEvents() only once and it requests to call the callback method with new argument of MaybeDispatchKeypressEvents() when it needs to dispatch 2 or more eKeyPress events.  Then, NativeKey::WillDispatchKeyboardEvent() can set each eKeyPress event to raw information of the message and proper modifier state.

With this change, we can dispatch multiple eKeyPress events with retrieved WM_(SYS)CHAR message information rather than retrieved information from active keyboard layout.  Therefore, NeedsToHandleWithoutFollowingCharMessages() doesn't return true even when mCommittedCharsAndModifiers stores two or more characters.

FYI: there is a bug in test_keycodes.xul. That is, Alt+'A' of Greek keyboard layout should cause WM_SYSCHAR with a corresponding Greek character but ASCII characters are specified.  Therefore, this patch includes the fix of these bugs

MozReview-Commit-ID: JVm7ZJVug0O
  • Loading branch information
@masayuki-nakano
masayuki-nakano committed Oct 6, 2016
1 parent e7d1ae4 commit 48e9a49
Show file tree
Hide file tree
Showing 5 changed files with 153 additions and 134 deletions.
10 changes: 6 additions & 4 deletions widget/TextEventDispatcher.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,8 @@ TextEventDispatcher::DispatchKeyboardEventInternal(
const WidgetKeyboardEvent& aKeyboardEvent,
nsEventStatus& aStatus,
void* aData,
uint32_t aIndexOfKeypress)
uint32_t aIndexOfKeypress,
bool aNeedsCallback)
{
// Note that this method is also used for dispatching key events on a plugin
// because key events on a plugin should be dispatched same as normal key
Expand Down Expand Up @@ -501,7 +502,7 @@ TextEventDispatcher::DispatchKeyboardEventInternal(
keyEvent.mAlternativeCharCodes.Clear();
if ((WidgetKeyboardEvent::IsKeyDownOrKeyDownOnPlugin(aMessage) ||
aMessage == eKeyPress) &&
(keyEvent.IsControl() || keyEvent.IsAlt() ||
(aNeedsCallback || keyEvent.IsControl() || keyEvent.IsAlt() ||
keyEvent.IsMeta() || keyEvent.IsOS())) {
nsCOMPtr<TextEventDispatcherListener> listener =
do_QueryReferent(mListener);
Expand Down Expand Up @@ -538,7 +539,8 @@ bool
TextEventDispatcher::MaybeDispatchKeypressEvents(
const WidgetKeyboardEvent& aKeyboardEvent,
nsEventStatus& aStatus,
void* aData)
void* aData,
bool aNeedsCallback)
{
// If the key event was consumed, keypress event shouldn't be fired.
if (aStatus == nsEventStatus_eConsumeNoDefault) {
Expand All @@ -563,7 +565,7 @@ TextEventDispatcher::MaybeDispatchKeypressEvents(
for (size_t i = 0; i < keypressCount; i++) {
aStatus = nsEventStatus_eIgnore;
if (!DispatchKeyboardEventInternal(eKeyPress, aKeyboardEvent,
aStatus, aData, i)) {
aStatus, aData, i, aNeedsCallback)) {
// The widget must have been gone.
break;
}
Expand Down
12 changes: 10 additions & 2 deletions widget/TextEventDispatcher.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -284,12 +284,16 @@ class TextEventDispatcher final
* @param aData Calling this method may cause calling
* WillDispatchKeyboardEvent() of the listener.
* aData will be set to its argument.
* @param aNeedsCallback Set true when caller needs to initialize each
* eKeyPress event immediately before dispatch.
* Then, WillDispatchKeyboardEvent() is always called.
* @return true if one or more events are dispatched.
* Otherwise, false.
*/
bool MaybeDispatchKeypressEvents(const WidgetKeyboardEvent& aKeyboardEvent,
nsEventStatus& aStatus,
void* aData = nullptr);
void* aData = nullptr,
bool aNeedsCallback = false);

private:
// mWidget is owner of the instance. When this is created, this is set.
Expand Down Expand Up @@ -463,13 +467,17 @@ class TextEventDispatcher final
* text, this must be between 0 and
* mKeyValue.Length() - 1 since keypress events
* sending only one character per event.
* @param aNeedsCallback Set true when caller needs to initialize each
* eKeyPress event immediately before dispatch.
* Then, WillDispatchKeyboardEvent() is always called.
* @return true if an event is dispatched. Otherwise, false.
*/
bool DispatchKeyboardEventInternal(EventMessage aMessage,
const WidgetKeyboardEvent& aKeyboardEvent,
nsEventStatus& aStatus,
void* aData,
uint32_t aIndexOfKeypress = 0);
uint32_t aIndexOfKeypress = 0,
bool aNeedsCallback = false);
};

} // namespace widget
Expand Down
8 changes: 4 additions & 4 deletions widget/tests/test_keycodes.xul
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3760,17 +3760,17 @@ function* runAccessKeyTests()
// Greek layout can activate a Latin accesskey
yield testKey({layout:KEYBOARD_LAYOUT_GREEK, keyCode:WIN_VK_A,
modifiers:{shiftKey:1, altKey:1}, chars:"A"},
modifiers:{shiftKey:1, altKey:1}, chars:"\u0391"},
"a", true);
yield testKey({layout:KEYBOARD_LAYOUT_GREEK, keyCode:WIN_VK_A,
modifiers:{shiftKey:1, altKey:1}, chars:"A"},
modifiers:{shiftKey:1, altKey:1}, chars:"\u0391"},
"A", true);
// ... and a Greek accesskey!
yield testKey({layout:KEYBOARD_LAYOUT_GREEK, keyCode:WIN_VK_A,
modifiers:{shiftKey:1, altKey:1}, chars:"A"},
modifiers:{shiftKey:1, altKey:1}, chars:"\u0391"},
"\u03b1", true);
yield testKey({layout:KEYBOARD_LAYOUT_GREEK, keyCode:WIN_VK_A,
modifiers:{shiftKey:1, altKey:1}, chars:"A"},
modifiers:{shiftKey:1, altKey:1}, chars:"\u0391"},
"\u0391", true);
// bug 359638
Expand Down
237 changes: 120 additions & 117 deletions widget/windows/KeyboardLayout.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1519,7 +1519,7 @@ NativeKey::InitWithKeyChar()
// modifier state if this key event won't cause text input actually.
// They will be used for setting mAlternativeCharCodes in the callback
// method which will be called by TextEventDispatcher.
if (NeedsToHandleWithoutFollowingCharMessages()) {
if (!IsFollowedByPrintableCharMessage()) {
ComputeInputtingStringWithKeyboardLayout();
}
// Remove odd char messages if there are.
Expand Down Expand Up @@ -1978,13 +1978,8 @@ NativeKey::InitKeyEvent(WidgetKeyboardEvent& aKeyEvent,
aKeyEvent.mLocation = GetKeyLocation();
aModKeyState.InitInputEvent(aKeyEvent);

NPEvent pluginEvent;
if (aMsgSentToPlugin &&
mWidget->GetInputContext().mIMEState.mEnabled == IMEState::PLUGIN) {
pluginEvent.event = aMsgSentToPlugin->message;
pluginEvent.wParam = aMsgSentToPlugin->wParam;
pluginEvent.lParam = aMsgSentToPlugin->lParam;
aKeyEvent.mPluginEvent.Copy(pluginEvent);
if (aMsgSentToPlugin) {
MaybeInitPluginEventOfKeyEvent(aKeyEvent, *aMsgSentToPlugin);
}

KeyboardLayout::NotifyIdleServiceOfUserActivity();
Expand All @@ -2006,6 +2001,20 @@ NativeKey::InitKeyEvent(WidgetKeyboardEvent& aKeyEvent,
nsEventStatus_eIgnore;
}

void
NativeKey::MaybeInitPluginEventOfKeyEvent(WidgetKeyboardEvent& aKeyEvent,
const MSG& aMsgSentToPlugin) const
{
if (mWidget->GetInputContext().mIMEState.mEnabled != IMEState::PLUGIN) {
return;
}
NPEvent pluginEvent;
pluginEvent.event = aMsgSentToPlugin.message;
pluginEvent.wParam = aMsgSentToPlugin.wParam;
pluginEvent.lParam = aMsgSentToPlugin.lParam;
aKeyEvent.mPluginEvent.Copy(pluginEvent);
}

bool
NativeKey::DispatchCommandEvent(uint32_t aEventCommand) const
{
Expand Down Expand Up @@ -2444,6 +2453,15 @@ NativeKey::HandleKeyDownMessage(bool* aEventDispatched) const
return true;
}

// If mCommittedCharsAndModifiers was initialized with following char
// messages, we should dispatch keypress events with its information.
if (IsFollowedByPrintableCharOrSysCharMessage()) {
MOZ_LOG(sNativeKeyLogger, LogLevel::Info,
("%p NativeKey::HandleKeyDownMessage(), tries to be dispatching "
"keypress events with retrieved char messages...", this));
return DispatchKeyPressEventsWithRetrievedCharMessages();
}

// If we won't be getting a WM_CHAR, WM_SYSCHAR or WM_DEADCHAR, synthesize a
// keypress for almost all keys
if (NeedsToHandleWithoutFollowingCharMessages()) {
Expand All @@ -2454,34 +2472,6 @@ NativeKey::HandleKeyDownMessage(bool* aEventDispatched) const
DispatchKeyPressEventsWithoutCharMessage());
}

if (!mFollowingCharMsgs.IsEmpty()) {
bool consumed = false;
for (size_t i = 0; i < mFollowingCharMsgs.Length(); ++i) {
MOZ_LOG(sNativeKeyLogger, LogLevel::Info,
("%p NativeKey::HandleKeyDownMessage(), stopped dispatching "
"keypress events for remaining char messages, consumed=%s, "
"mFollowingCharMsgs[%u]=%s, mMsg=%s, "
"mFocusedWndBeforeDispatch=0x%p, ::GetFocus()=0x%p",
this, GetBoolName(consumed), i,
ToString(mFollowingCharMsgs[i]).get(),
ToString(mMsg).get(), mFocusedWndBeforeDispatch, ::GetFocus()));
consumed =
DispatchKeyPressEventForFollowingCharMessage(mFollowingCharMsgs[i]) ||
consumed;
if (mWidget->Destroyed() || IsFocusedWindowChanged()) {
MOZ_LOG(sNativeKeyLogger, LogLevel::Info,
("%p NativeKey::HandleKeyDownMessage(), %s event caused "
"destroying the widget", this));
return true;
}
}
MOZ_LOG(sNativeKeyLogger, LogLevel::Info,
("%p NativeKey::HandleKeyDownMessage(), handled all following char "
"messages, consumed=%s",
this, GetBoolName(consumed)));
return consumed;
}

// If WM_KEYDOWN of VK_PACKET isn't followed by WM_CHAR, we don't need to
// dispatch keypress events.
if (mVirtualKeyCode == VK_PACKET) {
Expand Down Expand Up @@ -2715,16 +2705,10 @@ NativeKey::NeedsToHandleWithoutFollowingCharMessages() const
return true;
}

// If inputting two or more characters, should be dispatched after removing
// whole following char messages.
if (mCommittedCharsAndModifiers.mLength > 1) {
return true;
}

// If keydown message is followed by WM_CHAR whose wParam isn't a control
// character, we should dispatch keypress event with the char message
// even with any modifier state.
if (IsFollowedByPrintableCharMessage()) {
// If keydown message is followed by WM_CHAR or WM_SYSCHAR whose wParam isn't
// a control character, we should dispatch keypress event with the char
// message even with any modifier state.
if (IsFollowedByPrintableCharOrSysCharMessage()) {
return false;
}

Expand Down Expand Up @@ -3203,6 +3187,56 @@ NativeKey::ComputeInputtingStringWithKeyboardLayout()
}
}

bool
NativeKey::DispatchKeyPressEventsWithRetrievedCharMessages() const
{
MOZ_ASSERT(IsKeyDownMessage());
MOZ_ASSERT(IsFollowedByPrintableCharOrSysCharMessage());

nsresult rv = mDispatcher->BeginNativeInputTransaction();
if (NS_WARN_IF(NS_FAILED(rv))) {
MOZ_LOG(sNativeKeyLogger, LogLevel::Error,
("%p NativeKey::DispatchKeyPressEventsWithRetrievedCharMessages(), "
"FAILED due to BeginNativeInputTransaction() failure", this));
return true;
}
WidgetKeyboardEvent keypressEvent(true, eKeyPress, mWidget);
MOZ_LOG(sNativeKeyLogger, LogLevel::Debug,
("%p NativeKey::DispatchKeyPressEventsWithRetrievedCharMessages(), "
"initializing keypress event...", this));
ModifierKeyState modKeyState(mModKeyState);
if (IsFollowedByPrintableCharMessage()) {
// If eKeyPress event should cause inputting text in focused editor,
// we need to remove Alt and Ctrl state.
modKeyState.Unset(MODIFIER_ALT | MODIFIER_CONTROL);
}
// We don't need to send char message here if there are two or more retrieved
// messages because we need to set each message to each eKeyPress event.
bool needsCallback = mFollowingCharMsgs.Length() > 1;
nsEventStatus status =
InitKeyEvent(keypressEvent, modKeyState,
!needsCallback ? &mFollowingCharMsgs[0] : nullptr);
MOZ_LOG(sNativeKeyLogger, LogLevel::Info,
("%p NativeKey::DispatchKeyPressEventsWithRetrievedCharMessages(), "
"dispatching keypress event(s)...", this));
bool dispatched =
mDispatcher->MaybeDispatchKeypressEvents(keypressEvent, status,
const_cast<NativeKey*>(this),
needsCallback);
if (mWidget->Destroyed()) {
MOZ_LOG(sNativeKeyLogger, LogLevel::Info,
("%p NativeKey::DispatchKeyPressEventsWithRetrievedCharMessages(), "
"keypress event(s) caused destroying the widget", this));
return true;
}
bool consumed = status == nsEventStatus_eConsumeNoDefault;
MOZ_LOG(sNativeKeyLogger, LogLevel::Info,
("%p NativeKey::DispatchKeyPressEventsWithRetrievedCharMessages(), "
"dispatched keypress event(s), dispatched=%s, consumed=%s",
this, GetBoolName(dispatched), GetBoolName(consumed)));
return consumed;
}

bool
NativeKey::DispatchKeyPressEventsWithoutCharMessage() const
{
Expand Down Expand Up @@ -3250,6 +3284,46 @@ void
NativeKey::WillDispatchKeyboardEvent(WidgetKeyboardEvent& aKeyboardEvent,
uint32_t aIndex)
{
// If it's an eKeyPress event and it's generated from retrieved char message,
// we need to set raw message information for plugins.
if (aKeyboardEvent.mMessage == eKeyPress &&
IsFollowedByPrintableCharOrSysCharMessage()) {
MOZ_RELEASE_ASSERT(aIndex < mCommittedCharsAndModifiers.mLength);
uint32_t foundPrintableCharMessages = 0;
for (size_t i = 0; i < mFollowingCharMsgs.Length(); ++i) {
if (!IsPrintableCharOrSysCharMessage(mFollowingCharMsgs[i])) {
// XXX Should we dispatch a plugin event for WM_*DEADCHAR messages and
// WM_CHAR with a control character here? But we're not sure
// how can we create such message queue (i.e., WM_CHAR or
// WM_SYSCHAR with a printable character and such message are
// generated by a keydown). So, let's ignore such case until
// we'd get some bug reports.
MOZ_LOG(sNativeKeyLogger, LogLevel::Warning,
("%p NativeKey::WillDispatchKeyboardEvent(), WARNING, "
"ignoring %uth message due to non-printable char message, %s",
this, i + 1, ToString(mFollowingCharMsgs[i]).get()));
continue;
}
if (foundPrintableCharMessages++ == aIndex) {
// Found message which caused the eKeyPress event. Let's set the
// message for plugin if it's necessary.
MaybeInitPluginEventOfKeyEvent(aKeyboardEvent, mFollowingCharMsgs[i]);
break;
}
}
// Set modifier state from mCommittedCharsAndModifiers because some of them
// might be different. For example, Shift key was pressed at inputting
// dead char but Shift key was released before inputting next character.
ModifierKeyState modKeyState(mModKeyState);
modKeyState.Unset(MODIFIER_SHIFT | MODIFIER_CONTROL | MODIFIER_ALT |
MODIFIER_ALTGRAPH | MODIFIER_CAPSLOCK);
modKeyState.Set(mCommittedCharsAndModifiers.mModifiers[aIndex]);
modKeyState.InitInputEvent(aKeyboardEvent);
MOZ_LOG(sNativeKeyLogger, LogLevel::Info,
("%p NativeKey::WillDispatchKeyboardEvent(), "
"setting %uth modifier state to %s",
this, aIndex + 1, ToString(modKeyState).get()));
}
uint32_t longestLength =
std::max(mInputtingStringAndModifiers.mLength,
std::max(mShiftedString.mLength, mUnshiftedString.mLength));
Expand Down Expand Up @@ -3384,77 +3458,6 @@ NativeKey::WillDispatchKeyboardEvent(WidgetKeyboardEvent& aKeyboardEvent,
}
}

bool
NativeKey::DispatchKeyPressEventForFollowingCharMessage(
const MSG& aCharMsg) const
{
MOZ_ASSERT(IsKeyDownMessage());

if (mFakeCharMsgs) {
if (IsDeadCharMessage(aCharMsg)) {
return false;
}
#ifdef DEBUG
if (mIsPrintableKey) {
nsPrintfCString log(
"mOriginalVirtualKeyCode=0x%02X, mCommittedCharsAndModifiers={ "
"mChars=[ 0x%04X, 0x%04X, 0x%04X, 0x%04X, 0x%04X ], mLength=%d }, "
"wParam=0x%04X",
mOriginalVirtualKeyCode, mCommittedCharsAndModifiers.mChars[0],
mCommittedCharsAndModifiers.mChars[1],
mCommittedCharsAndModifiers.mChars[2],
mCommittedCharsAndModifiers.mChars[3],
mCommittedCharsAndModifiers.mChars[4],
mCommittedCharsAndModifiers.mLength, aCharMsg.wParam);
if (mCommittedCharsAndModifiers.IsEmpty()) {
log.Insert("length is zero: ", 0);
NS_ERROR(log.get());
NS_ABORT();
} else if (mCommittedCharsAndModifiers.mChars[0] != aCharMsg.wParam) {
log.Insert("character mismatch: ", 0);
NS_ERROR(log.get());
NS_ABORT();
}
}
#endif // #ifdef DEBUG
return HandleCharMessage(aCharMsg);
}

if (IsDeadCharMessage(aCharMsg)) {
if (!mWidget->PluginHasFocus()) {
MOZ_LOG(sNativeKeyLogger, LogLevel::Info,
("%p NativeKey::DispatchKeyPressEventForFollowingCharMessage(), "
"plugin doesn't have focus", this));
return false;
}
MOZ_LOG(sNativeKeyLogger, LogLevel::Info,
("%p NativeKey::DispatchKeyPressEventForFollowingCharMessage(), "
"dispatching plugin event...", this));
bool ok = mWidget->DispatchPluginEvent(aCharMsg) || mWidget->Destroyed();
MOZ_LOG(sNativeKeyLogger, LogLevel::Info,
("%p NativeKey::DispatchKeyPressEventForFollowingCharMessage(), "
"dispatched plugin event, result=%s, mWidget->Destroyed()=%s",
this, GetBoolName(ok), GetBoolName(mWidget->Destroyed())));
}

bool defaultPrevented = HandleCharMessage(aCharMsg);
// If a syschar keypress wasn't processed, Windows may want to
// handle it to activate a native menu.
if (!defaultPrevented && IsSysCharMessage(aCharMsg)) {
MOZ_LOG(sNativeKeyLogger, LogLevel::Info,
("%p NativeKey::DispatchKeyPressEventForFollowingCharMessage(), "
"calling DefWindowProcW(aCharMsg=%s)...",
this, ToString(aCharMsg).get()));
::DefWindowProcW(aCharMsg.hwnd, aCharMsg.message,
aCharMsg.wParam, aCharMsg.lParam);
MOZ_LOG(sNativeKeyLogger, LogLevel::Info,
("%p NativeKey::DispatchKeyPressEventForFollowingCharMessage(), "
"called DefWindowProcW(aCharMsg=%s)",
this, ToString(aCharMsg).get()));
}
return defaultPrevented;
}

/*****************************************************************************
* mozilla::widget::KeyboardLayout
*****************************************************************************/
Expand Down
Loading

0 comments on commit 48e9a49

Please sign in to comment.