Bug 1297985 - Part 2: KeyboardLayout should handle a composite character produced by 2 dead keys. r=m_kato, a=ritu
authorMasayuki Nakano <masayuki@d-toybox.com>
Thu, 01 Sep 2016 17:29:11 +0900
changeset 350153 befed59939f765b9bb263572d7c16e096612986b
parent 350152 0ebd731f42bb9685f3438ce25aebe49311abf3e5
child 350154 fe5e0299f26d8b493f325f7fe2fe474a69e41f8c
push id1230
push userjlund@mozilla.com
push dateMon, 31 Oct 2016 18:13:35 +0000
treeherdermozilla-release@5e06e3766db2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato, ritu
bugs1297985
milestone50.0a2
Bug 1297985 - Part 2: KeyboardLayout should handle a composite character produced by 2 dead keys. r=m_kato, a=ritu On some keyboard layouts, a key sequence, a dead key -> another dead key, may produce a composite character instead of two base characters for each key. For example, with "Russian - Mnemonic" keyboard layout on Win 8 or later, both 's' and 'c' are dead keys but key sequence, 's' -> 'c', produces a Unicode character. For solving this issue, this patch fixes 2 bugs: First, KeyboardLayout::GetDeadKeyCombinations() doesn't add dead key entry if 2nd key is a dead key (::ToUnicodeEx() returns -1). In such case, it should add a dead key entry with the first character which is produced when only the 2nd key is pressed (the character is called as "base character" and used for index of the dead key table). Next, KeyboardLayout::InitNativeKey() should check if 2nd dead key press produces a composite character. If it's produced, it should initialize given NativeKey with the composite character. Otherwise, it should initialize with base characters of each key. This patch does it with KeyboardLayout::MaybeInitNativeKeyWithCompositeChar(). Finally, we should add automated test for this. However, unfortunately, it's not available on Win7 and our infra is still using Win7 for running automated tests. Therefore, this patch doesn't include new automated tests. MozReview-Commit-ID: G1DrfkHKNcK
widget/windows/KeyboardLayout.cpp
widget/windows/KeyboardLayout.h
--- a/widget/windows/KeyboardLayout.cpp
+++ b/widget/windows/KeyboardLayout.cpp
@@ -2842,22 +2842,21 @@ KeyboardLayout::InitNativeKey(NativeKey&
       UniCharsAndModifiers deadChars =
         mVirtualKeys[virtualKeyIndex].GetNativeUniChars(shiftState);
       NS_ASSERTION(deadChars.mLength == 1,
                    "dead key must generate only one character");
       aNativeKey.mKeyNameIndex = KEY_NAME_INDEX_Dead;
       return;
     }
 
-    // Dead key followed by another dead key causes inputting both character.
-    // However, at keydown message handling, we need to forget the first
-    // dead key because there is no guarantee coming WM_KEYUP for the second
-    // dead key before next WM_KEYDOWN.  E.g., due to auto key repeat or
-    // pressing another dead key before releasing current key.  Therefore,
-    // we can set only a character for current key for keyup event.
+    // At keydown message handling, we need to forget the first dead key
+    // because there is no guarantee coming WM_KEYUP for the second dead
+    // key before next WM_KEYDOWN.  E.g., due to auto key repeat or pressing
+    // another dead key before releasing current key.  Therefore, we can
+    // set only a character for current key for keyup event.
     if (mActiveDeadKey < 0) {
       aNativeKey.mCommittedCharsAndModifiers =
         mVirtualKeys[virtualKeyIndex].GetUniChars(shiftState);
       return;
     }
 
     int32_t activeDeadKeyIndex = GetKeyIndex(mActiveDeadKey);
     if (activeDeadKeyIndex < 0 || activeDeadKeyIndex >= NS_NUM_OF_KEYS) {
@@ -2869,66 +2868,112 @@ KeyboardLayout::InitNativeKey(NativeKey&
       NS_WARNING(warning.get());
 #ifdef MOZ_CRASHREPORTER
       CrashReporter::AppendAppNotesToCrashReport(
                        NS_LITERAL_CSTRING("\n") + warning);
 #endif // #ifdef MOZ_CRASHREPORTER
 #endif // #if defined(DEBUG) || defined(MOZ_CRASHREPORTER)
       MOZ_CRASH("Trying to reference out of range of mVirtualKeys");
     }
+
+    // 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)) {
+      return;
+    }
+
+    // Otherwise, dead key followed by another dead key causes inputting both
+    // character.
     UniCharsAndModifiers prevDeadChars =
       mVirtualKeys[activeDeadKeyIndex].GetUniChars(mDeadKeyShiftState);
     UniCharsAndModifiers newChars =
       mVirtualKeys[virtualKeyIndex].GetUniChars(shiftState);
     // But keypress events should be fired for each committed character.
     aNativeKey.mCommittedCharsAndModifiers = prevDeadChars + newChars;
     if (isKeyDown) {
       DeactivateDeadKeyState();
     }
     return;
   }
 
+  if (MaybeInitNativeKeyWithCompositeChar(aNativeKey, aModKeyState)) {
+    return;
+  }
+
   UniCharsAndModifiers baseChars =
     mVirtualKeys[virtualKeyIndex].GetUniChars(shiftState);
   if (mActiveDeadKey < 0) {
     // No dead-keys are active. Just return the produced characters.
     aNativeKey.mCommittedCharsAndModifiers = baseChars;
     return;
   }
 
-  // Dead-key was active. See if pressed base character does produce
-  // valid composite character.
   int32_t activeDeadKeyIndex = GetKeyIndex(mActiveDeadKey);
-  char16_t compositeChar = (baseChars.mLength == 1 && baseChars.mChars[0]) ?
-    mVirtualKeys[activeDeadKeyIndex].GetCompositeChar(mDeadKeyShiftState,
-                                                      baseChars.mChars[0]) : 0;
-  if (compositeChar) {
-    // Active dead-key and base character does produce exactly one
-    // composite character.
-    aNativeKey.mCommittedCharsAndModifiers.Append(compositeChar,
-                                                  baseChars.mModifiers[0]);
-    if (isKeyDown) {
-      DeactivateDeadKeyState();
-    }
+  if (NS_WARN_IF(activeDeadKeyIndex < 0)) {
     return;
   }
 
   // There is no valid dead-key and base character combination.
   // Return dead-key character followed by base character.
   UniCharsAndModifiers deadChars =
     mVirtualKeys[activeDeadKeyIndex].GetUniChars(mDeadKeyShiftState);
   // But keypress events should be fired for each committed character.
   aNativeKey.mCommittedCharsAndModifiers = deadChars + baseChars;
   if (isKeyDown) {
     DeactivateDeadKeyState();
   }
 
   return;
 }
 
+bool
+KeyboardLayout::MaybeInitNativeKeyWithCompositeChar(
+                  NativeKey& aNativeKey,
+                  const ModifierKeyState& aModKeyState)
+{
+  if (mActiveDeadKey < 0) {
+    return false;
+  }
+
+  int32_t activeDeadKeyIndex = GetKeyIndex(mActiveDeadKey);
+  if (NS_WARN_IF(activeDeadKeyIndex < 0)) {
+    return false;
+  }
+
+  int32_t virtualKeyIndex = GetKeyIndex(aNativeKey.mOriginalVirtualKeyCode);
+  if (NS_WARN_IF(virtualKeyIndex < 0)) {
+    return false;
+  }
+
+  uint8_t shiftState =
+    VirtualKey::ModifiersToShiftState(aModKeyState.GetModifiers());
+
+  UniCharsAndModifiers baseChars =
+    mVirtualKeys[virtualKeyIndex].GetUniChars(shiftState);
+  if (baseChars.IsEmpty() || !baseChars.mChars[0]) {
+    return false;
+  }
+
+  char16_t compositeChar =
+    mVirtualKeys[activeDeadKeyIndex].GetCompositeChar(mDeadKeyShiftState,
+                                                      baseChars.mChars[0]);
+  if (!compositeChar) {
+    return false;
+  }
+
+  // Active dead-key and base character does produce exactly one composite
+  // character.
+  aNativeKey.mCommittedCharsAndModifiers.Append(compositeChar,
+                                                baseChars.mModifiers[0]);
+  if (aNativeKey.IsKeyDownMessage()) {
+    DeactivateDeadKeyState();
+  }
+  return true;
+}
+
 UniCharsAndModifiers
 KeyboardLayout::GetUniCharsAndModifiers(
                   uint8_t aVirtualKey,
                   const ModifierKeyState& aModKeyState) const
 {
   UniCharsAndModifiers result;
   int32_t key = GetKeyIndex(aVirtualKey);
   if (key < 0) {
@@ -3266,29 +3311,68 @@ KeyboardLayout::GetDeadKeyCombinations(u
           ::ToUnicodeEx(virtualKey, 0, kbdState, (LPWSTR)compositeChars,
                         ArrayLength(compositeChars), 0, mKeyboardLayout);
         switch (ret) {
           case 0:
             // This key combination does not produce any characters. The
             // dead-key is still in active state.
             break;
           case 1: {
-            // Exactly one composite character produced. Now, when dead-key
-            // is not active, repeat the last character one more time to
-            // determine the base character.
             char16_t baseChars[5];
             ret = ::ToUnicodeEx(virtualKey, 0, kbdState, (LPWSTR)baseChars,
                                 ArrayLength(baseChars), 0, mKeyboardLayout);
-            NS_ASSERTION(ret == 1, "One base character expected");
-            if (ret == 1 && entries < aMaxEntries &&
-                AddDeadKeyEntry(baseChars[0], compositeChars[0],
-                                aDeadKeyArray, entries)) {
-              entries++;
+            if (entries < aMaxEntries) {
+              switch (ret) {
+                case 1:
+                  // Exactly one composite character produced. Now, when
+                  // dead-key is not active, repeat the last character one more
+                  // time to determine the base character.
+                  if (AddDeadKeyEntry(baseChars[0], compositeChars[0],
+                                      aDeadKeyArray, entries)) {
+                    entries++;
+                  }
+                  deadKeyActive = false;
+                  break;
+                case -1: {
+                  // If pressing another dead-key produces different character,
+                  // we should register the dead-key entry with first character
+                  // produced by current key.
+
+                  // First inactivate the dead-key state completely.
+                  deadKeyActive =
+                    EnsureDeadKeyActive(false, aDeadKey, aDeadKeyKbdState);
+                  if (NS_WARN_IF(deadKeyActive)) {
+                    MOZ_LOG(sKeyboardLayoutLogger, LogLevel::Error,
+                      ("  failed to deactivating the dead-key state..."));
+                    break;
+                  }
+                  for (int32_t i = 0; i < 5; ++i) {
+                    ret = ::ToUnicodeEx(virtualKey, 0, kbdState,
+                                        (LPWSTR)baseChars,
+                                        ArrayLength(baseChars),
+                                        0, mKeyboardLayout);
+                    if (ret >= 0) {
+                      break;
+                    }
+                  }
+                  if (ret > 0 &&
+                      AddDeadKeyEntry(baseChars[0], compositeChars[0],
+                                      aDeadKeyArray, entries)) {
+                    entries++;
+                  }
+                  // Inactivate dead-key state for current virtual keycode.
+                  EnsureDeadKeyActive(false, virtualKey, kbdState);
+                  break;
+                }
+                default:
+                  NS_WARN_IF("File a bug for this dead-key handling!");
+                  deadKeyActive = false;
+                  break;
+              }
             }
-            deadKeyActive = false;
             MOZ_LOG(sKeyboardLayoutLogger, LogLevel::Debug,
               ("  %s -> %s (%d): DeadKeyEntry(%s, %s) (ret=%d)",
                kVirtualKeyName[aDeadKey], kVirtualKeyName[virtualKey], vki,
                GetCharacterCodeName(compositeChars, 1).get(),
                ret <= 0 ? "''" :
                  GetCharacterCodeName(baseChars, std::min(ret, 5)).get(), ret));
             break;
           }
--- a/widget/windows/KeyboardLayout.h
+++ b/widget/windows/KeyboardLayout.h
@@ -600,16 +600,25 @@ public:
   /**
    * IsSysKey() returns true if aVirtualKey with aModKeyState causes WM_SYSKEY*
    * or WM_SYS*CHAR messages.
    */
   bool IsSysKey(uint8_t aVirtualKey,
                 const ModifierKeyState& aModKeyState) const;
 
   /**
+   * 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);
+
+  /**
    * GetUniCharsAndModifiers() returns characters which is inputted by the
    * aVirtualKey with aModKeyState.  This method isn't stateful.
    */
   UniCharsAndModifiers GetUniCharsAndModifiers(
                          uint8_t aVirtualKey,
                          const ModifierKeyState& aModKeyState) const;
 
   /**