Disable the deprecated Alt+Shift for NEXT_IME

ash/accelerators/accelerator_controller.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ash/accelerators/accelerator_controller.h"
 
 #include <algorithm>
 #include <cmath>
 #include <string>
 #include <utility>
@@ skipping 282 matching lines @@
     base::RecordAction(base::UserMetricsAction("Accel_NewTab_T"));
   Shell::GetInstance()->new_window_delegate()->NewTab();
 }
 
 void HandleNewWindow() {
   base::RecordAction(base::UserMetricsAction("Accel_New_Window"));
   Shell::GetInstance()->new_window_delegate()->NewWindow(
       false /* is_incognito */);
 }
 
-bool CanHandleNextIme(ImeControlDelegate* ime_control_delegate,
-                      const ui::Accelerator& previous_accelerator,
-                      bool current_accelerator_is_deprecated) {
-  if (current_accelerator_is_deprecated) {
-    // We only allow next IME to be triggered if the previous is accelerator key
-    // is ONLY either Shift, Alt, Enter or Space.
-    // The first six cases below are to avoid conflicting accelerators that
-    // contain Alt+Shift (like Alt+Shift+Tab or Alt+Shift+S) to trigger next IME
-    // when the wrong order of key sequences is pressed. crbug.com/527154.
-    // The latter two cases are needed for CJK IME users who tend to press Enter
-    // (or Space) and Shift+Alt almost at the same time to commit an IME string
-    // and then switch from the IME to the English layout. This allows these
-    // users to trigger NEXT_IME even if they press Shift+Alt before releasing
-    // Enter. crbug.com/139556.
-    // TODO(nona|mazda): Fix crbug.com/139556 in a cleaner way.
-    const ui::KeyboardCode previous_key_code = previous_accelerator.key_code();
-    switch (previous_key_code) {
-      case ui::VKEY_SHIFT:
-      case ui::VKEY_LSHIFT:
-      case ui::VKEY_RSHIFT:
-      case ui::VKEY_MENU:
-      case ui::VKEY_LMENU:
-      case ui::VKEY_RMENU:
-      case ui::VKEY_RETURN:
-      case ui::VKEY_SPACE:
-        break;
+bool CanHandleNextIme(ImeControlDelegate* ime_control_delegate) {
+  return ime_control_delegate && ime_control_delegate->CanCycleIme();
+}
 
-      default:
-        return false;
-    }
+// We must avoid showing the Deprecated NEXT_IME notification erronously.
+bool ShouldShowDeprecatedNextImeNotification(
+    const ui::Accelerator& previous_accelerator) {
+  // We only show the deprecation notification if the previous accelerator key
+  // is ONLY either Shift, or Alt.
+  const ui::KeyboardCode previous_key_code = previous_accelerator.key_code();
+  switch (previous_key_code) {
+    case ui::VKEY_SHIFT:
+    case ui::VKEY_LSHIFT:
+    case ui::VKEY_RSHIFT:
+    case ui::VKEY_MENU:
+    case ui::VKEY_LMENU:
+    case ui::VKEY_RMENU:
+      return true;
+
+    default:
+      return false;
   }
-
-  return ime_control_delegate && ime_control_delegate->CanCycleIme();
 }
 
 void HandleNextIme(ImeControlDelegate* ime_control_delegate) {
   base::RecordAction(UserMetricsAction("Accel_Next_Ime"));
   ime_control_delegate->HandleNextIme();
 }
 
 void HandleOpenFeedbackPage() {
   base::RecordAction(UserMetricsAction("Accel_Open_Feedback_Page"));
   ash::Shell::GetInstance()->new_window_delegate()->OpenFeedbackPage();
@@ skipping 525 matching lines @@
     auto itr = actions_with_deprecations_.find(action);
     if (itr != actions_with_deprecations_.end()) {
       const DeprecatedAcceleratorData* data = itr->second;
       if (deprecated_accelerators_.count(accelerator)) {
         // This accelerator has been deprecated and should be treated according
         // to its |DeprecatedAcceleratorData|.
 
         // Record UMA stats.
         RecordUmaHistogram(data->uma_histogram_name, DEPRECATED_USED);
 
-        // We always display the notification as long as this entry exists.
-        ShowDeprecatedAcceleratorNotification(
-            data->uma_histogram_name, data->notification_message_id,
-            data->old_shortcut_id, data->new_shortcut_id);
+        // We always display the notification as long as this entry exists,
+        // except for NEXT_IME, we must check to avoid showing it for the wrong
+        // shortcut, as Alt+Shift is tricky and trigger the action on release.
+        if (action != NEXT_IME ||
+            (action == NEXT_IME &&
+             ShouldShowDeprecatedNextImeNotification(
+                 accelerator_history_->previous_accelerator()))) {
+          ShowDeprecatedAcceleratorNotification(
+              data->uma_histogram_name, data->notification_message_id,
+              data->old_shortcut_id, data->new_shortcut_id);
+        }
 
         if (!data->deprecated_enabled)
           return false;
       } else {
         // This is a new accelerator replacing the old deprecated one.
         // Record UMA stats and proceed normally.
         RecordUmaHistogram(data->uma_histogram_name, NEW_USED);
       }
     }
 
@@ skipping 112 matching lines @@
     case DEBUG_TOGGLE_ROOT_WINDOW_FULL_SCREEN:
     case DEBUG_TOGGLE_SHOW_DEBUG_BORDERS:
     case DEBUG_TOGGLE_SHOW_FPS_COUNTER:
     case DEBUG_TOGGLE_SHOW_PAINT_RECTS:
       return debug::DebugAcceleratorsEnabled();
     case MAGNIFY_SCREEN_ZOOM_IN:
     case MAGNIFY_SCREEN_ZOOM_OUT:
       return CanHandleMagnifyScreen();
     case NEW_INCOGNITO_WINDOW:
       return CanHandleNewIncognitoWindow();
-    case NEXT_IME: {
-#if defined(OS_CHROMEOS)
-      bool accelerator_is_deprecated =
-          (deprecated_accelerators_.count(accelerator) != 0);
-#else
-      // On non-chromeos, the NEXT_IME deprecated accelerators are always used.
-      bool accelerator_is_deprecated = true;
-#endif  // defined(OS_CHROMEOS)
-
-      return CanHandleNextIme(ime_control_delegate_.get(), previous_accelerator,
-                              accelerator_is_deprecated);
-    }
+    case NEXT_IME:
+      return CanHandleNextIme(ime_control_delegate_.get());
     case PREVIOUS_IME:
       return CanHandlePreviousIme(ime_control_delegate_.get());
     case SCALE_UI_RESET:
     case SCALE_UI_UP:
     case SCALE_UI_DOWN:
       return accelerators::IsInternalDisplayZoomEnabled();
     case SHOW_MESSAGE_CENTER_BUBBLE:
       return CanHandleShowMessageCenterBubble();
     case SWITCH_IME:
       return CanHandleSwitchIme(ime_control_delegate_.get(), accelerator);
@@ skipping 412 matching lines @@
 }
 
 void AcceleratorController::SetKeyboardBrightnessControlDelegate(
     std::unique_ptr<KeyboardBrightnessControlDelegate>
         keyboard_brightness_control_delegate) {
   keyboard_brightness_control_delegate_ =
       std::move(keyboard_brightness_control_delegate);
 }
 
 }  // namespace ash