Left align language/keyboard menus on network select screen.

 * removed delta_x/y from keyboard menu. this should use menu's offset, which takes care of rtl.

BUG=chromium-os:7965
TEST=manual: open lanugae/keyboard menus on network select screen. These menus should be left aligned. (or right aligned for RTL)

Review URL: http://codereview.chromium.org/3828012

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@63159 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 64 insertions, 52 deletions

diff --git a/chrome/browser/chromeos/login/keyboard_switch_menu.cc b/chrome/browser/chromeos/login/keyboard_switch_menu.cc
index de0884b..d4e55d7 100644
--- a/chrome/browser/chromeos/login/keyboard_switch_menu.cc
+++ b/chrome/browser/chromeos/login/keyboard_switch_menu.cc
@@ -5,6 +5,7 @@
 #include "chrome/browser/chromeos/login/keyboard_switch_menu.h"
 
 #include "app/l10n_util.h"
+#include "base/i18n/rtl.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/chromeos/cros/cros_library.h"
 #include "chrome/browser/chromeos/cros/keyboard_library.h"
@@ -17,9 +18,7 @@ namespace chromeos {
 KeyboardSwitchMenu::KeyboardSwitchMenu()
     : InputMethodMenu(NULL /* pref_service */,
                       false /* is_browser_mode */,
-                      false /* is_screen_locker */),
-      delta_x_(0),
-      delta_y_(0) {
+                      false /* is_screen_locker */) {
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -33,10 +32,18 @@ void KeyboardSwitchMenu::UpdateUI(
 
 ////////////////////////////////////////////////////////////////////////////////
 // views::ViewMenuDelegate implementation.
-void KeyboardSwitchMenu::RunMenu(views::View* source, const gfx::Point& point) {
-  gfx::Point offset_point(point);
-  offset_point.Offset(delta_x_, delta_y_);
-  InputMethodMenu::RunMenu(source, offset_point);
+void KeyboardSwitchMenu::RunMenu(views::View* source, const gfx::Point& pt) {
+  PrepareForMenuOpen();
+  gfx::Point new_pt(pt);
+  views::MenuButton* button = static_cast<views::MenuButton*>(source);
+  // Keyboard switch menu is aligned on left by default.
+  int reverse_offset = button->width() + button->menu_offset().x() * 2;
+  if (base::i18n::IsRTL()) {
+    new_pt.set_x(pt.x() + reverse_offset);
+  } else {
+    new_pt.set_x(pt.x() - reverse_offset);
+  }
+  language_menu().RunMenuAt(new_pt, views::Menu2::ALIGN_TOPLEFT);
 }
 
 std::wstring KeyboardSwitchMenu::GetCurrentKeyboardName() const {
diff --git a/chrome/browser/chromeos/login/keyboard_switch_menu.h b/chrome/browser/chromeos/login/keyboard_switch_menu.h
index e07af0b..64bd2aa 100644
--- a/chrome/browser/chromeos/login/keyboard_switch_menu.h
+++ b/chrome/browser/chromeos/login/keyboard_switch_menu.h
@@ -35,16 +35,7 @@ class KeyboardSwitchMenu : public InputMethodMenu {
   // Returns current keyboard name to be placed on the keyboard menu-button.
   std::wstring GetCurrentKeyboardName() const;
 
-  // Changes the position of the dropdown menu.
-  void SetMenuOffset(int delta_x, int delta_y) {
-    delta_x_ = delta_x;
-    delta_y_ = delta_y;
-  }
-
  private:
-  int delta_x_;
-  int delta_y_;
-
   DISALLOW_COPY_AND_ASSIGN(KeyboardSwitchMenu);
 };
 
diff --git a/chrome/browser/chromeos/login/language_switch_menu.cc b/chrome/browser/chromeos/login/language_switch_menu.cc
index 92e4750..3d8bcbc 100644
--- a/chrome/browser/chromeos/login/language_switch_menu.cc
+++ b/chrome/browser/chromeos/login/language_switch_menu.cc
@@ -5,6 +5,7 @@
 #include "chrome/browser/chromeos/login/language_switch_menu.h"
 
 #include "app/resource_bundle.h"
+#include "base/i18n/rtl.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chromeos/cros/cros_library.h"
@@ -15,6 +16,7 @@
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/common/pref_names.h"
 #include "grit/generated_resources.h"
+#include "views/controls/button/menu_button.h"
 #include "views/widget/widget_gtk.h"
 
 namespace {
@@ -30,7 +32,8 @@ namespace chromeos {
 
 LanguageSwitchMenu::LanguageSwitchMenu()
     : ALLOW_THIS_IN_INITIALIZER_LIST(menu_model_(this)),
-      ALLOW_THIS_IN_INITIALIZER_LIST(menu_model_submenu_(this)) {
+      ALLOW_THIS_IN_INITIALIZER_LIST(menu_model_submenu_(this)),
+      menu_alignment_(views::Menu2::ALIGN_TOPRIGHT) {
 }
 
 void LanguageSwitchMenu::InitLanguageMenu() {
@@ -106,7 +109,18 @@ void LanguageSwitchMenu::SwitchLanguage(const std::string& locale) {
 
 void LanguageSwitchMenu::RunMenu(views::View* source, const gfx::Point& pt) {
   DCHECK(menu_ != NULL);
-  menu_->RunMenuAt(pt, views::Menu2::ALIGN_TOPRIGHT);
+  views::MenuButton* button = static_cast<views::MenuButton*>(source);
+  // We align the on left edge of the button for non RTL case.
+  gfx::Point new_pt(pt);
+  if (menu_alignment_ == views::Menu2::ALIGN_TOPLEFT) {
+    int reverse_offset = button->width() + button->menu_offset().x() * 2;
+    if (base::i18n::IsRTL()) {
+      new_pt.set_x(pt.x() + reverse_offset);
+    } else {
+      new_pt.set_x(pt.x() - reverse_offset);
+    }
+  }
+  menu_->RunMenuAt(new_pt, menu_alignment_);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
diff --git a/chrome/browser/chromeos/login/language_switch_menu.h b/chrome/browser/chromeos/login/language_switch_menu.h
index 33d34af..cbb562d 100644
--- a/chrome/browser/chromeos/login/language_switch_menu.h
+++ b/chrome/browser/chromeos/login/language_switch_menu.h
@@ -30,6 +30,11 @@ class LanguageSwitchMenu : public views::ViewMenuDelegate,
   // Initializes language selection menu contents.
   void InitLanguageMenu();
 
+  // Sets menu's alignment.
+  void set_menu_alignment(views::Menu2::Alignment alignment) {
+    menu_alignment_ = alignment;
+  }
+
   // Returns current locale name to be placed on the language menu-button.
   std::wstring GetCurrentLocaleName() const;
 
@@ -58,6 +63,9 @@ class LanguageSwitchMenu : public views::ViewMenuDelegate,
   // Language locale name storage.
   scoped_ptr<LanguageList> language_list_;
 
+  // Menu alignment.
+  views::Menu2::Alignment menu_alignment_;
+
   FRIEND_TEST(::WizardControllerTest, SwitchLanguage);
   DISALLOW_COPY_AND_ASSIGN(LanguageSwitchMenu);
 };
diff --git a/chrome/browser/chromeos/login/network_screen.cc b/chrome/browser/chromeos/login/network_screen.cc
index 4f17870..2da1197 100644
--- a/chrome/browser/chromeos/login/network_screen.cc
+++ b/chrome/browser/chromeos/login/network_screen.cc
@@ -16,6 +16,7 @@
 #include "grit/chromium_strings.h"
 #include "grit/generated_resources.h"
 #include "grit/theme_resources.h"
+#include "views/controls/menu/menu_2.h"
 #include "views/widget/widget.h"
 #include "views/window/window.h"
 
@@ -43,6 +44,7 @@ NetworkScreen::NetworkScreen(WizardScreenDelegate* delegate)
       is_network_subscribed_(false),
       continue_pressed_(false),
       bubble_(NULL) {
+  language_switch_menu_.set_menu_alignment(views::Menu2::ALIGN_TOPLEFT);
 }
 
 NetworkScreen::~NetworkScreen() {
diff --git a/chrome/browser/chromeos/login/network_selection_view.cc b/chrome/browser/chromeos/login/network_selection_view.cc
index b8d3ba0..62ca881 100644
--- a/chrome/browser/chromeos/login/network_selection_view.cc
+++ b/chrome/browser/chromeos/login/network_selection_view.cc
@@ -307,8 +307,8 @@ void NetworkSelectionView::Init() {
   keyboards_menubutton_->SetFocusable(true);
   keyboards_menubutton_->SetNormalHasBorder(true);
   keyboards_menubutton_->set_animate_on_state_change(false);
-  delegate_->keyboard_switch_menu()->SetMenuOffset(kMenuHorizontalOffset,
-                                                   kMenuVerticalOffset);
+  keyboards_menubutton_->set_menu_offset(kMenuHorizontalOffset,
+                                         kMenuVerticalOffset);
 
   select_network_label_ = new views::Label();
   select_network_label_->SetFont(rb.GetFont(ResourceBundle::MediumFont));
diff --git a/chrome/browser/chromeos/status/input_method_menu.cc b/chrome/browser/chromeos/status/input_method_menu.cc
index 779c767..c94e125 100644
--- a/chrome/browser/chromeos/status/input_method_menu.cc
+++ b/chrome/browser/chromeos/status/input_method_menu.cc
@@ -6,8 +6,6 @@
 
 #include <string>
 
-#include <gtk/gtk.h>  // for gtk_menu_shell_set_take_focus.
-
 #include "app/l10n_util.h"
 #include "app/resource_bundle.h"
 #include "base/string_split.h"
@@ -368,35 +366,7 @@ void InputMethodMenu::ActivatedAt(int index) {
 
 void InputMethodMenu::RunMenu(
     views::View* unused_source, const gfx::Point& pt) {
-  UserMetrics::RecordAction(UserMetricsAction("LanguageMenuButton_Open"));
-  input_method_descriptors_.reset(CrosLibrary::Get()->GetInputMethodLibrary()->
-                                  GetActiveInputMethods());
-  RebuildModel();
-  language_menu_.Rebuild();
-
-  // Disallow the menu widget to grab the keyboard focus. This is necessary to
-  // enable users to change status of an input method (e.g. change the input
-  // mode from Japanese Hiragana to Japanese Katakana) without discarding a
-  // preedit string. See crosbug.com/5796 for details. Note that menus other
-  // than this one should not call the Gtk+ API since it is a special API only
-  // for a menu related to IME/keyboard. See the Gtk+ API reference at:
-  // http://library.gnome.org/devel/gtk/stable/GtkMenuShell.html
-/*
-  Temporarily disable due to crash. (http://crosbug.com/7598)
-
-  gfx::NativeMenu native_menu = language_menu_.GetNativeMenu();
-  if (native_menu) {
-    gtk_menu_shell_set_take_focus(GTK_MENU_SHELL(native_menu), FALSE);
-  } else {
-    LOG(ERROR)
-        << "Can't call gtk_menu_shell_set_take_focus since NativeMenu is NULL";
-  }
-*/
-
-  language_menu_.UpdateStates();
-  if (minimum_language_menu_width_ > 0) {
-    language_menu_.SetMinimumWidth(minimum_language_menu_width_);
-  }
+  PrepareForMenuOpen();
   language_menu_.RunMenuAt(pt, views::Menu2::ALIGN_TOPRIGHT);
 }
 
@@ -432,6 +402,17 @@ void InputMethodMenu::InputMethodChanged(InputMethodLibrary* obj) {
   }
 }
 
+void InputMethodMenu::PrepareForMenuOpen() {
+  UserMetrics::RecordAction(UserMetricsAction("LanguageMenuButton_Open"));
+  input_method_descriptors_.reset(CrosLibrary::Get()->GetInputMethodLibrary()->
+                                  GetActiveInputMethods());
+  RebuildModel();
+  language_menu_.Rebuild();
+  if (minimum_language_menu_width_ > 0) {
+    language_menu_.SetMinimumWidth(minimum_language_menu_width_);
+  }
+}
+
 void InputMethodMenu::ActiveInputMethodsChanged(InputMethodLibrary* obj) {
   // Update the icon if active input methods are changed. See also
   // comments in UpdateUI()
diff --git a/chrome/browser/chromeos/status/input_method_menu.h b/chrome/browser/chromeos/status/input_method_menu.h
index c91d885..fb87f16 100644
--- a/chrome/browser/chromeos/status/input_method_menu.h
+++ b/chrome/browser/chromeos/status/input_method_menu.h
@@ -56,7 +56,8 @@ class InputMethodMenu : public views::ViewMenuDelegate,
 
   // views::ViewMenuDelegate implementation. Sub classes can override the method
   // to adjust the position of the menu.
-  virtual void RunMenu(views::View* unused_source, const gfx::Point& pt);
+  virtual void RunMenu(views::View* unused_source,
+                       const gfx::Point& pt);
 
   // InputMethodLibrary::Observer implementation.
   virtual void InputMethodChanged(InputMethodLibrary* obj);
@@ -87,6 +88,14 @@ class InputMethodMenu : public views::ViewMenuDelegate,
   // Parses |input_method| and then calls UpdateUI().
   void UpdateUIFromInputMethod(const InputMethodDescriptor& input_method);
 
+  // Rebuilds model and menu2 objects in preparetion to open the menu.
+  void PrepareForMenuOpen();
+
+  // Returns menu2 object for language menu.
+  views::Menu2& language_menu() {
+    return language_menu_;
+  }
+
  private:
   // Updates UI of a container of the menu (e.g. the "US" menu button in the
   // status area). Sub classes have to implement the interface for their own UI.