Skip to content

Commit

Permalink
Bug 900750 - part 3: Remove unnecessary ModifierKeyState argument fro…
Browse files Browse the repository at this point in the history
…m some methods of NativeKey and KeyboardLayout r=m_kato

KeyboardLayout::InitNativeKey() takes |const ModifierKeyState&| as its
argument with NativeKey reference and it calls some internal methods with
the given ModifierKeyState without any changes.  Additionally, its caller
is only NativeKey::InitWithKeyChar() and its called with given NativeKey
instance's mModKeyState.  So, removing the redundant arguments from
some methods makes them clearer what they compute with.

So, this patch does not change any behavior.

MozReview-Commit-ID: 3w9Ee7PMU05

--HG--
extra : rebase_source : b724a18d5a14672e60ffa5fb9feca5c11dac42a3
  • Loading branch information
masayuki-nakano committed Jun 1, 2018
1 parent 8fc19f6 commit ad475fd
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 81 deletions.
75 changes: 24 additions & 51 deletions widget/windows/KeyboardLayout.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1524,7 +1524,7 @@ NativeKey::InitWithKeyOrChar()
}
}

keyboardLayout->InitNativeKey(*this, mModKeyState);
keyboardLayout->InitNativeKey(*this);

mIsDeadKey =
(IsFollowedByDeadCharMessage() ||
Expand Down Expand Up @@ -1554,14 +1554,13 @@ NativeKey::InitWithKeyOrChar()
}

void
NativeKey::InitCommittedCharsAndModifiersWithFollowingCharMessages(
const ModifierKeyState& aModKeyState)
NativeKey::InitCommittedCharsAndModifiersWithFollowingCharMessages()
{
mCommittedCharsAndModifiers.Clear();
// This should cause inputting text in focused editor. However, it
// ignores keypress events whose altKey or ctrlKey is true.
// Therefore, we need to remove these modifier state here.
Modifiers modifiers = aModKeyState.GetModifiers();
Modifiers modifiers = mModKeyState.GetModifiers();
if (IsFollowedByPrintableCharMessage()) {
modifiers &= ~(MODIFIER_ALT | MODIFIER_CONTROL);
}
Expand Down Expand Up @@ -3840,8 +3839,7 @@ KeyboardLayout::IsSysKey(uint8_t aVirtualKey,
}

void
KeyboardLayout::InitNativeKey(NativeKey& aNativeKey,
const ModifierKeyState& aModKeyState)
KeyboardLayout::InitNativeKey(NativeKey& aNativeKey)
{
if (mIsPendingToRestoreKeyboardLayout) {
LoadLayout(::GetKeyboardLayout(0));
Expand All @@ -3857,7 +3855,7 @@ KeyboardLayout::InitNativeKey(NativeKey& aNativeKey,
if (!NativeKey::IsControlChar(ch)) {
aNativeKey.mKeyNameIndex = KEY_NAME_INDEX_USE_STRING;
Modifiers modifiers =
aModKeyState.GetModifiers() & ~(MODIFIER_ALT | MODIFIER_CONTROL);
aNativeKey.GetModifiers() & ~(MODIFIER_ALT | MODIFIER_CONTROL);
aNativeKey.mCommittedCharsAndModifiers.Append(ch, modifiers);
return;
}
Expand All @@ -3876,8 +3874,7 @@ KeyboardLayout::InitNativeKey(NativeKey& aNativeKey,
MOZ_ASSERT(!aNativeKey.IsCharMessage(aNativeKey.mMsg));
if (aNativeKey.IsFollowedByPrintableCharOrSysCharMessage()) {
// Initialize mCommittedCharsAndModifiers with following char messages.
aNativeKey.
InitCommittedCharsAndModifiersWithFollowingCharMessages(aModKeyState);
aNativeKey.InitCommittedCharsAndModifiersWithFollowingCharMessages();
MOZ_ASSERT(!aNativeKey.mCommittedCharsAndModifiers.IsEmpty());

// Currently, we are doing a ugly hack to keypress events to cause
Expand All @@ -3888,11 +3885,11 @@ KeyboardLayout::InitNativeKey(NativeKey& aNativeKey,
// we should mark as not removable if Ctrl or Alt key does not cause
// changing inputting character.
if (IsPrintableCharKey(aNativeKey.mOriginalVirtualKeyCode) &&
(aModKeyState.IsControl() ^ aModKeyState.IsAlt())) {
ModifierKeyState state = aModKeyState;
(aNativeKey.IsControl() ^ aNativeKey.IsAlt())) {
ModifierKeyState state = aNativeKey.ModifierKeyStateRef();
state.Unset(MODIFIER_ALT | MODIFIER_CONTROL);
UniCharsAndModifiers charsWithoutModifier =
GetUniCharsAndModifiers(aNativeKey.mOriginalVirtualKeyCode, state);
GetUniCharsAndModifiers(aNativeKey.GenericVirtualKeyCode(), state);
aNativeKey.mCanIgnoreModifierStateAtKeyPress =
!charsWithoutModifier.UniCharsEqual(
aNativeKey.mCommittedCharsAndModifiers);
Expand Down Expand Up @@ -3920,7 +3917,7 @@ KeyboardLayout::InitNativeKey(NativeKey& aNativeKey,

// If it's a dead key, aNativeKey will be initialized by
// MaybeInitNativeKeyAsDeadKey().
if (MaybeInitNativeKeyAsDeadKey(aNativeKey, aModKeyState)) {
if (MaybeInitNativeKeyAsDeadKey(aNativeKey)) {
return;
}

Expand All @@ -3941,12 +3938,11 @@ KeyboardLayout::InitNativeKey(NativeKey& aNativeKey,
// If it's in dead key handling and the pressed key causes a composite
// character, aNativeKey will be initialized by
// MaybeInitNativeKeyWithCompositeChar().
if (MaybeInitNativeKeyWithCompositeChar(aNativeKey, aModKeyState)) {
if (MaybeInitNativeKeyWithCompositeChar(aNativeKey)) {
return;
}

UniCharsAndModifiers baseChars =
GetUniCharsAndModifiers(aNativeKey.mOriginalVirtualKeyCode, aModKeyState);
UniCharsAndModifiers baseChars = GetUniCharsAndModifiers(aNativeKey);

// If the key press isn't related to any dead keys, initialize aNativeKey
// with the characters which should be caused by the key.
Expand All @@ -3966,13 +3962,10 @@ KeyboardLayout::InitNativeKey(NativeKey& aNativeKey,
}

bool
KeyboardLayout::MaybeInitNativeKeyAsDeadKey(
NativeKey& aNativeKey,
const ModifierKeyState& aModKeyState)
KeyboardLayout::MaybeInitNativeKeyAsDeadKey(NativeKey& aNativeKey)
{
// Only when it's not in dead key sequence, we can trust IsDeadKey() result.
if (!IsInDeadKeySequence() &&
!IsDeadKey(aNativeKey.mOriginalVirtualKeyCode, aModKeyState)) {
if (!IsInDeadKeySequence() && !IsDeadKey(aNativeKey)) {
return false;
}

Expand All @@ -3987,10 +3980,10 @@ KeyboardLayout::MaybeInitNativeKeyAsDeadKey(
// different.
bool isDeadKeyUpEvent =
!aNativeKey.IsKeyDownMessage() &&
mActiveDeadKeys.Contains(aNativeKey.mOriginalVirtualKeyCode);
mActiveDeadKeys.Contains(aNativeKey.GenericVirtualKeyCode());

if (isDeadKeyDownEvent || isDeadKeyUpEvent) {
ActivateDeadKeyState(aNativeKey, aModKeyState);
ActivateDeadKeyState(aNativeKey);
// Any dead key events don't generate characters. So, a dead key should
// cause only keydown event and keyup event whose KeyboardEvent.key
// values are "Dead".
Expand All @@ -4006,14 +3999,14 @@ KeyboardLayout::MaybeInitNativeKeyAsDeadKey(
// set only a character for current key for keyup event.
if (!IsInDeadKeySequence()) {
aNativeKey.mCommittedCharsAndModifiers =
GetUniCharsAndModifiers(aNativeKey.mOriginalVirtualKeyCode, aModKeyState);
GetUniCharsAndModifiers(aNativeKey);
return true;
}

// When non-printable key event comes during a dead key sequence, that must
// be a modifier key event. So, such events shouldn't be handled as a part
// of the dead key sequence.
if (!IsDeadKey(aNativeKey.mOriginalVirtualKeyCode, aModKeyState)) {
if (!IsDeadKey(aNativeKey)) {
return false;
}

Expand All @@ -4024,15 +4017,14 @@ KeyboardLayout::MaybeInitNativeKeyAsDeadKey(

// Dead key followed by another dead key may cause a composed character
// (e.g., "Russian - Mnemonic" keyboard layout's 's' -> 'c').
if (MaybeInitNativeKeyWithCompositeChar(aNativeKey, aModKeyState)) {
if (MaybeInitNativeKeyWithCompositeChar(aNativeKey)) {
return true;
}

// Otherwise, dead key followed by another dead key causes inputting both
// character.
UniCharsAndModifiers prevDeadChars = GetDeadUniCharsAndModifiers();
UniCharsAndModifiers newChars =
GetUniCharsAndModifiers(aNativeKey.mOriginalVirtualKeyCode, aModKeyState);
UniCharsAndModifiers newChars = GetUniCharsAndModifiers(aNativeKey);
// But keypress events should be fired for each committed character.
aNativeKey.mCommittedCharsAndModifiers = prevDeadChars + newChars;
if (aNativeKey.IsKeyDownMessage()) {
Expand All @@ -4042,9 +4034,7 @@ KeyboardLayout::MaybeInitNativeKeyAsDeadKey(
}

bool
KeyboardLayout::MaybeInitNativeKeyWithCompositeChar(
NativeKey& aNativeKey,
const ModifierKeyState& aModKeyState)
KeyboardLayout::MaybeInitNativeKeyWithCompositeChar(NativeKey& aNativeKey)
{
if (!IsInDeadKeySequence()) {
return false;
Expand All @@ -4054,8 +4044,7 @@ KeyboardLayout::MaybeInitNativeKeyWithCompositeChar(
return false;
}

UniCharsAndModifiers baseChars =
GetUniCharsAndModifiers(aNativeKey.mOriginalVirtualKeyCode, aModKeyState);
UniCharsAndModifiers baseChars = GetUniCharsAndModifiers(aNativeKey);
if (baseChars.IsEmpty() || !baseChars.CharAt(0)) {
return false;
}
Expand Down Expand Up @@ -4088,20 +4077,6 @@ KeyboardLayout::GetUniCharsAndModifiers(
return mVirtualKeys[key].GetUniChars(aShiftState);
}

UniCharsAndModifiers
KeyboardLayout::GetNativeUniCharsAndModifiers(
uint8_t aVirtualKey,
const ModifierKeyState& aModKeyState) const
{
int32_t key = GetKeyIndex(aVirtualKey);
if (key < 0) {
return UniCharsAndModifiers();
}
VirtualKey::ShiftState shiftState =
VirtualKey::ModifierKeyStateToShiftState(aModKeyState);
return mVirtualKeys[key].GetNativeUniChars(shiftState);
}

UniCharsAndModifiers
KeyboardLayout::GetDeadUniCharsAndModifiers() const
{
Expand Down Expand Up @@ -4533,17 +4508,15 @@ KeyboardLayout::EnsureDeadKeyActive(bool aIsActive,
}

void
KeyboardLayout::ActivateDeadKeyState(const NativeKey& aNativeKey,
const ModifierKeyState& aModKeyState)
KeyboardLayout::ActivateDeadKeyState(const NativeKey& aNativeKey)
{
// Dead-key state should be activated at keydown.
if (!aNativeKey.IsKeyDownMessage()) {
return;
}

mActiveDeadKeys.AppendElement(aNativeKey.mOriginalVirtualKeyCode);
mDeadKeyShiftStates.AppendElement(
VirtualKey::ModifierKeyStateToShiftState(aModKeyState));
mDeadKeyShiftStates.AppendElement(aNativeKey.GetShiftState());
}

void
Expand Down
87 changes: 57 additions & 30 deletions widget/windows/KeyboardLayout.h
Original file line number Diff line number Diff line change
Expand Up @@ -478,6 +478,33 @@ class MOZ_STACK_CLASS NativeKey final
*/
static bool IsControlChar(char16_t aChar);

bool IsControl() const { return mModKeyState.IsControl(); }
bool IsAlt() const { return mModKeyState.IsAlt(); }
Modifiers GetModifiers() const { return mModKeyState.GetModifiers(); }
const ModifierKeyState& ModifierKeyStateRef() const
{
return mModKeyState;
}
VirtualKey::ShiftState GetShiftState() const
{
return VirtualKey::ModifierKeyStateToShiftState(mModKeyState);
}

/**
* GenericVirtualKeyCode() returns virtual keycode which cannot distinguish
* position of modifier keys. E.g., VK_CONTROL for both ControlLeft and
* ControlRight.
*/
uint8_t GenericVirtualKeyCode() const { return mOriginalVirtualKeyCode; }

/**
* SpecificVirtualKeyCode() returns virtual keycode which can distinguish
* position of modifier keys. E.g., returns VK_LCONTROL or VK_RCONTROL
* instead of VK_CONTROL. If the key message is synthesized with not
* enough information, this prefers left position's keycode.
*/
uint8_t SpecificVirtualKeyCode() const { return mVirtualKeyCode; }

private:
NativeKey* mLastInstance;
// mRemovingMsg is set at removing a char message from
Expand Down Expand Up @@ -594,12 +621,11 @@ class MOZ_STACK_CLASS NativeKey final

/**
* InitCommittedCharsAndModifiersWithFollowingCharMessages() initializes
* mCommittedCharsAndModifiers with mFollowingCharMsgs and aModKeyState.
* mCommittedCharsAndModifiers with mFollowingCharMsgs and mModKeyState.
* If mFollowingCharMsgs includes non-printable char messages, they are
* ignored (skipped).
*/
void InitCommittedCharsAndModifiersWithFollowingCharMessages(
const ModifierKeyState& aModKeyState);
void InitCommittedCharsAndModifiersWithFollowingCharMessages();

UINT GetScanCodeWithExtendedFlag() const;

Expand Down Expand Up @@ -864,6 +890,11 @@ class KeyboardLayout
*/
bool IsDeadKey(uint8_t aVirtualKey,
const ModifierKeyState& aModKeyState) const;
bool IsDeadKey(const NativeKey& aNativeKey) const
{
return IsDeadKey(aNativeKey.GenericVirtualKeyCode(),
aNativeKey.ModifierKeyStateRef());
}

/**
* IsInDeadKeySequence() returns true when it's in a dead key sequence.
Expand All @@ -878,31 +909,32 @@ class KeyboardLayout
*/
bool IsSysKey(uint8_t aVirtualKey,
const ModifierKeyState& aModKeyState) const;
bool IsSysKey(const NativeKey& aNativeKey) const
{
return IsSysKey(aNativeKey.GenericVirtualKeyCode(),
aNativeKey.ModifierKeyStateRef());
}

/**
* GetUniCharsAndModifiers() returns characters which are inputted by
* aVirtualKey with aModKeyState. This method isn't stateful.
* Note that if the combination causes text input, the result's Ctrl and
* Alt key state are never active.
*/
UniCharsAndModifiers GetUniCharsAndModifiers(
uint8_t aVirtualKey,
const ModifierKeyState& aModKeyState) const
UniCharsAndModifiers
GetUniCharsAndModifiers(uint8_t aVirtualKey,
const ModifierKeyState& aModKeyState) const
{
VirtualKey::ShiftState shiftState =
VirtualKey::ModifierKeyStateToShiftState(aModKeyState);
return GetUniCharsAndModifiers(aVirtualKey, shiftState);
}

/**
* GetNativeUniCharsAndModifiers() returns characters which are inputted by
* aVirtualKey with aModKeyState. The method isn't stateful.
* Note that different from GetUniCharsAndModifiers(), this returns
* actual modifier state of Ctrl and Alt.
*/
UniCharsAndModifiers GetNativeUniCharsAndModifiers(
uint8_t aVirtualKey,
const ModifierKeyState& aModKeyState) const;
UniCharsAndModifiers
GetUniCharsAndModifiers(const NativeKey& aNativeKey) const
{
return GetUniCharsAndModifiers(aNativeKey.GenericVirtualKeyCode(),
aNativeKey.GetShiftState());
}

/**
* OnLayoutChange() must be called before the first keydown message is
Expand Down Expand Up @@ -1004,9 +1036,9 @@ class KeyboardLayout
static int CompareDeadKeyEntries(const void* aArg1, const void* aArg2,
void* aData);
static bool AddDeadKeyEntry(char16_t aBaseChar, char16_t aCompositeChar,
DeadKeyEntry* aDeadKeyArray, uint32_t aEntries);
DeadKeyEntry* aDeadKeyArray, uint32_t aEntries);
bool EnsureDeadKeyActive(bool aIsActive, uint8_t aDeadKey,
const PBYTE aDeadKeyKbdState);
const PBYTE aDeadKeyKbdState);
uint32_t GetDeadKeyCombinations(uint8_t aDeadKey,
const PBYTE aDeadKeyKbdState,
uint16_t aShiftStatesWithBaseChars,
Expand All @@ -1015,8 +1047,7 @@ class KeyboardLayout
/**
* Activates or deactivates dead key state.
*/
void ActivateDeadKeyState(const NativeKey& aNativeKey,
const ModifierKeyState& aModKeyState);
void ActivateDeadKeyState(const NativeKey& aNativeKey);
void DeactivateDeadKeyState();

const DeadKeyTable* AddDeadKeyTable(const DeadKeyEntry* aDeadKeyArray,
Expand All @@ -1041,8 +1072,7 @@ class KeyboardLayout
* WM_KEYDOWN. Additionally, computes current inputted character(s) and set
* them to the aNativeKey.
*/
void InitNativeKey(NativeKey& aNativeKey,
const ModifierKeyState& aModKeyState);
void InitNativeKey(NativeKey& aNativeKey);

/**
* MaybeInitNativeKeyAsDeadKey() initializes aNativeKey only when aNativeKey
Expand All @@ -1053,24 +1083,21 @@ class KeyboardLayout
* be caused by aNativeKey.
* Returns true when this initializes aNativeKey. Otherwise, false.
*/
bool MaybeInitNativeKeyAsDeadKey(NativeKey& aNativeKey,
const ModifierKeyState& aModKeyState);
bool MaybeInitNativeKeyAsDeadKey(NativeKey& aNativeKey);

/**
* MaybeInitNativeKeyWithCompositeChar() may initialize aNativeKey with
* proper composite character when dead key produces a composite character.
* Otherwise, just returns false.
*/
bool MaybeInitNativeKeyWithCompositeChar(
NativeKey& aNativeKey,
const ModifierKeyState& aModKeyState);
bool MaybeInitNativeKeyWithCompositeChar(NativeKey& aNativeKey);

/**
* See the comment of GetUniCharsAndModifiers() below.
*/
UniCharsAndModifiers GetUniCharsAndModifiers(
uint8_t aVirtualKey,
VirtualKey::ShiftState aShiftState) const;
UniCharsAndModifiers
GetUniCharsAndModifiers(uint8_t aVirtualKey,
VirtualKey::ShiftState aShiftState) const;

/**
* GetDeadUniCharsAndModifiers() returns dead chars which are stored in
Expand Down

0 comments on commit ad475fd

Please sign in to comment.