Fixing french keyboard handling

chrome/browser/ui/views/ash/ime_controller_chromeos.cc

Index: chrome/browser/ui/views/ash/ime_controller_chromeos.cc
diff --git a/chrome/browser/ui/views/ash/ime_controller_chromeos.cc b/chrome/browser/ui/views/ash/ime_controller_chromeos.cc
index 1934357e90140bc62cc7e8ea681e6ab6cc68b498..352851d8f6ce7dc7f40d2f5bb23bc05d73b801a8 100644
--- a/chrome/browser/ui/views/ash/ime_controller_chromeos.cc
+++ b/chrome/browser/ui/views/ash/ime_controller_chromeos.cc
@@ -4,8 +4,13 @@
 
 #include "chrome/browser/ui/views/ash/ime_controller_chromeos.h"
 
+#include "base/string_util.h"
 #include "chrome/browser/chromeos/input_method/input_method_manager.h"
 
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/prefs/pref_service.h"
+#include "chrome/browser/chromeos/language_preferences.h"
+
 bool ImeController::HandleNextIme() {
   chromeos::input_method::InputMethodManager* manager =
       chromeos::input_method::InputMethodManager::GetInstance();
@@ -23,3 +28,18 @@ bool ImeController::HandleSwitchIme(const ui::Accelerator& accelerator) {
       chromeos::input_method::InputMethodManager::GetInstance();
   return manager->SwitchInputMethod(accelerator);
 }
+
+bool ImeController::IsFrenchKeyboard() {
+  // First check the settings.
+  std::string layout = g_browser_process->local_state()->GetString(
+      chromeos::language_prefs::kPreferredKeyboardLayout);

[Yusuke Sato, 2012/05/29 08:29:11]

please remove line 33-36. the pref is for login screen, and therefore does not
represent the current keyboard layout.

[Mr4D (OOO till 08-26), 2012/05/29 14:59:25]

I don't think so. When a user overwrites the keyboard properties of the (Chrome
OS) settings, this value will represent the chosen keyboard layout while the
hardware layout remains the original layout. Furthermore - there might be an
external keyboard which could then be french. So we need to read first the user
preference and then the real hardware layout.

[Yusuke Sato, 2012/05/29 16:13:38]

In short: the return value of GetCurrentInputMethod() should be the "user
preference".

First, InputMethodManager::GetCurrentInputMethod() does not return the hardware
keyboard layout actually. It just returns a layout which is currently selected
(i.e. the one shown in the status area at the bottom right corner of the
screen). I think you could easily confirm the behavior by enabling the "US
Dvorak" layout and disabling all other layouts via chrome://settings/. Then the
function would return "xkb:us:dvorak:eng" regardless of the actual hardware
keyboard layout.

If you really need to get hardware information, you should call
InputMethodUtil::GetHardwareInputMethodId() which retrieves the information from
VPD (the firmware). The function would return 'xkb:us::eng' if you're using a
Chromebook with a US Qwerty hardware keyboard (i.e. the one sold in the US), and
return 'xkb:fr::fra' for a Chromebook with a French Azerty hardware keyboard
(i.e. the one sold in France). However, I don't think calling the function is
necessary here.

What we have to do here would be to know the layout currently in use. For that
purpose, we can just use InputMethodManager::GetCurrentInputMethod(). That
function returns 'xkb:us::eng' when the status area shows "US", and
'xkb:fr::fra' when it's "FR".

The kPreferredKeyboardLayout pref wouldn't work here since the preference is
just for remembering the preferred keyboard layout for the login screen. The
pref is updated every time when you change the keyboard layout on the login
screen, but isn't updated once you sign in. For example, if the preference is
set to 'xkb:fr::fra', the French Azerty keyboard layout is used on the login
screen when you boot your Chromebook.

Assuming that you use either the US Qwerty ("US") or French Azerty ("FR")
keyboard layout on the login screen, please try the following:

1. Confirm either "US" or "FR" is shown in the status area on the login screen.
2. Sign in.
3. Enable US Dvorak and US Colemak keyboard layouts. Disable others.
4. Press Shift+Alt several times. DV and CO are shown in the status area
alternatively.
5. Move to the shell (via crosh or by pressing Ctrl+Alt+F2), and grep
/home/chronos/Local\ State for 'xkb:'.

Then you'll see the preferred keyboard layout pref is still set to US Qwerty
(xkb:us::eng) or French Azerty (xkb:fr::fra) depending on the layout you used on
the login screen last time, and it isn't set to the current layout, US Dvorak
(xkb:us:dvorak:eng) or US Colemak (xkb:us:colemak:eng).

-Yusuke

On 2012/05/29 14:59:25, Mr4D wrote:

[Mr4D (OOO till 08-26), 2012/05/29 21:40:36]

This is now really weird: I have tried this before and it did not work. However
- now I used LOG statements and everything seems to work as described. Changed,
Done.

+  if (layout == "") {
+    // If it is unset we check the hardware.
+    chromeos::input_method::InputMethodManager* manager =
+        chromeos::input_method::InputMethodManager::GetInstance();
+    const chromeos::input_method::InputMethodDescriptor& descriptor =
+        manager->GetCurrentInputMethod();
+    layout = descriptor.id();
+  }
+  return EndsWith(layout, ":fra", false);

[Yusuke Sato, 2012/05/29 08:29:11]

This matches the following 4 layouts: xkb:fr::fra, xkb:be::fra, xkb:ca::fra, and
xkb:ch:fr:fra. However, only the first two layouts require Shift to input
numbers. Please use exact matching:

return (layout == "xkb:fr::fra") and (layout == "xkb:be::fra");

[Mr4D (OOO till 08-26), 2012/05/29 14:59:25]

Done. I had no idea that there are even different French keyboard layouts. I
wanted to catch then all...
Good find!

+}