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

ui/views/controls/styled_label.cc

 // Copyright 2013 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/controls/styled_label.h"
 
 #include <stddef.h>
 
 #include <limits>
 #include <vector>
@@ skipping 24 matching lines @@
 std::unique_ptr<Label> CreateLabelRange(
     const base::string16& text,
     const gfx::FontList& font_list,
     const StyledLabel::RangeStyleInfo& style_info,
     views::LinkListener* link_listener) {
   std::unique_ptr<Label> result;
 
   if (style_info.is_link) {
     Link* link = new Link(text);
     link->set_listener(link_listener);
-    link->SetUnderline((style_info.font_style & gfx::Font::UNDERLINE) != 0);
+
+    // Ignore requests to change the default underline style on Link if that's
+    // being used for the default focus style.
+    if (Link::GetDefaultFocusStyle() != Link::FocusStyle::UNDERLINE)
+      link->SetUnderline((style_info.font_style & gfx::Font::UNDERLINE) != 0);
+
     result.reset(link);
   } else {
     result.reset(new Label(text));
   }
 
   if (style_info.color != SK_ColorTRANSPARENT)
     result->SetEnabledColor(style_info.color);
   result->SetFontList(font_list);
 
   if (!style_info.tooltip.empty())
     result->SetTooltipText(style_info.tooltip);
   if (style_info.font_style != gfx::Font::NORMAL ||
       style_info.weight != gfx::Font::Weight::NORMAL) {
     result->SetFontList(result->font_list().Derive(0, style_info.font_style,
                                                    style_info.weight));
   }
 
   return result;
 }
 
 }  // namespace
 
-
 // StyledLabel::RangeStyleInfo ------------------------------------------------
 
 StyledLabel::RangeStyleInfo::RangeStyleInfo()
     : font_style(gfx::Font::NORMAL),
       weight(gfx::Font::Weight::NORMAL),
       color(SK_ColorTRANSPARENT),
       disable_line_wrapping(false),
       is_link(false) {}
 
 StyledLabel::RangeStyleInfo::~RangeStyleInfo() {}
 
 // static
 StyledLabel::RangeStyleInfo StyledLabel::RangeStyleInfo::CreateForLink() {
   RangeStyleInfo result;
   result.disable_line_wrapping = true;
   result.is_link = true;
+
+  // When focus isn't indicated by underline, default to making links underlined
+  // by default.
+  if (Link::GetDefaultFocusStyle() != Link::FocusStyle::UNDERLINE)
+    result.font_style = gfx::Font::UNDERLINE;

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

Of course.. this will add underlines to some StyledLabel instances that
currently don't have underlines -- even before --secondary-ui-md is flipped. I
don't think we want that. Plan for this is to add a `bool` (or FontStyle)
argument to CreateForLink() since the majority are setting one, and some do
really gross things like check for
MaterialDesignController::IsSecondaryUiMaterial() whenever calling
CreateForLink() -- e.g.
https://cs.chromium.org/chromium/src/chrome/browser/ui/views/page_info/page_i...
(and line 264). 

This would blow out the CL diff, so if this sounds good, I'll change the
CreateForLink() signature in a precursor CL to this.

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

After my brute-force refactoring in crrev.com/2833563003 it turns out there is
exactly one consumer that wants an underlined Link in StyledLabel (page info
bubble), and then only under MD. So it's straightforward to revert this part and
update CreateLabelRange to check for things unexpectedly adding underlines in
MD.

+
   return result;
 }
 
 
 // StyledLabel::StyleRange ----------------------------------------------------
 
 bool StyledLabel::StyleRange::operator<(
     const StyledLabel::StyleRange& other) const {
   return range.start() < other.range.start();
 }
@@ skipping 76 matching lines @@
 
   SetSize(CalculateAndDoLayout(max_width, true));
 }
 
 const char* StyledLabel::GetClassName() const {
   return kViewClassName;
 }
 
 gfx::Insets StyledLabel::GetInsets() const {
   gfx::Insets insets = View::GetInsets();
+  if (Link::GetDefaultFocusStyle() != Link::FocusStyle::RING)
+    return insets;
 
   // We need a focus border iff we contain a link that will have a focus border.
   // That in turn will be true only if the link is non-empty.
   for (StyleRanges::const_iterator i(style_ranges_.begin());
         i != style_ranges_.end(); ++i) {
     if (i->style_info.is_link && !i->range.is_empty()) {
-      const gfx::Insets focus_border_padding(
-          Label::kFocusBorderPadding, Label::kFocusBorderPadding,
-          Label::kFocusBorderPadding, Label::kFocusBorderPadding);
-      insets += focus_border_padding;
+      insets += gfx::Insets(Link::kFocusBorderPadding);
       break;
     }
   }
 
   return insets;
 }
 
 gfx::Size StyledLabel::GetPreferredSize() const {
   return calculated_size_;
 }
@@ skipping 142 matching lines @@
       // This chunk is normal text.
       if (position + chunk.size() > range.start())
         chunk = chunk.substr(0, range.start() - position);
       label = CreateLabelRange(chunk, font_list_, default_style_info_, this);
     }
 
     if (displayed_on_background_color_set_)
       label->SetBackgroundColor(displayed_on_background_color_);
     label->SetAutoColorReadabilityEnabled(auto_color_readability_enabled_);
 
-    // Calculate the size of the optional focus border, and overlap by that
-    // amount. Otherwise, "<a>link</a>," will render as "link ,".
-    gfx::Insets focus_border_insets(label->GetInsets());
-    focus_border_insets -= label->View::GetInsets();
     const gfx::Size view_size = label->GetPreferredSize();
     const gfx::Insets insets = GetInsets();
-    label->SetBoundsRect(gfx::Rect(
-        gfx::Point(
-            insets.left() + x - focus_border_insets.left(),
-            insets.top() + line * line_height - focus_border_insets.top()),
-        view_size));
-    x += view_size.width() - focus_border_insets.width();
+    gfx::Point view_origin(insets.left() + x,
+                           insets.top() + line * line_height);
+    if (Link::GetDefaultFocusStyle() == Link::FocusStyle::RING) {
+      // Calculate the size of the optional focus border, and overlap by that
+      // amount. Otherwise, "<a>link</a>," will render as "link ,".
+      gfx::Insets focus_border_insets(label->GetInsets());
+      focus_border_insets -= label->View::GetInsets();
+      view_origin.Offset(-focus_border_insets.left(),
+                         -focus_border_insets.top());
+      label->SetBoundsRect(gfx::Rect(view_origin, view_size));
+      x += view_size.width() - focus_border_insets.width();
+      used_width = std::max(used_width, x);
+      total_height =
+          std::max(total_height, label->bounds().bottom() + insets.bottom() -
+                                     focus_border_insets.bottom());
+    } else {
+      label->SetBoundsRect(gfx::Rect(view_origin, view_size));
+      x += view_size.width();
+      total_height =
+          std::max(total_height, label->bounds().bottom() + insets.bottom());
+    }
     used_width = std::max(used_width, x);
-    total_height =
-        std::max(total_height, label->bounds().bottom() + insets.bottom() -
-                                   focus_border_insets.bottom());
+
     if (!dry_run)
       AddChildView(label.release());
 
     // If |gfx::ElideRectangleText| returned more than one substring, that
     // means the whole text did not fit into remaining line width, with text
     // after |susbtring[0]| spilling into next line. If whole |substring[0]|
     // was added to the current line (this may not be the case if part of the
     // substring has different style), proceed to the next line.
     if (substrings.size() > 1 && chunk.size() == substrings[0].size()) {
       x = 0;
       ++line;
     }
 
     remaining_string = remaining_string.substr(chunk.size());
   }
 
   DCHECK_LE(used_width, width);
   calculated_size_ = gfx::Size(used_width + GetInsets().width(), total_height);
   return calculated_size_;
 }
 
 }  // namespace views