Views: Don't add insets for views::Link focus rings under MD.

ui/views/controls/label.cc

Index: ui/views/controls/label.cc
diff --git a/ui/views/controls/label.cc b/ui/views/controls/label.cc
index 762227f7b716461fc942bdcffd859ba2da2a135d..c1b98d0bd4cddf1fe4aac1496455db2a4f388ea3 100644
--- a/ui/views/controls/label.cc
+++ b/ui/views/controls/label.cc
@@ -22,7 +22,6 @@
 #include "ui/base/clipboard/scoped_clipboard_writer.h"
 #include "ui/base/cursor/cursor.h"
 #include "ui/base/default_style.h"
-#include "ui/base/material_design/material_design_controller.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/color_utils.h"
 #include "ui/gfx/geometry/insets.h"
@@ -36,9 +35,8 @@
 #include "ui/views/selection_controller.h"
 
 namespace views {
-// static
+
 const char Label::kViewClassName[] = "Label";
-const int Label::kFocusBorderPadding = 1;
 
 Label::Label() : Label(base::string16()) {
 }
@@ -302,15 +300,6 @@ void Label::SelectRange(const gfx::Range& range) {
     SchedulePaint();
 }
 
-gfx::Insets Label::GetInsets() const {
-  gfx::Insets insets = View::GetInsets();
-  if (focus_behavior() != FocusBehavior::NEVER) {
-    insets += gfx::Insets(kFocusBorderPadding, kFocusBorderPadding,
-                          kFocusBorderPadding, kFocusBorderPadding);
-  }
-  return insets;
-}
-
 int Label::GetBaseline() const {
   return GetInsets().top() + font_list().GetBaseline();
 }
@@ -453,6 +442,29 @@ std::unique_ptr<gfx::RenderText> Label::CreateRenderText(
   return render_text;
 }
 
+void Label::MaybePaintFocusRing(gfx::Canvas* canvas) const {
+  // No focus ring by default.
+}
+
+gfx::Rect Label::GetFocusRingBounds() const {
+  MaybeBuildRenderTextLines();
+
+  gfx::Rect focus_bounds;
+  if (lines_.empty()) {
+    focus_bounds = gfx::Rect(GetTextSize());
+  } else {
+    for (size_t i = 0; i < lines_.size(); ++i) {
+      gfx::Point origin;
+      origin += lines_[i]->GetLineOffset(0);
+      focus_bounds.Union(gfx::Rect(origin, lines_[i]->GetStringSize()));
+    }
+  }
+
+  focus_bounds.Inset(-(GetInsets() - View::GetInsets()));
+  focus_bounds.Intersect(GetLocalBounds());
+  return focus_bounds;
+}
+
 void Label::PaintText(gfx::Canvas* canvas) {
   MaybeBuildRenderTextLines();
 
@@ -499,13 +511,8 @@ void Label::OnPaint(gfx::Canvas* canvas) {
     PaintText(canvas);
   }
 
-  // Check for IsAccessibilityFocusable() to prevent drawing a focus rect for
-  // non-focusable labels with selection, which are given focus explicitly in

[tapted, 2017/04/19 12:28:45]

views::Link are never selectable (Link::IsSelectionSupported() overrides to
return false), so OnMousePressed will never RequestFocus and Label no longer has
focus rings so this should no longer be an issue.

-  // OnMousePressed.
-  if (HasFocus() && !ui::MaterialDesignController::IsSecondaryUiMaterial() &&
-      IsAccessibilityFocusable()) {
-    canvas->DrawFocusRect(GetFocusBounds());
-  }
+  if (HasFocus())
+    MaybePaintFocusRing(canvas);
 }
 
 void Label::OnNativeThemeChanged(const ui::NativeTheme* theme) {
@@ -843,10 +850,10 @@ void Label::MaybeBuildRenderTextLines() const {
     return;
 
   gfx::Rect rect = GetContentsBounds();
-  if (focus_behavior() != FocusBehavior::NEVER)
-    rect.Inset(kFocusBorderPadding, kFocusBorderPadding);
+  rect.Inset(GetInsets() - View::GetInsets());

[sky, 2017/04/19 17:27:46]

This is very subtle and worth a comment. In fact I tend to think it's a bug that
View::GetContentsBounds() doesn't call GetInsets().

[tapted, 2017/04/20 11:50:17]

Done.

   if (rect.IsEmpty())
     return;
+
   rect.Inset(-gfx::ShadowValue::GetMargin(shadows()));
 
   gfx::HorizontalAlignment alignment = horizontal_alignment();
@@ -902,25 +909,6 @@ void Label::MaybeBuildRenderTextLines() const {
   ApplyTextColors();
 }
 
-gfx::Rect Label::GetFocusBounds() const {
-  MaybeBuildRenderTextLines();
-
-  gfx::Rect focus_bounds;
-  if (lines_.empty()) {
-    focus_bounds = gfx::Rect(GetTextSize());
-  } else {
-    for (size_t i = 0; i < lines_.size(); ++i) {
-      gfx::Point origin;
-      origin += lines_[i]->GetLineOffset(0);
-      focus_bounds.Union(gfx::Rect(origin, lines_[i]->GetStringSize()));
-    }
-  }
-
-  focus_bounds.Inset(-kFocusBorderPadding, -kFocusBorderPadding);
-  focus_bounds.Intersect(GetLocalBounds());
-  return focus_bounds;
-}
-
 std::vector<base::string16> Label::GetLinesForWidth(int width) const {
   std::vector<base::string16> lines;
   // |width| can be 0 when getting the default text size, in that case