Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2024-11-27 23:02:20 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 1441821 - NativeKey shouldn't mark eKeyDown and eKeyPress as "skippable in remote process" if message is not caused by physical key press r=m_kato,smaug

Browse Source 
Currently, TabChild discards eKeyDown and eKeyPress events which are marked as
"repeated" and were dispatched after the latest eKeyDown event comes into the
process.  However, keyboard layout utils may generate native key events
as "repeated" even if each native key is important to input proper text.

So, TabChild shouldn't decide if coming keyboard event is skippable only with
mIsRepeat.  For solving this issue, this patch adds
mMaybeSkippableInRemoteProcess to WidgetKeyboardEvent and makes
TabChild::SkipRepeatedKeyEvent() check
WidgetKeyboardEvent::CanSkipInRemoteProcess() instead.

On Windows, there are two ways to generate keyboard input messages.  One is
using SendMessage() or PostMessage().  The other is SendInput() API.  In both
ways, utils can make their input as repeated key messages.

The former case must be safe for this issue since such utils need to set 31st
bit of lParam to 1 explicitly.

On the other hand, in the latter case, the utils probably need to append
KEYEVENTF_KEYUP into KEYBDINPUT::dwFlags.  Otherwise, only first call is
treated as non-repeated event.

So, when given message does not came from physical key operation, NativeKey
should set WidgetKeyboardEvent::mMaybeSkippableInRemoteProcess to false
even if WidgetKeyboardEvent::mIsRepeat is true.

MozReview-Commit-ID: 3rinrOjx8Tf

--HG--
extra : rebase_source : 26b6d869260176fc7ef535323b83001bb4b725c2
This commit is contained in:
Masayuki Nakano 2018-06-06 23:35:16 +09:00
parent 944f20e73f
commit a20855c9f5
6 changed files with 182 additions and 57 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
dom/ipc/TabChild.cpp
View File

@ -2041,7 +2041,7 @@ bool
TabChild::SkipRepeatedKeyEvent(const WidgetKeyboardEvent& aEvent)
{
if (mRepeatedKeyEventTime.IsNull() ||
!aEvent.mIsRepeat ||
!aEvent.CanSkipInRemoteProcess() ||
(aEvent.mMessage != eKeyDown && aEvent.mMessage != eKeyPress)) {
mRepeatedKeyEventTime = TimeStamp();
mSkipKeyPress = false;

22
widget/TextEvents.h
View File

@ -164,6 +164,7 @@ protected:
, mIsRepeat(false)
, mIsComposing(false)
, mIsSynthesizedByTIP(false)
, mMaybeSkippableInRemoteProcess(true)
, mEditCommandsForSingleLineEditorInitialized(false)
, mEditCommandsForMultiLineEditorInitialized(false)
, mEditCommandsForRichTextEditorInitialized(false)
@ -192,6 +193,7 @@ public:
, mIsRepeat(false)
, mIsComposing(false)
, mIsSynthesizedByTIP(false)
, mMaybeSkippableInRemoteProcess(true)
, mEditCommandsForSingleLineEditorInitialized(false)
, mEditCommandsForMultiLineEditorInitialized(false)
, mEditCommandsForRichTextEditorInitialized(false)
@ -393,6 +395,25 @@ public:
// Indicates whether the event is synthesized from Text Input Processor
// or an actual event from nsAppShell.
bool mIsSynthesizedByTIP;
// Indicates whether the event is skippable in remote process.
// Don't refer this member directly when you need to check this.
// Use CanSkipInRemoteProcess() instead.
bool mMaybeSkippableInRemoteProcess;
bool CanSkipInRemoteProcess() const
{
// If this is a repeat event (i.e., generated by auto-repeat feature of
// the platform), remove process may skip to handle it because of
// performances reasons.. However, if it's caused by odd keyboard utils,
// we should not ignore any key events even marked as repeated since
// generated key sequence may be important to input proper text. E.g.,
// "SinhalaTamil IME" on Windows emulates dead key like input with
// generating WM_KEYDOWN for VK_PACKET (inputting any Unicode characters
// without keyboard layout information) and VK_BACK (Backspace) to remove
// previous character(s) and those messages may be marked as "repeat" by
// their bug.
return mIsRepeat && mMaybeSkippableInRemoteProcess;
}
/**
* Retrieves all edit commands from mWidget. This shouldn't be called when
@ -621,6 +642,7 @@ public:
mPluginTextEventString.Assign(aEvent.mPluginTextEventString);
#endif
mIsSynthesizedByTIP = aEvent.mIsSynthesizedByTIP;
mMaybeSkippableInRemoteProcess = aEvent.mMaybeSkippableInRemoteProcess;
// Don't copy mEditCommandsFor*Editor because it may require a lot of
// memory space. For example, if the event is dispatched but grabbed by

2
widget/nsGUIEventIPC.h
View File

@ -445,6 +445,7 @@ struct ParamTraits<mozilla::WidgetKeyboardEvent>
WriteParam(aMsg, aParam.mLocation);
WriteParam(aMsg, aParam.mUniqueId);
WriteParam(aMsg, aParam.mIsSynthesizedByTIP);
WriteParam(aMsg, aParam.mMaybeSkippableInRemoteProcess);
#ifdef XP_MACOSX
WriteParam(aMsg, aParam.mNativeKeyCode);
WriteParam(aMsg, aParam.mNativeModifierFlags);
@ -482,6 +483,7 @@ struct ParamTraits<mozilla::WidgetKeyboardEvent>
ReadParam(aMsg, aIter, &aResult->mLocation) &&
ReadParam(aMsg, aIter, &aResult->mUniqueId) &&
ReadParam(aMsg, aIter, &aResult->mIsSynthesizedByTIP) &&
ReadParam(aMsg, aIter, &aResult->mMaybeSkippableInRemoteProcess) &&
#ifdef XP_MACOSX
ReadParam(aMsg, aIter, &aResult->mNativeKeyCode) &&
ReadParam(aMsg, aIter, &aResult->mNativeModifierFlags) &&

140
widget/windows/KeyboardLayout.cpp
View File

@ -1146,6 +1146,7 @@ VirtualKey::FillKbdState(PBYTE aKbdState,
uint8_t NativeKey::sDispatchedKeyOfAppCommand = 0;
NativeKey* NativeKey::sLatestInstance = nullptr;
const MSG NativeKey::sEmptyMSG = {};
MSG NativeKey::sLastKeyOrCharMSG = {};
MSG NativeKey::sLastKeyMSG = {};
LazyLogModule sNativeKeyLogger("NativeKeyWidgets");
@ -1172,7 +1173,10 @@ NativeKey::NativeKey(nsWindowBase* aWidget,
, mUnshiftedLatinChar(0)
, mScanCode(0)
, mIsExtended(false)
, mIsRepeat(false)
, mIsDeadKey(false)
, mIsPrintableKey(false)
, mIsSkippableInRemoteProcess(false)
, mCharMessageHasGone(false)
, mCanIgnoreModifierStateAtKeyPress(true)
, mFakeCharMsgs(aFakeCharMsgs && aFakeCharMsgs->Length() ?
@ -1196,13 +1200,13 @@ NativeKey::NativeKey(nsWindowBase* aWidget,
mIsOverridingKeyboardLayout = true;
} else {
mIsOverridingKeyboardLayout = false;
sLastKeyMSG = aMessage;
sLastKeyOrCharMSG = aMessage;
}
if (mMsg.message == WM_APPCOMMAND) {
InitWithAppCommand();
} else {
InitWithKeyChar();
InitWithKeyOrChar();
}
MOZ_LOG(sNativeKeyLogger, LogLevel::Info,
@ -1213,7 +1217,8 @@ NativeKey::NativeKey(nsWindowBase* aWidget,
"mCommittedCharsAndModifiers=%s, mInputtingStringAndModifiers=%s, "
"mShiftedString=%s, mUnshiftedString=%s, mShiftedLatinChar=%s, "
"mUnshiftedLatinChar=%s, mScanCode=0x%04X, mIsExtended=%s, "
"mIsDeadKey=%s, mIsPrintableKey=%s, mCharMessageHasGone=%s, "
"mIsRepeat=%s, mIsDeadKey=%s, mIsPrintableKey=%s, "
"mIsSkippableInRemoteProcess=%s, mCharMessageHasGone=%s, "
"mIsOverridingKeyboardLayout=%s",
this, mKeyboardLayout, mFocusedWndBeforeDispatch,
GetDOMKeyCodeName(mDOMKeyCode).get(), ToString(mKeyNameIndex).get(),
@ -1226,14 +1231,103 @@ NativeKey::NativeKey(nsWindowBase* aWidget,
ToString(mShiftedString).get(), ToString(mUnshiftedString).get(),
GetCharacterCodeName(mShiftedLatinChar).get(),
GetCharacterCodeName(mUnshiftedLatinChar).get(),
mScanCode, GetBoolName(mIsExtended), GetBoolName(mIsDeadKey),
GetBoolName(mIsPrintableKey), GetBoolName(mCharMessageHasGone),
mScanCode, GetBoolName(mIsExtended),
GetBoolName(mIsRepeat), GetBoolName(mIsDeadKey),
GetBoolName(mIsPrintableKey), GetBoolName(mIsSkippableInRemoteProcess),
GetBoolName(mCharMessageHasGone),
GetBoolName(mIsOverridingKeyboardLayout)));
}
void
NativeKey::InitWithKeyChar()
NativeKey::InitIsSkippableForKeyOrChar(const MSG& aLastKeyMSG)
{
mIsSkippableInRemoteProcess = false;
if (!mIsRepeat) {
// If the message is not repeated key message, the event should be always
// handled in remote process even if it's too old.
return;
}
// Keyboard utilities may send us some generated messages and such messages
// may be marked as "repeated", e.g., SendInput() calls with
// KEYEVENTF_UNICODE but without KEYEVENTF_KEYUP. However, key sequence
// comes from such utilities may be really important. For example, utilities
// may send WM_KEYDOWN for VK_BACK to remove previous character and send
// WM_KEYDOWN for VK_PACKET to insert a composite character. Therefore, we
// should check if current message and previous key message are caused by
// same physical key. If not, the message may be generated by such
// utility.
// XXX With this approach, if VK_BACK messages are generated with known
// scancode, we cannot distinguish whether coming VK_BACK message is
// actually repeated by the auto-repeat feature. Currently, we need
// this hack only for "SinhalaTamil IME" and fortunately, it generates
// VK_BACK messages with odd scancode. So, we don't need to handle
// VK_BACK specially at least for now.
if (mCodeNameIndex == CODE_NAME_INDEX_UNKNOWN) {
// If current event is not caused by physical key operation, it may be
// caused by a keyboard utility. If so, the event shouldn't be ignored by
// TabChild since it want to insert the character, delete a character or
// move caret.
return;
}
if (mOriginalVirtualKeyCode == VK_PACKET) {
// If the message is VK_PACKET, that means that a keyboard utility
// tries to insert a character.
return;
}
switch (mMsg.message) {
case WM_KEYDOWN:
case WM_SYSKEYDOWN:
case MOZ_WM_KEYDOWN:
case WM_CHAR:
case WM_SYSCHAR:
case WM_DEADCHAR:
case WM_SYSDEADCHAR:
// However, some keyboard layouts may send some keyboard messages with
// activating the bit. If we dispatch repeated keyboard events, they
// may be ignored by TabChild due to performance reason. So, we need
// to check if actually a physical key is repeated by the auto-repeat
// feature.
switch (aLastKeyMSG.message) {
case WM_KEYDOWN:
case WM_SYSKEYDOWN:
case MOZ_WM_KEYDOWN:
if (aLastKeyMSG.wParam == VK_PACKET) {
// If the last message was VK_PACKET, that means that a keyboard
// utility tried to insert a character. So, current message is
// not repeated key event of the previous event.
return;
}
// Let's check whether current message and previous message are
// caused by same physical key.
mIsSkippableInRemoteProcess =
mScanCode == WinUtils::GetScanCode(aLastKeyMSG.lParam) &&
mIsExtended == WinUtils::IsExtendedScanCode(aLastKeyMSG.lParam);
return;
default:
// If previous message is not a keydown, this must not be generated
// by the auto-repeat feature.
return;
}
return;
case WM_APPCOMMAND:
MOZ_ASSERT_UNREACHABLE("WM_APPCOMMAND should be handled in "
"InitWithAppCommand()");
return;
default:
// keyup message shouldn't be repeated by the auto-repeat feature.
return;
}
}
void
NativeKey::InitWithKeyOrChar()
{
MSG lastKeyMSG = sLastKeyMSG;
mScanCode = WinUtils::GetScanCode(mMsg.lParam);
mIsExtended = WinUtils::IsExtendedScanCode(mMsg.lParam);
switch (mMsg.message) {
@ -1243,6 +1337,10 @@ NativeKey::InitWithKeyChar()
case WM_SYSKEYUP:
case MOZ_WM_KEYDOWN:
case MOZ_WM_KEYUP: {
// Modify sLastKeyMSG now since retrieving following char messages may
// cause sending another key message if odd tool hooks GetMessage(),
// PeekMessage().
sLastKeyMSG = mMsg;
// First, resolve the IME converted virtual keycode to its original
// keycode.
if (mMsg.wParam == VK_PROCESSKEY) {
@ -1355,7 +1453,7 @@ NativeKey::InitWithKeyChar()
// XXX Do we need to make mReceivedMsg an array?
MOZ_ASSERT(IsEmptyMSG(mLastInstance->mReceivedMsg));
MOZ_LOG(sNativeKeyLogger, LogLevel::Warning,
("%p NativeKey::InitWithKeyChar(), WARNING, detecting another "
("%p NativeKey::InitWithKeyOrChar(), WARNING, detecting another "
"instance is trying to remove a char message, so, this instance "
"should do nothing, mLastInstance=0x%p, mRemovingMsg=%s, "
"mReceivedMsg=%s",
@ -1416,7 +1514,7 @@ NativeKey::InitWithKeyChar()
continue;
}
MOZ_LOG(sNativeKeyLogger, LogLevel::Info,
("%p NativeKey::InitWithKeyChar(), removed char message, %s",
("%p NativeKey::InitWithKeyOrChar(), removed char message, %s",
this, ToString(charMsg).get()));
Unused << NS_WARN_IF(charMsg.hwnd != mMsg.hwnd);
mFollowingCharMsgs.AppendElement(charMsg);
@ -1431,6 +1529,13 @@ NativeKey::InitWithKeyChar()
mIsPrintableKey =
mKeyNameIndex == KEY_NAME_INDEX_USE_STRING ||
KeyboardLayout::IsPrintableCharKey(mOriginalVirtualKeyCode);
// The repeat count in mMsg.lParam isn't useful to check whether the event
// is caused by the auto-repeat feature because it's not incremented even
// if it's repeated twice or more (i.e., always 1). Therefore, we need to
// check previous key state (31th bit) instead. If it's 1, the key was down
// before the message was sent.
mIsRepeat = (mMsg.lParam & (1 << 30)) != 0;
InitIsSkippableForKeyOrChar(lastKeyMSG);
if (IsKeyDownMessage()) {
// Compute some strings which may be inputted by the key with various
@ -1573,7 +1678,21 @@ NativeKey::InitWithAppCommand()
ConvertNativeKeyCodeToDOMKeyCode(mOriginalVirtualKeyCode);
mCodeNameIndex =
KeyboardLayout::ConvertScanCodeToCodeNameIndex(
GetScanCodeWithExtendedFlag());
GetScanCodeWithExtendedFlag());
// If we can map the WM_APPCOMMAND to a virtual keycode, we can trust
// the result of GetKeyboardState(). Otherwise, we dispatch both
// keydown and keyup events from WM_APPCOMMAND handler. Therefore,
// even if WM_APPCOMMAND is caused by auto key repeat, web apps receive
// a pair of DOM keydown and keyup events. I.e., KeyboardEvent.repeat
// should be never true of such keys.
// XXX Isn't the key state always true? If the key press caused this
// WM_APPCOMMAND, that means it's pressed at that time.
if (mVirtualKeyCode) {
BYTE kbdState[256];
memset(kbdState, 0, sizeof(kbdState));
::GetKeyboardState(kbdState);
mIsSkippableInRemoteProcess = mIsRepeat = !!kbdState[mVirtualKeyCode];
}
}
// static
@ -1875,7 +1994,8 @@ NativeKey::InitKeyEvent(WidgetKeyboardEvent& aKeyEvent,
MOZ_CRASH("Invalid event message");
}
aKeyEvent.mIsRepeat = IsRepeat();
aKeyEvent.mIsRepeat = mIsRepeat;
aKeyEvent.mMaybeSkippableInRemoteProcess = mIsSkippableInRemoteProcess;
aKeyEvent.mKeyNameIndex = mKeyNameIndex;
if (mKeyNameIndex == KEY_NAME_INDEX_USE_STRING) {
aKeyEvent.mKeyValue = mCommittedCharsAndModifiers.ToString();

69
widget/windows/KeyboardLayout.h
View File

@ -417,25 +417,31 @@ private:
uint32_t mShiftedLatinChar;
uint32_t mUnshiftedLatinChar;
WORD mScanCode;
bool mIsExtended;
bool mIsDeadKey;
WORD mScanCode;
bool mIsExtended;
// mIsRepeat is true if the key message is caused by the auto-repeat
// feature.
bool mIsRepeat;
bool mIsDeadKey;
// mIsPrintableKey is true if the key may be a printable key without
// any modifier keys. Otherwise, false.
// Please note that the event may not cause any text input even if this
// is true. E.g., it might be dead key state or Ctrl key may be pressed.
bool mIsPrintableKey;
bool mIsPrintableKey;
// mIsSkippableInRemoteProcess is false if the key event shouldn't be
// skipped in the remote process even if it's too old event.
bool mIsSkippableInRemoteProcess;
// mCharMessageHasGone is true if the message is a keydown message and
// it's followed by at least one char message but it's gone at removing
// from the queue. This could occur if PeekMessage() or something is
// hooked by odd tool.
bool mCharMessageHasGone;
bool mCharMessageHasGone;
// mIsOverridingKeyboardLayout is true if the instance temporarily overriding
// keyboard layout with specified by the constructor.
bool mIsOverridingKeyboardLayout;
bool mIsOverridingKeyboardLayout;
// mCanIgnoreModifierStateAtKeyPress is true if it's allowed to remove
// Ctrl or Alt modifier state at dispatching eKeyPress.
bool mCanIgnoreModifierStateAtKeyPress;
bool mCanIgnoreModifierStateAtKeyPress;
nsTArray<FakeCharMsg>* mFakeCharMsgs;
@ -451,7 +457,14 @@ private:
}
void InitWithAppCommand();
void InitWithKeyChar();
void InitWithKeyOrChar();
/**
* InitIsSkippableForKeyOrChar() initializes mIsSkippableInRemoteProcess with
* mIsRepeat and previous key message information. So, this must be called
* after mIsRepeat is initialized.
*/
void InitIsSkippableForKeyOrChar(const MSG& aLastKeyMSG);
/**
* InitCommittedCharsAndModifiersWithFollowingCharMessages() initializes
@ -462,40 +475,6 @@ private:
void InitCommittedCharsAndModifiersWithFollowingCharMessages(
const ModifierKeyState& aModKeyState);
/**
* Returns true if the key event is caused by auto repeat.
*/
bool IsRepeat() const
{
switch (mMsg.message) {
case WM_KEYDOWN:
case WM_SYSKEYDOWN:
case WM_CHAR:
case WM_SYSCHAR:
case WM_DEADCHAR:
case WM_SYSDEADCHAR:
case MOZ_WM_KEYDOWN:
return ((mMsg.lParam & (1 << 30)) != 0);
case WM_APPCOMMAND:
if (mVirtualKeyCode) {
// If we can map the WM_APPCOMMAND to a virtual keycode, we can trust
// the result of GetKeyboardState().
BYTE kbdState[256];
memset(kbdState, 0, sizeof(kbdState));
::GetKeyboardState(kbdState);
return !!kbdState[mVirtualKeyCode];
}
// If there is no virtual keycode for the command, we dispatch both
// keydown and keyup events from WM_APPCOMMAND handler. Therefore,
// even if WM_APPCOMMAND is caused by auto key repeat, web apps receive
// a pair of DOM keydown and keyup events. I.e., KeyboardEvent.repeat
// should be never true of such keys.
return false;
default:
return false;
}
}
UINT GetScanCodeWithExtendedFlag() const;
// The result is one of eKeyLocation*.
@ -714,6 +693,8 @@ private:
static const MSG sEmptyMSG;
static MSG sLastKeyOrCharMSG;
static MSG sLastKeyMSG;
static bool IsEmptyMSG(const MSG& aMSG)
@ -728,10 +709,10 @@ private:
public:
/**
* Returns last key MSG. If no key MSG has been received yet, the result
* Returns last key or char MSG. If no MSG has been received yet, the result
* is empty MSG (i.e., .message is WM_NULL).
*/
static const MSG& LastKeyMSG() { return sLastKeyMSG; }
static const MSG& LastKeyOrCharMSG() { return sLastKeyOrCharMSG; }
};
class KeyboardLayout

4
widget/windows/nsWindow.cpp
View File

@ -4487,8 +4487,8 @@ nsWindow::DispatchMouseEvent(EventMessage aEventMessage, WPARAM wParam,
// XXX Should we allow to block web page to prevent its default with
// Ctrl+Shift+F10 or Alt+Shift+F10 instead?
if (aEventMessage == eContextMenu && aIsContextMenuKey && event.IsShift() &&
NativeKey::LastKeyMSG().message == WM_SYSKEYDOWN &&
NativeKey::LastKeyMSG().wParam == VK_F10) {
NativeKey::LastKeyOrCharMSG().message == WM_SYSKEYDOWN &&
NativeKey::LastKeyOrCharMSG().wParam == VK_F10) {
event.mModifiers &= ~MODIFIER_SHIFT;
}