MacViews: Implement Full Keyboard Access.

ui/views/controls/link.cc

Index: ui/views/controls/link.cc
diff --git a/ui/views/controls/link.cc b/ui/views/controls/link.cc
index ec955b385979911957b4c951a59fe53d2357399d..dfd5ba7e1d7b19ce8cbafa8c1075af97200bb23b 100644
--- a/ui/views/controls/link.cc
+++ b/ui/views/controls/link.cc
@@ -20,6 +20,7 @@
 #include "ui/native_theme/native_theme.h"
 #include "ui/views/controls/link_listener.h"
 #include "ui/views/native_cursor.h"
+#include "ui/views/style/platform_style.h"
 
 namespace views {
 
@@ -159,10 +160,16 @@ void Link::SetFontList(const gfx::FontList& font_list) {
 
 void Link::SetText(const base::string16& text) {
   Label::SetText(text);
+
   // Disable focusability for empty links.  Otherwise Label::GetInsets() will

[tapted, 2016/02/23 03:01:19]

This comment about Label::GetInsets() concerns me. Does Label::GetInsets() need
to be changed to use IsAccessibilityFocusable()?  (or, perhaps neater, should
the text.empty() check move to Label::GetInsets()? then leave off the
SetAccessibilityFocusable(false); calls 

[karandeepb, 2016/03/15 02:19:50]

Yeah GetInsets() is currently using focusable() to determine whether to add
insets or not. This way insets won't be added to links while in Full Keyboard
access mode on Mac. This can be corrected by using IsAccessibilityFocusable().
However, using focusable() instead of IsFocusable() seems to have been a
conscious decision -  https://codereview.chromium.org/111023004/#msg4

   // give them an unconditional 1-px. inset on every side to allow for a focus
   // border, when in this case we probably wanted zero width.
-  SetFocusable(!text.empty());
+  if (!text.empty()) {
+    PlatformStyle::SetControlStyleFocus(this);
+  } else {
+    SetFocusable(false);
+    SetAccessibilityFocusable(false);
+  }
 }
 
 void Link::OnNativeThemeChanged(const ui::NativeTheme* theme) {
@@ -198,9 +205,14 @@ void Link::Init() {
 
   // Label::Init() calls SetText(), but if that's being called from Label(), our
   // SetText() override will not be reached (because the constructed class is
-  // only a Label at the moment, not yet a Link).  So so the set_focusable()
-  // call explicitly here.
-  SetFocusable(!text().empty());
+  // only a Label at the moment, not yet a Link).  So explicitly set
+  // focusability here.
+  if (!text().empty()) {

[tapted, 2016/02/23 03:01:19]

This could also just call SetText again, but we should be able to simplify it so
there's not as much copy-paste

[karandeepb, 2016/03/15 02:19:50]

Done.

+    PlatformStyle::SetControlStyleFocus(this);
+  } else {
+    SetFocusable(false);
+    SetAccessibilityFocusable(false);
+  }
 }
 
 void Link::SetPressed(bool pressed) {