MacViews: Implement Full Keyboard Access.

ui/views/focus/focus_traversal_unittest.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 "ui/views/focus/focus_manager.h"
+#include <stddef.h>
 
-#include <stddef.h>
+#include <iterator>
 
 #include "base/macros.h"
 #include "base/run_loop.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/utf_string_conversions.h"
 #include "ui/base/models/combobox_model.h"
 #include "ui/views/background.h"
 #include "ui/views/border.h"
 #include "ui/views/controls/button/checkbox.h"
 #include "ui/views/controls/button/label_button.h"
 #include "ui/views/controls/button/radio_button.h"
 #include "ui/views/controls/combobox/combobox.h"
 #include "ui/views/controls/label.h"
 #include "ui/views/controls/link.h"
 #include "ui/views/controls/native/native_view_host.h"
 #include "ui/views/controls/scroll_view.h"
 #include "ui/views/controls/tabbed_pane/tabbed_pane.h"
 #include "ui/views/controls/textfield/textfield.h"
+#include "ui/views/focus/focus_manager.h"
 #include "ui/views/test/focus_manager_test.h"
 #include "ui/views/widget/root_view.h"
 #include "ui/views/widget/widget.h"
 
 using base::ASCIIToUTF16;
 
 namespace views {
 
 namespace {
 
+// Helper function to reverse an iterator.
+template <typename Iterator>
+std::reverse_iterator<Iterator> reverse_iter(Iterator iter) {
+  return std::reverse_iterator<Iterator>(iter);
+}
+
 int count = 1;
 
 const int kTopCheckBoxID = count++;  // 1
 const int kLeftContainerID = count++;
 const int kAppleLabelID = count++;
 const int kAppleTextfieldID = count++;
 const int kOrangeLabelID = count++;  // 5
 const int kOrangeTextfieldID = count++;
 const int kBananaLabelID = count++;
 const int kBananaTextfieldID = count++;
@@ skipping 149 matching lines @@
       view = style_tab_->GetSelectedTab()->GetViewByID(id);
     if (view)
       return view;
     view = search_border_view_->GetContentsRootView()->GetViewByID(id);
     if (view)
       return view;
     return NULL;
   }
 
  protected:
+  // Helper function to advance focus multiple times in a loop. |begin| and
+  // |end| are iterators pointing to view-ids and should be in the order in
+  // which views gain focus. |reverse| denotes the direction in which focus
+  // should be advanced.
+  template <typename Iterator>
+  void AdvanceEntireFocusLoop(Iterator begin, Iterator end, bool reverse) {

[tapted, 2016/05/03 08:08:25]

Hm it would be nice to avoid the template<> - I think it's the reversal
requirement that means we can't just use int*. And since callers change what
they do in between reversing (i.e. FocusManager::ClearFocus or
View::RequestFocus), we can't just put that in here.

An improvement might be

template <size_t SIZE>
void AdvanceEntireFocusLoop(const int (&traversal_ids)[SIZE], bool reverse) {
..
  for (int i...3
    for (int j = 0..N
      EXPECT_EQ(traversal_id[reverse ? N - j - 1 : j], focused_view->id())
}



Since we're not going in both directions, there should be a 

SCOPED_TRACE(testing::Message() << "In reverse: " << reverse);

at the start of the function too.

We could also pass in (possibly null) FocusManager* or View* arguments to do
both directions, calling ClearFocus or ReqeustFocus appropriately, and drop the
`reverse` argument. That might become less readable though

[karandeepb, 2016/05/04 01:56:38]

Done. 

+    for (int i = 0; i < 3; ++i) {
+      for (auto it = begin; it != end; it++) {
+        GetFocusManager()->AdvanceFocus(reverse);
+        View* focused_view = GetFocusManager()->GetFocusedView();
+        EXPECT_NE(nullptr, focused_view);
+        if (focused_view)
+          EXPECT_EQ(*it, focused_view->id());
+      }
+    }
+  }
+
   TabbedPane* style_tab_;
   BorderView* search_border_view_;
   DummyComboboxModel combobox_model_;
   PaneView* left_container_;
   PaneView* right_container_;
 
   DISALLOW_COPY_AND_ASSIGN(FocusTraversalTest);
 };
 
 FocusTraversalTest::FocusTraversalTest()
@@ skipping 355 matching lines @@
       kFruitButtonID, kFruitCheckBoxID, kComboboxID, kBroccoliButtonID,
       kRosettaLinkID, kStupeurEtTremblementLinkID,
       kDinerGameLinkID, kRidiculeLinkID, kClosetLinkID, kVisitingLinkID,
       kAmelieLinkID, kJoyeuxNoelLinkID, kCampingLinkID, kBriceDeNiceLinkID,
       kTaxiLinkID, kAsterixLinkID, kOKButtonID, kCancelButtonID, kHelpButtonID,
       kStyleContainerID, kBoldCheckBoxID, kItalicCheckBoxID,
       kUnderlinedCheckBoxID, kStyleHelpLinkID, kStyleTextEditID,
       kSearchTextfieldID, kSearchButtonID, kHelpLinkID,
       kThumbnailContainerID, kThumbnailStarID, kThumbnailSuperStarID };
 
+  SCOPED_TRACE("NormalTraversal");
+
   // Let's traverse the whole focus hierarchy (several times, to make sure it
   // loops OK).
   GetFocusManager()->ClearFocus();
-  for (int i = 0; i < 3; ++i) {
-    for (size_t j = 0; j < arraysize(kTraversalIDs); j++) {
-      GetFocusManager()->AdvanceFocus(false);
-      View* focused_view = GetFocusManager()->GetFocusedView();
-      EXPECT_TRUE(focused_view != NULL);
-      if (focused_view)
-        EXPECT_EQ(kTraversalIDs[j], focused_view->id());
-    }
-  }
+  bool reverse = false;

[tapted, 2016/05/03 08:08:26]

I probably wouldn't worry about this extra variable - the AdvanceEntireFocusLoop
prototype is close enough to see what a true/false argument is doing

[karandeepb, 2016/05/04 01:56:38]

Done.

+  AdvanceEntireFocusLoop(std::begin(kTraversalIDs), std::end(kTraversalIDs),
+                         reverse);
 
   // Let's traverse in reverse order.
   GetFocusManager()->ClearFocus();
-  for (int i = 0; i < 3; ++i) {
-    for (int j = arraysize(kTraversalIDs) - 1; j >= 0; --j) {
-      GetFocusManager()->AdvanceFocus(true);
-      View* focused_view = GetFocusManager()->GetFocusedView();
-      EXPECT_TRUE(focused_view != NULL);
-      if (focused_view)
-        EXPECT_EQ(kTraversalIDs[j], focused_view->id());
-    }
-  }
+  reverse = true;
+  AdvanceEntireFocusLoop(reverse_iter(std::end(kTraversalIDs)),
+                         reverse_iter(std::begin(kTraversalIDs)), reverse);
 }
 
+#if defined(OS_MACOSX)
+// Test focus traversal with full keyboard access off on Mac.
+TEST_F(FocusTraversalTest, NormalTraversalMac) {
+  GetFocusManager()->SetKeyboardAccessible(false);
+
+  // Now only views with FocusBehavior of ALWAYS will be focusable.
+  const int kTraversalIDs[] = {kAppleTextfieldID,    kOrangeTextfieldID,
+                               kBananaTextfieldID,   kKiwiTextfieldID,
+                               kStyleTextEditID,     kSearchTextfieldID,
+                               kThumbnailContainerID};
+
+  SCOPED_TRACE("NormalTraversalMac");
+
+  // Let's traverse the whole focus hierarchy (several times, to make sure it
+  // loops OK).
+  GetFocusManager()->ClearFocus();
+  bool reverse = false;
+  AdvanceEntireFocusLoop(std::begin(kTraversalIDs), std::end(kTraversalIDs),
+                         reverse);
+
+  // Let's traverse in reverse order.
+  GetFocusManager()->ClearFocus();
+  reverse = true;
+  AdvanceEntireFocusLoop(reverse_iter(std::end(kTraversalIDs)),
+                         reverse_iter(std::begin(kTraversalIDs)), reverse);
+}
+
+// Test toggling full keyboard access correctly changes the focused view on Mac.
+TEST_F(FocusTraversalTest, FullKeyboardToggle) {
+  // Give focus to kTopCheckBoxID .
+  FindViewByID(kTopCheckBoxID)->RequestFocus();
+  EXPECT_EQ(kTopCheckBoxID, GetFocusManager()->GetFocusedView()->id());
+
+  // Turn off full keyboard access. Focus should move to next view with ALWAYS
+  // focus behavior.
+  GetFocusManager()->SetKeyboardAccessible(false);
+  EXPECT_EQ(kAppleTextfieldID, GetFocusManager()->GetFocusedView()->id());
+
+  // Turning on full keyboard access should not change the focused view.
+  GetFocusManager()->SetKeyboardAccessible(true);
+  EXPECT_EQ(kAppleTextfieldID, GetFocusManager()->GetFocusedView()->id());
+
+  // Give focus to kSearchButtonID.
+  FindViewByID(kSearchButtonID)->RequestFocus();
+  EXPECT_EQ(kSearchButtonID, GetFocusManager()->GetFocusedView()->id());
+
+  // Turn off full keyboard access. Focus should move to next view with ALWAYS
+  // focus behavior.
+  GetFocusManager()->SetKeyboardAccessible(false);
+  EXPECT_EQ(kThumbnailContainerID, GetFocusManager()->GetFocusedView()->id());
+
+  // See focus advances correctly in both directions.
+  GetFocusManager()->AdvanceFocus(false);
+  EXPECT_EQ(kAppleTextfieldID, GetFocusManager()->GetFocusedView()->id());
+
+  GetFocusManager()->AdvanceFocus(true);
+  EXPECT_EQ(kThumbnailContainerID, GetFocusManager()->GetFocusedView()->id());
+}
+#endif  // OS_MACOSX
+
 TEST_F(FocusTraversalTest, TraversalWithNonEnabledViews) {
   const int kDisabledIDs[] = {
       kBananaTextfieldID, kFruitCheckBoxID, kComboboxID, kAsparagusButtonID,
       kCauliflowerButtonID, kClosetLinkID, kVisitingLinkID, kBriceDeNiceLinkID,
       kTaxiLinkID, kAsterixLinkID, kHelpButtonID, kBoldCheckBoxID,
       kSearchTextfieldID, kHelpLinkID };
 
   const int kTraversalIDs[] = { kTopCheckBoxID,  kAppleTextfieldID,
       kOrangeTextfieldID, kKiwiTextfieldID, kFruitButtonID, kBroccoliButtonID,
       kRosettaLinkID, kStupeurEtTremblementLinkID, kDinerGameLinkID,
       kRidiculeLinkID, kAmelieLinkID, kJoyeuxNoelLinkID, kCampingLinkID,
       kOKButtonID, kCancelButtonID, kStyleContainerID, kItalicCheckBoxID,
       kUnderlinedCheckBoxID, kStyleHelpLinkID, kStyleTextEditID,
       kSearchButtonID, kThumbnailContainerID, kThumbnailStarID,
       kThumbnailSuperStarID };
 
+  SCOPED_TRACE("TraversalWithNonEnabledViews");
+
   // Let's disable some views.
   for (size_t i = 0; i < arraysize(kDisabledIDs); i++) {
     View* v = FindViewByID(kDisabledIDs[i]);
     ASSERT_TRUE(v != NULL);
     v->SetEnabled(false);
   }
 
-  View* focused_view;
   // Let's do one traversal (several times, to make sure it loops ok).
   GetFocusManager()->ClearFocus();
-  for (int i = 0; i < 3; ++i) {
-    for (size_t j = 0; j < arraysize(kTraversalIDs); j++) {
-      GetFocusManager()->AdvanceFocus(false);
-      focused_view = GetFocusManager()->GetFocusedView();
-      EXPECT_TRUE(focused_view != NULL);
-      if (focused_view)
-        EXPECT_EQ(kTraversalIDs[j], focused_view->id());
-    }
-  }
+  bool reverse = false;
+  AdvanceEntireFocusLoop(std::begin(kTraversalIDs), std::end(kTraversalIDs),
+                         reverse);
 
   // Same thing in reverse.
   GetFocusManager()->ClearFocus();
-  for (int i = 0; i < 3; ++i) {
-    for (int j = arraysize(kTraversalIDs) - 1; j >= 0; --j) {
-      GetFocusManager()->AdvanceFocus(true);
-      focused_view = GetFocusManager()->GetFocusedView();
-      EXPECT_TRUE(focused_view != NULL);
-      if (focused_view)
-        EXPECT_EQ(kTraversalIDs[j], focused_view->id());
-    }
-  }
+  reverse = true;
+  AdvanceEntireFocusLoop(reverse_iter(std::end(kTraversalIDs)),
+                         reverse_iter(std::begin(kTraversalIDs)), reverse);
 }
 
 TEST_F(FocusTraversalTest, TraversalWithInvisibleViews) {
   const int kInvisibleIDs[] = { kTopCheckBoxID, kOKButtonID,
       kThumbnailContainerID };
 
   const int kTraversalIDs[] = { kAppleTextfieldID, kOrangeTextfieldID,
       kBananaTextfieldID, kKiwiTextfieldID, kFruitButtonID, kFruitCheckBoxID,
       kComboboxID, kBroccoliButtonID, kRosettaLinkID,
       kStupeurEtTremblementLinkID, kDinerGameLinkID, kRidiculeLinkID,
       kClosetLinkID, kVisitingLinkID, kAmelieLinkID, kJoyeuxNoelLinkID,
       kCampingLinkID, kBriceDeNiceLinkID, kTaxiLinkID, kAsterixLinkID,
       kCancelButtonID, kHelpButtonID, kStyleContainerID, kBoldCheckBoxID,
       kItalicCheckBoxID, kUnderlinedCheckBoxID, kStyleHelpLinkID,
       kStyleTextEditID, kSearchTextfieldID, kSearchButtonID, kHelpLinkID };
 
+  SCOPED_TRACE("TraversalWithInvisibleViews");
 
   // Let's make some views invisible.
   for (size_t i = 0; i < arraysize(kInvisibleIDs); i++) {
     View* v = FindViewByID(kInvisibleIDs[i]);
     ASSERT_TRUE(v != NULL);
     v->SetVisible(false);
   }
 
-  View* focused_view;
   // Let's do one traversal (several times, to make sure it loops ok).
   GetFocusManager()->ClearFocus();
-  for (int i = 0; i < 3; ++i) {
-    for (size_t j = 0; j < arraysize(kTraversalIDs); j++) {
-      GetFocusManager()->AdvanceFocus(false);
-      focused_view = GetFocusManager()->GetFocusedView();
-      EXPECT_TRUE(focused_view != NULL);
-      if (focused_view)
-        EXPECT_EQ(kTraversalIDs[j], focused_view->id());
-    }
-  }
+  bool reverse = false;
+  AdvanceEntireFocusLoop(std::begin(kTraversalIDs), std::end(kTraversalIDs),
+                         reverse);
 
   // Same thing in reverse.
   GetFocusManager()->ClearFocus();
-  for (int i = 0; i < 3; ++i) {
-    for (int j = arraysize(kTraversalIDs) - 1; j >= 0; --j) {
-      GetFocusManager()->AdvanceFocus(true);
-      focused_view = GetFocusManager()->GetFocusedView();
-      EXPECT_TRUE(focused_view != NULL);
-      if (focused_view)
-        EXPECT_EQ(kTraversalIDs[j], focused_view->id());
-    }
-  }
+  reverse = true;
+  AdvanceEntireFocusLoop(reverse_iter(std::end(kTraversalIDs)),
+                         reverse_iter(std::begin(kTraversalIDs)), reverse);
 }
 
 TEST_F(FocusTraversalTest, PaneTraversal) {
   // Tests trapping the traversal within a pane - useful for full
   // keyboard accessibility for toolbars.
 
   // First test the left container.
   const int kLeftTraversalIDs[] = {
     kAppleTextfieldID,
     kOrangeTextfieldID, kBananaTextfieldID, kKiwiTextfieldID,
     kFruitButtonID, kFruitCheckBoxID, kComboboxID };
 
+  SCOPED_TRACE("PaneTraversal");
+
   FocusSearch focus_search_left(left_container_, true, false);
   left_container_->EnablePaneFocus(&focus_search_left);
   FindViewByID(kComboboxID)->RequestFocus();
 
   // Traverse the focus hierarchy within the pane several times.
-  for (int i = 0; i < 3; ++i) {
-    for (size_t j = 0; j < arraysize(kLeftTraversalIDs); j++) {
-      GetFocusManager()->AdvanceFocus(false);
-      View* focused_view = GetFocusManager()->GetFocusedView();
-      EXPECT_TRUE(focused_view != NULL);
-      if (focused_view)
-        EXPECT_EQ(kLeftTraversalIDs[j], focused_view->id());
-    }
-  }
+  bool reverse = false;
+  AdvanceEntireFocusLoop(std::begin(kLeftTraversalIDs),
+                         std::end(kLeftTraversalIDs), reverse);
 
   // Traverse in reverse order.
   FindViewByID(kAppleTextfieldID)->RequestFocus();
-  for (int i = 0; i < 3; ++i) {
-    for (int j = arraysize(kLeftTraversalIDs) - 1; j >= 0; --j) {
-      GetFocusManager()->AdvanceFocus(true);
-      View* focused_view = GetFocusManager()->GetFocusedView();
-      EXPECT_TRUE(focused_view != NULL);
-      if (focused_view)
-        EXPECT_EQ(kLeftTraversalIDs[j], focused_view->id());
-    }
-  }
+  reverse = true;
+  AdvanceEntireFocusLoop(reverse_iter(std::end(kLeftTraversalIDs)),
+                         reverse_iter(std::begin(kLeftTraversalIDs)), reverse);
 
   // Now test the right container, but this time with accessibility mode.
   // Make some links not focusable, but mark one of them as
   // "accessibility focusable", so it should show up in the traversal.
   const int kRightTraversalIDs[] = {
     kBroccoliButtonID, kDinerGameLinkID, kRidiculeLinkID,
     kClosetLinkID, kVisitingLinkID, kAmelieLinkID, kJoyeuxNoelLinkID,
     kCampingLinkID, kBriceDeNiceLinkID, kTaxiLinkID, kAsterixLinkID };
 
   FocusSearch focus_search_right(right_container_, true, true);
   right_container_->EnablePaneFocus(&focus_search_right);
   FindViewByID(kRosettaLinkID)->SetFocusBehavior(View::FocusBehavior::NEVER);
   FindViewByID(kStupeurEtTremblementLinkID)
       ->SetFocusBehavior(View::FocusBehavior::NEVER);
   FindViewByID(kDinerGameLinkID)
       ->SetFocusBehavior(View::FocusBehavior::ACCESSIBLE_ONLY);
   FindViewByID(kAsterixLinkID)->RequestFocus();
 
   // Traverse the focus hierarchy within the pane several times.
-  for (int i = 0; i < 3; ++i) {
-    for (size_t j = 0; j < arraysize(kRightTraversalIDs); j++) {
-      GetFocusManager()->AdvanceFocus(false);
-      View* focused_view = GetFocusManager()->GetFocusedView();
-      EXPECT_TRUE(focused_view != NULL);
-      if (focused_view)
-        EXPECT_EQ(kRightTraversalIDs[j], focused_view->id());
-    }
-  }
+  reverse = false;
+  AdvanceEntireFocusLoop(std::begin(kRightTraversalIDs),
+                         std::end(kRightTraversalIDs), reverse);
 
   // Traverse in reverse order.
   FindViewByID(kBroccoliButtonID)->RequestFocus();
-  for (int i = 0; i < 3; ++i) {
-    for (int j = arraysize(kRightTraversalIDs) - 1; j >= 0; --j) {
-      GetFocusManager()->AdvanceFocus(true);
-      View* focused_view = GetFocusManager()->GetFocusedView();
-      EXPECT_TRUE(focused_view != NULL);
-      if (focused_view)
-        EXPECT_EQ(kRightTraversalIDs[j], focused_view->id());
-    }
-  }
+  reverse = true;
+  AdvanceEntireFocusLoop(reverse_iter(std::end(kRightTraversalIDs)),
+                         reverse_iter(std::begin(kRightTraversalIDs)), reverse);
 }
 
 class FocusTraversalNonFocusableTest : public FocusManagerTest {
  public:
   ~FocusTraversalNonFocusableTest() override {}
 
   void InitContentView() override;
 
  protected:
   FocusTraversalNonFocusableTest() {}
@@ skipping 58 matching lines @@
   GetFocusManager()->AdvanceFocus(false);
   EXPECT_FALSE(GetFocusManager()->GetFocusedView());
 
   // Advance backwards from the root node.
   GetFocusManager()->ClearFocus();
   GetFocusManager()->AdvanceFocus(true);
   EXPECT_FALSE(GetFocusManager()->GetFocusedView());
 }
 
 }  // namespace views