focus reverse traversal was not working for TextfieldViews.

 Both Textfield and TextfieldViews were focusable and
alt-tab was setting the focus back to the Textfield, which
sets the focus to the its TextfieldViews. This fixes the issue
by making Textfield non focusable.

BUG=none
TEST=FocusTest is added to unittest.

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

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

views/controls/textfield/native_textfield_gtk.cc

@@ -257,10 +257,6 @@ void NativeTextfieldGtk::UpdateEnabled() {
   SetEnabled(textfield_->IsEnabled());
 }
 
-bool NativeTextfieldGtk::IsPassword() {
-  return textfield_->IsPassword();
-}
-
 gfx::Insets NativeTextfieldGtk::CalculateInsets() {
   if (!native_view())
     return gfx::Insets();
@@ -329,8 +325,9 @@ void NativeTextfieldGtk::UpdateVerticalMargins() {
   }
 }
 
-void NativeTextfieldGtk::SetFocus() {
+bool NativeTextfieldGtk::SetFocus() {
   Focus();
+  return true;
 }
 
 View* NativeTextfieldGtk::GetView() {
@@ -345,6 +342,23 @@ bool NativeTextfieldGtk::IsIMEComposing() const {
   return false;
 }
 
+bool NativeTextfieldGtk::HandleKeyPressed(const views::KeyEvent& e) {
+  return false;
+}
+
+bool NativeTextfieldGtk::HandleKeyReleased(const views::KeyEvent& e) {
+  return false;
+}
+
+void NativeTextfieldGtk::HandleWillGainFocus() {
+}
+
+void NativeTextfieldGtk::HandleDidGainFocus() {
+}
+
+void NativeTextfieldGtk::HandleWillLoseFocus() {
+}
+
 // static
 gboolean NativeTextfieldGtk::OnKeyPressEventHandler(
     GtkWidget* widget,
@@ -444,4 +458,8 @@ void NativeTextfieldGtk::NativeControlCreated(GtkWidget* widget) {
   g_signal_connect(widget, "activate", G_CALLBACK(OnActivateHandler), this);
 }
 
+bool NativeTextfieldGtk::IsPassword() {
+  return textfield_->IsPassword();
+}
+
 }  // namespace views

views/controls/textfield/native_textfield_gtk.h

@@ -41,19 +41,26 @@ class NativeTextfieldGtk : public NativeControlGtk,
   virtual void UpdateFont();
   virtual void UpdateIsPassword();
   virtual void UpdateEnabled();
-  virtual bool IsPassword();
   virtual gfx::Insets CalculateInsets();
   virtual void UpdateHorizontalMargins();
   virtual void UpdateVerticalMargins();
-  virtual void SetFocus();
+  virtual bool SetFocus();
   virtual View* GetView();
   virtual gfx::NativeView GetTestingHandle() const;
   virtual bool IsIMEComposing() const;
+  virtual bool HandleKeyPressed(const views::KeyEvent& e);
+  virtual bool HandleKeyReleased(const views::KeyEvent& e);
+  virtual void HandleWillGainFocus();
+  virtual void HandleDidGainFocus();
+  virtual void HandleWillLoseFocus();
 
   // Overridden from NativeControlGtk:
   virtual void CreateNativeControl();
   virtual void NativeControlCreated(GtkWidget* widget);
 
+  // Returns true if the textfield is for password.
+  bool IsPassword();
+
  private:
   Textfield* textfield_;
 

views/controls/textfield/native_textfield_views.cc

@@ -57,7 +57,6 @@ NativeTextfieldViews::NativeTextfieldViews(Textfield* parent)
       insert_(true),
       is_cursor_visible_(false),
       ALLOW_THIS_IN_INITIALIZER_LIST(cursor_timer_(this)) {
-  SetFocusable(true);
   set_border(text_border_);
 
   // Multiline is not supported.
@@ -73,7 +72,7 @@ NativeTextfieldViews::~NativeTextfieldViews() {
 // NativeTextfieldViews, View overrides:
 
 bool NativeTextfieldViews::OnMousePressed(const views::MouseEvent& e) {
-  RequestFocus();
+  textfield_->RequestFocus();
   size_t pos = FindCursorPosition(e.location());
   if (model_->MoveCursorTo(pos, false)) {
     UpdateCursorBoundsAndTextOffset();
@@ -96,50 +95,41 @@ void NativeTextfieldViews::OnMouseReleased(const views::MouseEvent& e,
 }
 
 bool NativeTextfieldViews::OnKeyPressed(const views::KeyEvent& e) {
-  Textfield::Controller* controller = textfield_->GetController();
-  bool handled = false;
-  if (controller)
-    handled = controller->HandleKeyEvent(textfield_, e);
-  return handled || HandleKeyEvent(e);
+  // OnKeyPressed/OnKeyReleased/WillGainFocus/DidGainFocus/WillLoseFocus
+  // will never be invoked on NativeTextfieldViews as it will never
+  // gain focus.
+  NOTREACHED();
+  return false;
 }
 
 bool NativeTextfieldViews::OnKeyReleased(const views::KeyEvent& e) {
-  return true;
+  NOTREACHED();
+  return false;
 }
 
 void NativeTextfieldViews::Paint(gfx::Canvas* canvas) {
-  text_border_->set_has_focus(HasFocus());
+  text_border_->set_has_focus(textfield_->HasFocus());
   PaintBackground(canvas);
   PaintTextAndCursor(canvas);
   if (textfield_->draw_border())
     PaintBorder(canvas);
 }
 
+void NativeTextfieldViews::DidChangeBounds(const gfx::Rect& previous,
+                                           const gfx::Rect& current) {
+  UpdateCursorBoundsAndTextOffset();
+}
+
 void NativeTextfieldViews::WillGainFocus() {
+  NOTREACHED();
 }
 
 void NativeTextfieldViews::DidGainFocus() {
-  is_cursor_visible_ = true;
-  SchedulePaint();
-  // Start blinking cursor.
-  MessageLoop::current()->PostDelayedTask(
-      FROM_HERE,
-      cursor_timer_.NewRunnableMethod(&NativeTextfieldViews::UpdateCursor),
-      kCursorVisibleTimeMs);
+  NOTREACHED();
 }
 
 void NativeTextfieldViews::WillLoseFocus() {
-  // Stop blinking cursor.
-  cursor_timer_.RevokeAll();
-  if (is_cursor_visible_) {
-    is_cursor_visible_ = false;
-    RepaintCursor();
-  }
-}
-
-void NativeTextfieldViews::DidChangeBounds(const gfx::Rect& previous,
-                                          const gfx::Rect& current) {
-  UpdateCursorBoundsAndTextOffset();
+  NOTREACHED();
 }
 
 /////////////////////////////////////////////////////////////////
@@ -223,15 +213,10 @@ void NativeTextfieldViews::UpdateIsPassword() {
 }
 
 void NativeTextfieldViews::UpdateEnabled() {
+  SetEnabled(textfield_->IsEnabled());
   SchedulePaint();
 }
 
-bool NativeTextfieldViews::IsPassword() {
-  // looks unnecessary. should we remove?
-  NOTREACHED();
-  return false;
-}
-
 gfx::Insets NativeTextfieldViews::CalculateInsets() {
   return GetInsets();
 }
@@ -256,8 +241,8 @@ void NativeTextfieldViews::UpdateVerticalMargins() {
   UpdateCursorBoundsAndTextOffset();
 }
 
-void NativeTextfieldViews::SetFocus() {
-  RequestFocus();
+bool NativeTextfieldViews::SetFocus() {
+  return false;
 }
 
 View* NativeTextfieldViews::GetView() {
@@ -273,6 +258,41 @@ bool NativeTextfieldViews::IsIMEComposing() const {
   return false;
 }
 
+bool NativeTextfieldViews::HandleKeyPressed(const views::KeyEvent& e) {
+  Textfield::Controller* controller = textfield_->GetController();
+  bool handled = false;
+  if (controller) {
+    handled = controller->HandleKeyEvent(textfield_, e);
+  }
+  return handled || HandleKeyEvent(e);
+}
+
+bool NativeTextfieldViews::HandleKeyReleased(const views::KeyEvent& e) {
+  return true;
+}
+
+void NativeTextfieldViews::HandleWillGainFocus() {
+}
+
+void NativeTextfieldViews::HandleDidGainFocus() {
+  is_cursor_visible_ = true;
+  SchedulePaint();
+  // Start blinking cursor.
+  MessageLoop::current()->PostDelayedTask(
+      FROM_HERE,
+      cursor_timer_.NewRunnableMethod(&NativeTextfieldViews::UpdateCursor),
+      kCursorVisibleTimeMs);
+}
+
+void NativeTextfieldViews::HandleWillLoseFocus() {
+  // Stop blinking cursor.
+  cursor_timer_.RevokeAll();
+  if (is_cursor_visible_) {
+    is_cursor_visible_ = false;
+    RepaintCursor();
+  }
+}
+
 // static
 bool NativeTextfieldViews::IsTextfieldViewsEnabled() {
 #if defined(TOUCH_UI)
@@ -368,7 +388,8 @@ void NativeTextfieldViews::PaintTextAndCursor(gfx::Canvas* canvas) {
   int y = insets.top();
   int text_height = height() - insets.height();
   SkColor selection_color =
-      HasFocus() ? kFocusedSelectionColor : kUnfocusedSelectionColor;
+      textfield_->HasFocus() ?
+      kFocusedSelectionColor : kUnfocusedSelectionColor;
   SkColor text_color =
       textfield_->read_only() ? kReadonlyTextColor : GetTextColor();
 

views/controls/textfield/native_textfield_views.h

@@ -46,11 +46,11 @@ class NativeTextfieldViews : public views::View,
   virtual bool OnKeyPressed(const views::KeyEvent& e);
   virtual bool OnKeyReleased(const views::KeyEvent& e);
   virtual void Paint(gfx::Canvas* canvas);
+  virtual void DidChangeBounds(const gfx::Rect& previous,
+                               const gfx::Rect& current);
   virtual void WillGainFocus();
   virtual void DidGainFocus();
   virtual void WillLoseFocus();
-  virtual void DidChangeBounds(const gfx::Rect& previous,
-                               const gfx::Rect& current);
 
   // NativeTextfieldWrapper overrides:
   virtual string16 GetText() const;
@@ -66,14 +66,18 @@ class NativeTextfieldViews : public views::View,
   virtual void UpdateFont();
   virtual void UpdateIsPassword();
   virtual void UpdateEnabled();
-  virtual bool IsPassword();
   virtual gfx::Insets CalculateInsets();
   virtual void UpdateHorizontalMargins();
   virtual void UpdateVerticalMargins();
-  virtual void SetFocus();
+  virtual bool SetFocus();
   virtual View* GetView();
   virtual gfx::NativeView GetTestingHandle() const;
   virtual bool IsIMEComposing() const;
+  virtual bool HandleKeyPressed(const views::KeyEvent& e);
+  virtual bool HandleKeyReleased(const views::KeyEvent& e);
+  virtual void HandleWillGainFocus();
+  virtual void HandleDidGainFocus();
+  virtual void HandleWillLoseFocus();
 
   // class name of internal
   static const char kViewClassName[];

views/controls/textfield/native_textfield_views_unittest.cc

@@ -10,6 +10,7 @@
 #include "views/controls/textfield/textfield.h"
 #include "views/controls/textfield/textfield_views_model.h"
 #include "views/event.h"
+#include "views/focus/focus_manager.h"
 #include "views/widget/widget.h"
 
 namespace views {
@@ -56,6 +57,10 @@ class NativeTextfieldViewsTest : public ::testing::Test,
   }
 
   void InitTextfield(Textfield::StyleFlags style) {
+    InitTextfields(style, 1);
+  }
+
+  void InitTextfields(Textfield::StyleFlags style, int count) {
     ASSERT_FALSE(textfield_);
     textfield_ = new Textfield(style);
     textfield_->SetController(this);
@@ -65,13 +70,25 @@ class NativeTextfieldViewsTest : public ::testing::Test,
         Widget::DeleteOnDestroy,
         Widget::DontMirrorOriginInRTL);
     widget_->Init(NULL, gfx::Rect());
-    widget_->SetContentsView(textfield_);
+
+    View* container = new View();
+    widget_->SetContentsView(container);
+    container->AddChildView(textfield_);
     textfield_view_
         = static_cast<NativeTextfieldViews*>(textfield_->native_wrapper());
+    textfield_->SetID(1);
+
+    for (int i = 1; i < count; i++) {
+      Textfield* textfield = new Textfield(style);
+      container->AddChildView(textfield);
+      textfield->SetID(i + 1);
+    }
+
     DCHECK(textfield_view_);
     model_ = textfield_view_->model_.get();
   }
 
+ protected:
   bool SendKeyEventToTextfieldViews(app::KeyboardCode key_code,
                                     bool shift,
                                     bool control,
@@ -80,7 +97,7 @@ class NativeTextfieldViewsTest : public ::testing::Test,
         (control ? KeyEvent::EF_CONTROL_DOWN : 0) |
         (capslock ? KeyEvent::EF_CAPS_LOCK_DOWN : 0);
     KeyEvent event(KeyEvent::ET_KEY_PRESSED, key_code, flags, 1, 0);
-    return textfield_view_->OnKeyPressed(event);
+    return textfield_->OnKeyPressed(event);
   }
 
   bool SendKeyEventToTextfieldViews(app::KeyboardCode key_code,
@@ -93,7 +110,10 @@ class NativeTextfieldViewsTest : public ::testing::Test,
     return SendKeyEventToTextfieldViews(key_code, false, false);
   }
 
- protected:
+  View* GetFocusedView() {
+    return widget_->GetFocusManager()->GetFocusedView();
+  }
+
   // We need widget to populate wrapper class.
   Widget* widget_;
 
@@ -241,7 +261,7 @@ TEST_F(NativeTextfieldViewsTest, PasswordTest) {
   EXPECT_STR_EQ("my password", last_contents_);
 }
 
-TEST_F(NativeTextfieldViewsTest, TestOnKeyPressReturnValue) {
+TEST_F(NativeTextfieldViewsTest, OnKeyPressReturnValueTest) {
   InitTextfield(Textfield::STYLE_DEFAULT);
   EXPECT_TRUE(SendKeyEventToTextfieldViews(app::VKEY_A));
   // F24, up/down key won't be handled.
@@ -300,4 +320,32 @@ TEST_F(NativeTextfieldViewsTest, CursorMovement) {
   EXPECT_STR_EQ("one two", last_contents_);
 }
 
+TEST_F(NativeTextfieldViewsTest, FocusTraversalTest) {
+  InitTextfields(Textfield::STYLE_DEFAULT, 3);
+  textfield_->RequestFocus();
+
+  EXPECT_EQ(1, GetFocusedView()->GetID());
+  widget_->GetFocusManager()->AdvanceFocus(false);
+  EXPECT_EQ(2, GetFocusedView()->GetID());
+  widget_->GetFocusManager()->AdvanceFocus(false);
+  EXPECT_EQ(3, GetFocusedView()->GetID());
+  // Cycle back to the first textfield.
+  widget_->GetFocusManager()->AdvanceFocus(false);
+  EXPECT_EQ(1, GetFocusedView()->GetID());
+
+  widget_->GetFocusManager()->AdvanceFocus(true);
+  EXPECT_EQ(3, GetFocusedView()->GetID());
+  widget_->GetFocusManager()->AdvanceFocus(true);
+  EXPECT_EQ(2, GetFocusedView()->GetID());
+  widget_->GetFocusManager()->AdvanceFocus(true);
+  EXPECT_EQ(1, GetFocusedView()->GetID());
+  // Cycle back to the last textfield.
+  widget_->GetFocusManager()->AdvanceFocus(true);
+  EXPECT_EQ(3, GetFocusedView()->GetID());
+
+  // Request focus should still work.
+  textfield_->RequestFocus();
+  EXPECT_EQ(1, GetFocusedView()->GetID());
+}
+
 }  // namespace views

views/controls/textfield/native_textfield_win.cc

@@ -275,10 +275,11 @@ void NativeTextfieldWin::UpdateVerticalMargins() {
   NOTIMPLEMENTED();
 }
 
-void NativeTextfieldWin::SetFocus() {
+bool NativeTextfieldWin::SetFocus() {
   // Focus the associated HWND.
   //container_view_->Focus();
   ::SetFocus(m_hWnd);
+  return true;
 }
 
 View* NativeTextfieldWin::GetView() {
@@ -303,6 +304,23 @@ bool NativeTextfieldWin::IsIMEComposing() const {
   return composition_size > 0;
 }
 
+bool NativeTextfieldWin::HandleKeyPressed(const views::KeyEvent& e) {
+  return false;
+}
+
+bool NativeTextfieldWin::HandleKeyReleased(const views::KeyEvent& e) {
+  return false;
+}
+
+void NativeTextfieldWin::HandleWillGainFocus() {
+}
+
+void NativeTextfieldWin::HandleDidGainFocus() {
+}
+
+void NativeTextfieldWin::HandleWillLoseFocus() {
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // NativeTextfieldWin, menus::SimpleMenuModel::Delegate implementation:
 

views/controls/textfield/native_textfield_win.h

@@ -22,9 +22,9 @@
 
 namespace views {
 
+class Menu2;
 class NativeViewHost;
 class Textfield;
-class Menu2;
 
 static const int kDefaultEditStyle = WS_CHILD | WS_VISIBLE | WS_CLIPCHILDREN |
     WS_CLIPSIBLINGS;
@@ -62,10 +62,15 @@ class NativeTextfieldWin
   virtual gfx::Insets CalculateInsets();
   virtual void UpdateHorizontalMargins();
   virtual void UpdateVerticalMargins();
-  virtual void SetFocus();
+  virtual bool SetFocus();
   virtual View* GetView();
   virtual gfx::NativeView GetTestingHandle() const;
   virtual bool IsIMEComposing() const;
+  virtual bool HandleKeyPressed(const views::KeyEvent& e);
+  virtual bool HandleKeyReleased(const views::KeyEvent& e);
+  virtual void HandleWillGainFocus();
+  virtual void HandleDidGainFocus();
+  virtual void HandleWillLoseFocus();
 
   // Overridden from menus::SimpleMenuModel::Delegate:
   virtual bool IsCommandIdChecked(int command_id) const;

views/controls/textfield/native_textfield_wrapper.h

@@ -15,6 +15,7 @@ class Insets;
 
 namespace views {
 
+class KeyEvent;
 class Textfield;
 class View;
 
@@ -77,8 +78,9 @@ class NativeTextfieldWrapper {
   // Updates the vertical margins for the native text field.
   virtual void UpdateVerticalMargins() = 0;
 
-  // Sets the focus to the text field.
-  virtual void SetFocus() = 0;
+  // Sets the focus to the text field. Returns false if the wrapper
+  // didn't take focus.
+  virtual bool SetFocus() = 0;
 
   // Retrieves the views::View that hosts the native control.
   virtual View* GetView() = 0;
@@ -89,6 +91,23 @@ class NativeTextfieldWrapper {
   // Returns whether or not an IME is composing text.
   virtual bool IsIMEComposing() const = 0;
 
+  // Following methods are to forward key/focus related events to the
+  // views wrapper so that TextfieldViews can handle key inputs without
+  // having focus.
+
+  // Invoked when a key is pressed/release on Textfield.  Subclasser
+  // should return true if the event has been processed and false
+  // otherwise.
+  // See also View::OnKeyPressed/OnKeyReleased.
+  virtual bool HandleKeyPressed(const views::KeyEvent& e) = 0;
+  virtual bool HandleKeyReleased(const views::KeyEvent& e) = 0;
+
+  // Invoked when focus is being moved from or to the Textfield.
+  // See also View::WillGainFocus/DidGainFocus/WillLoseFocus.
+  virtual void HandleWillGainFocus() = 0;
+  virtual void HandleDidGainFocus() = 0;
+  virtual void HandleWillLoseFocus() = 0;
+
   // Creates an appropriate NativeTextfieldWrapper for the platform.
   static NativeTextfieldWrapper* CreateWrapper(Textfield* field);
 };

views/controls/textfield/textfield.cc

@@ -261,7 +261,7 @@ gfx::Size Textfield::GetPreferredSize() {
 }
 
 bool Textfield::IsFocusable() const {
-  return IsEnabled() && !read_only_;
+  return View::IsFocusable() && !read_only_;
 }
 
 void Textfield::AboutToRequestFocusFromTabTraversal(bool reverse) {
@@ -290,6 +290,29 @@ void Textfield::PaintFocusBorder(gfx::Canvas* canvas) {
     View::PaintFocusBorder(canvas);
 }
 
+bool Textfield::OnKeyPressed(const views::KeyEvent& e) {
+  return native_wrapper_ && native_wrapper_->HandleKeyPressed(e);
+}
+
+bool Textfield::OnKeyReleased(const views::KeyEvent& e) {
+  return native_wrapper_ && native_wrapper_->HandleKeyReleased(e);
+}
+
+void Textfield::WillGainFocus() {
+  if (native_wrapper_)
+    native_wrapper_->HandleWillGainFocus();
+}
+
+void Textfield::DidGainFocus() {
+  if (native_wrapper_)
+    native_wrapper_->HandleDidGainFocus();
+}
+
+void Textfield::WillLoseFocus() {
+  if (native_wrapper_)
+    native_wrapper_->HandleWillLoseFocus();
+}
+
 AccessibilityTypes::Role Textfield::GetAccessibleRole() {
   return AccessibilityTypes::ROLE_TEXT;
 }
@@ -316,12 +339,11 @@ void Textfield::SetEnabled(bool enabled) {
 }
 
 void Textfield::Focus() {
-  if (native_wrapper_) {
-    // Forward the focus to the wrapper if it exists.
-    native_wrapper_->SetFocus();
-  } else {
-    // If there is no wrapper, cause the RootView to be focused so that we still
-    // get keyboard messages.
+  // Forward the focus to the wrapper if it exists.
+  if (!native_wrapper_ || !native_wrapper_->SetFocus()) {
+    // If there is no wrapper or the wrapper din't take focus, call
+    // View::Focus to clear the native focus so that we still get
+    // keyboard messages.
     View::Focus();
   }
 }

views/controls/textfield/textfield.h

@@ -202,6 +202,11 @@ class Textfield : public View {
   virtual bool SkipDefaultKeyEventProcessing(const KeyEvent& e);
   virtual void SetEnabled(bool enabled);
   virtual void PaintFocusBorder(gfx::Canvas* canvas);
+  virtual bool OnKeyPressed(const views::KeyEvent& e);
+  virtual bool OnKeyReleased(const views::KeyEvent& e);
+  virtual void WillGainFocus();
+  virtual void DidGainFocus();
+  virtual void WillLoseFocus();
 
   // Accessibility accessors, overridden from View:
   virtual AccessibilityTypes::Role GetAccessibleRole();