Content Setting Bubbles: Make the "learn more" link a help image button

This aligns the look more with the future Harmony UI. Additionally,
this CL addds a .icon version of the "help-outline" svg image in
chrome/browser/resources/settings/icons.html

Bug: 736318
Change-Id: I41090a0ee5ed7e23490855213baf6b9d89b78629
Reviewed-on: https://chromium-review.googlesource.com/544199
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491191}

chrome/browser/ui/cocoa/content_settings/content_setting_bubble_cocoa.mm

@@ -1018,7 +1018,7 @@ const ContentTypeToNibPath kNibPaths[] = {
 }
 
 - (IBAction)learnMoreLinkClicked:(id)sender {
-  contentSettingBubbleModel_->OnLearnMoreLinkClicked();
+  contentSettingBubbleModel_->OnLearnMoreClicked();
 }
 
 - (IBAction)manageBlocking:(id)sender {

chrome/browser/ui/cocoa/subresource_filter/subresource_filter_bubble_controller.mm

@@ -64,8 +64,9 @@
   [messageLabel_ release];
 
   // Set up the "Learn more" link.
-  NSString* linkText = base::SysUTF16ToNSString(
-      contentSettingBubbleModel_->bubble_content().learn_more_link);
+  // TODO(csharrison): Migrate this to use the help icon to align with
+  // Windows/Linux.
+  NSString* linkText = l10n_util::GetNSString(IDS_LEARN_MORE);
   learnMoreLink_ = [[NSButton alloc] initWithFrame:NSMakeRect(18, 76, 282, 28)];
   base::scoped_nsobject<HyperlinkButtonCell> cell(
       [[HyperlinkButtonCell alloc] initTextCell:linkText]);

chrome/browser/ui/content_settings/content_setting_bubble_model.cc

@@ -443,7 +443,7 @@ class ContentSettingPluginBubbleModel : public ContentSettingSimpleBubbleModel {
                                   Profile* profile);
 
  private:
-  void OnLearnMoreLinkClicked() override;
+  void OnLearnMoreClicked() override;
   void OnCustomLinkClicked() override;
 
   void RunPluginsOnPage();
@@ -505,13 +505,13 @@ ContentSettingPluginBubbleModel::ContentSettingPluginBubbleModel(
     }
   }
 
-  set_learn_more_link(l10n_util::GetStringUTF16(IDS_LEARN_MORE));
+  set_show_learn_more(true);
 
   content_settings::RecordPluginsAction(
       content_settings::PLUGINS_ACTION_DISPLAYED_BUBBLE);
 }
 
-void ContentSettingPluginBubbleModel::OnLearnMoreLinkClicked() {
+void ContentSettingPluginBubbleModel::OnLearnMoreClicked() {
   if (delegate())
     delegate()->ShowLearnMorePage(CONTENT_SETTINGS_TYPE_PLUGINS);
 
@@ -1240,7 +1240,7 @@ ContentSettingSubresourceFilterBubbleModel::
   // TODO(csharrison): The learn more link UI layout is non ideal. Make it align
   // with the future Harmony UI version with a link icon in the bottom left of
   // the bubble.
-  set_learn_more_link(l10n_util::GetStringUTF16(IDS_LEARN_MORE));
+  set_show_learn_more(true);
   ChromeSubresourceFilterClient::LogAction(kActionDetailsShown);
 }
 
@@ -1273,7 +1273,7 @@ void ContentSettingSubresourceFilterBubbleModel::OnManageCheckboxChecked(
   is_checked_ = is_checked;
 }
 
-void ContentSettingSubresourceFilterBubbleModel::OnLearnMoreLinkClicked() {
+void ContentSettingSubresourceFilterBubbleModel::OnLearnMoreClicked() {
   DCHECK(delegate());
   ChromeSubresourceFilterClient::LogAction(kActionClickedLearnMore);
   delegate()->ShowLearnMorePage(CONTENT_SETTINGS_TYPE_ADS);

chrome/browser/ui/content_settings/content_setting_bubble_model.h

@@ -126,7 +126,7 @@ class ContentSettingBubbleModel : public content::NotificationObserver {
     base::string16 manage_text;
     bool show_manage_text_as_checkbox = false;
     MediaMenuMap media_menus;
-    base::string16 learn_more_link;
+    bool show_learn_more = false;
     base::string16 done_button_text;
 
    private:
@@ -157,7 +157,7 @@ class ContentSettingBubbleModel : public content::NotificationObserver {
   virtual void OnCustomLinkClicked() {}
   virtual void OnManageLinkClicked() {}
   virtual void OnManageCheckboxChecked(bool is_checked) {}
-  virtual void OnLearnMoreLinkClicked() {}
+  virtual void OnLearnMoreClicked() {}
   virtual void OnMediaMenuClicked(content::MediaStreamType type,
                                   const std::string& selected_device_id) {}
 
@@ -235,8 +235,8 @@ class ContentSettingBubbleModel : public content::NotificationObserver {
   void set_selected_device(const content::MediaStreamDevice& device) {
     bubble_content_.media_menus[device.type].selected_device = device;
   }
-  void set_learn_more_link(const base::string16& link) {
-    bubble_content_.learn_more_link = link;
+  void set_show_learn_more(bool show_learn_more) {
+    bubble_content_.show_learn_more = show_learn_more;
   }
   void set_done_button_text(const base::string16& done_button_text) {
     bubble_content_.done_button_text = done_button_text;
@@ -333,7 +333,7 @@ class ContentSettingSubresourceFilterBubbleModel
   void OnManageCheckboxChecked(bool is_checked) override;
   ContentSettingSubresourceFilterBubbleModel* AsSubresourceFilterBubbleModel()
       override;
-  void OnLearnMoreLinkClicked() override;
+  void OnLearnMoreClicked() override;
   void OnDoneClicked() override;
 
   bool is_checked_ = false;

chrome/browser/ui/content_settings/content_setting_bubble_model_unittest.cc

@@ -739,7 +739,7 @@ TEST_F(ContentSettingBubbleModelTest, Plugins) {
   EXPECT_FALSE(bubble_content.custom_link.empty());
   EXPECT_TRUE(bubble_content.custom_link_enabled);
   EXPECT_FALSE(bubble_content.manage_text.empty());
-  EXPECT_FALSE(bubble_content.learn_more_link.empty());
+  EXPECT_TRUE(bubble_content.show_learn_more);
 }
 
 TEST_F(ContentSettingBubbleModelTest, PepperBroker) {
@@ -943,8 +943,7 @@ TEST_F(ContentSettingBubbleModelTest, SubresourceFilter) {
             l10n_util::GetStringUTF16(IDS_BLOCKED_ADS_PROMPT_EXPLANATION));
   EXPECT_EQ(0U, bubble_content.radio_group.radio_items.size());
   EXPECT_EQ(0, bubble_content.radio_group.default_item);
-  EXPECT_EQ(l10n_util::GetStringUTF16(IDS_LEARN_MORE),
-            bubble_content.learn_more_link);
+  EXPECT_TRUE(bubble_content.show_learn_more);
   EXPECT_TRUE(bubble_content.custom_link.empty());
   EXPECT_FALSE(bubble_content.custom_link_enabled);
   EXPECT_EQ(bubble_content.manage_text,

chrome/browser/ui/views/content_setting_bubble_contents.cc

@@ -24,6 +24,7 @@
 #include "chrome/grit/generated_resources.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "components/strings/grit/components_strings.h"
+#include "components/vector_icons/vector_icons.h"
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/plugin_service.h"
 #include "content/public/browser/web_contents.h"
@@ -32,8 +33,14 @@
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/models/simple_menu_model.h"
 #include "ui/base/resource/resource_bundle.h"
+#include "ui/gfx/color_palette.h"
+#include "ui/gfx/color_utils.h"
 #include "ui/gfx/font_list.h"
+#include "ui/gfx/paint_vector_icon.h"
 #include "ui/gfx/text_utils.h"
+#include "ui/native_theme/native_theme.h"
+#include "ui/views/controls/button/image_button.h"
+#include "ui/views/controls/button/image_button_factory.h"
 #include "ui/views/controls/button/label_button_border.h"
 #include "ui/views/controls/button/menu_button.h"
 #include "ui/views/controls/button/radio_button.h"
@@ -172,7 +179,7 @@ ContentSettingBubbleContents::ContentSettingBubbleContents(
       custom_link_(nullptr),
       manage_link_(nullptr),
       manage_checkbox_(nullptr),
-      learn_more_link_(nullptr) {
+      learn_more_button_(nullptr) {
   // Compensate for built-in vertical padding in the anchor view's image.
   set_anchor_view_insets(gfx::Insets(
       GetLayoutConstant(LOCATION_BAR_BUBBLE_ANCHOR_VERTICAL_INSET), 0));
@@ -202,6 +209,13 @@ gfx::Size ContentSettingBubbleContents::CalculatePreferredSize() const {
   return preferred_size;
 }
 
+void ContentSettingBubbleContents::OnNativeThemeChanged(
+    const ui::NativeTheme* theme) {
+  views::BubbleDialogDelegateView::OnNativeThemeChanged(theme);
+  if (learn_more_button_)
+    StyleLearnMoreButton(theme);
+}
+
 void ContentSettingBubbleContents::Init() {
   using views::GridLayout;
 
@@ -253,14 +267,6 @@ void ContentSettingBubbleContents::Init() {
     bubble_content_empty = false;
   }
 
-  if (!bubble_content.learn_more_link.empty()) {
-    learn_more_link_ = new views::Link(bubble_content.learn_more_link);
-    learn_more_link_->set_listener(this);
-    learn_more_link_->SetHorizontalAlignment(gfx::ALIGN_LEFT);
-    layout->AddView(learn_more_link_);
-    bubble_content_empty = false;
-  }
-
   // Layout for the item list (blocked plugins and popups).
   if (!bubble_content.list_items.empty()) {
     const int kItemListColumnSetId = 2;
@@ -297,10 +303,10 @@ void ContentSettingBubbleContents::Init() {
     }
   }
 
-  const int indented_kSingleColumnSetId = 3;
+  const int kIndentedSingleColumnSetId = 3;
   // Insert a column set with greater indent.
   views::ColumnSet* indented_single_column_set =
-      layout->AddColumnSet(indented_kSingleColumnSetId);
+      layout->AddColumnSet(kIndentedSingleColumnSetId);
   indented_single_column_set->AddPaddingColumn(
       0, provider->GetDistanceMetric(DISTANCE_SUBSECTION_HORIZONTAL_INDENT));
   indented_single_column_set->AddColumn(GridLayout::LEADING, GridLayout::FILL,
@@ -318,7 +324,7 @@ void ContentSettingBubbleContents::Init() {
       radio->SetEnabled(bubble_content.radio_group_enabled);
       radio->set_listener(this);
       radio_group_.push_back(radio);
-      layout->StartRow(0, indented_kSingleColumnSetId);
+      layout->StartRow(0, kIndentedSingleColumnSetId);
       layout->AddView(radio);
       bubble_content_empty = false;
     }
@@ -380,7 +386,7 @@ void ContentSettingBubbleContents::Init() {
     layout->AddView(section_title, 1, 1, GridLayout::FILL, GridLayout::LEADING);
     for (std::set<std::string>::const_iterator j = i->hosts.begin();
          j != i->hosts.end(); ++j) {
-      layout->StartRow(0, indented_kSingleColumnSetId);
+      layout->StartRow(0, kIndentedSingleColumnSetId);
       // TODO(tapted): Verify this when we have a mock. http://crbug.com/700196.
       layout->AddView(new views::Label(
           base::UTF8ToUTF16(*j), CONTEXT_BODY_TEXT_LARGE, STYLE_EMPHASIZED));
@@ -399,12 +405,17 @@ void ContentSettingBubbleContents::Init() {
     bubble_content_empty = false;
   }
 
+  layout->AddPaddingRow(0, related_control_vertical_spacing);
   if (bubble_content.show_manage_text_as_checkbox) {
+    layout->StartRow(0, kIndentedSingleColumnSetId);
     manage_checkbox_ = new views::Checkbox(bubble_content.manage_text);
     manage_checkbox_->set_listener(this);
-    layout->AddPaddingRow(0, related_control_vertical_spacing);
-    layout->StartRow(0, indented_kSingleColumnSetId);
     layout->AddView(manage_checkbox_);
+  } else {
+    layout->StartRow(0, kSingleColumnSetId);
+    manage_link_ = new views::Link(bubble_content.manage_text);
+    manage_link_->set_listener(this);
+    layout->AddView(manage_link_);
   }
 
   if (!bubble_content_empty) {
@@ -419,15 +430,15 @@ void ContentSettingBubbleContents::Init() {
 }
 
 views::View* ContentSettingBubbleContents::CreateExtraView() {
-  const ContentSettingBubbleModel::BubbleContent& bubble_content =
-      content_setting_bubble_model_->bubble_content();
-  // Added as part of the primary view.
-  if (bubble_content.show_manage_text_as_checkbox)
+  // Optionally add a help icon if the view wants to link to a help page.
+  if (!content_setting_bubble_model_->bubble_content().show_learn_more)
     return nullptr;
 
-  manage_link_ = new views::Link(bubble_content.manage_text);
-  manage_link_->set_listener(this);
-  return manage_link_;
+  learn_more_button_ = views::CreateVectorImageButton(this);
+  learn_more_button_->SetFocusForPlatform();
+  learn_more_button_->SetTooltipText(l10n_util::GetStringUTF16(IDS_LEARN_MORE));
+  StyleLearnMoreButton(GetNativeTheme());
+  return learn_more_button_;
 }
 
 bool ContentSettingBubbleContents::Accept() {
@@ -452,6 +463,18 @@ base::string16 ContentSettingBubbleContents::GetDialogButtonLabel(
   return l10n_util::GetStringUTF16(IDS_DONE);
 }
 
+void ContentSettingBubbleContents::StyleLearnMoreButton(
+    const ui::NativeTheme* theme) {
+  DCHECK(learn_more_button_);
+  SkColor text_color =
+      theme->GetSystemColor(ui::NativeTheme::kColorId_LabelEnabledColor);
+  learn_more_button_->SetImage(
+      views::CustomButton::STATE_NORMAL,
+      gfx::CreateVectorIcon(vector_icons::kHelpOutlineIcon, 16,
+                            color_utils::DeriveDefaultIconColor(text_color)));
+  learn_more_button_->set_ink_drop_base_color(text_color);
+}
+
 void ContentSettingBubbleContents::DidFinishNavigation(
     content::NavigationHandle* navigation_handle) {
   if (!navigation_handle->IsInMainFrame() || !navigation_handle->HasCommitted())
@@ -464,13 +487,16 @@ void ContentSettingBubbleContents::DidFinishNavigation(
 
 void ContentSettingBubbleContents::ButtonPressed(views::Button* sender,
                                                  const ui::Event& event) {
-  if (manage_checkbox_ == sender) {
+  if (sender == manage_checkbox_) {
     content_setting_bubble_model_->OnManageCheckboxChecked(
         manage_checkbox_->checked());
 
     // Toggling the check state may change the dialog button text.
     GetDialogClientView()->UpdateDialogButtons();
     GetDialogClientView()->Layout();
+  } else if (sender == learn_more_button_) {
+    content_setting_bubble_model_->OnLearnMoreClicked();
+    GetWidget()->Close();
   } else {
     RadioGroup::const_iterator i(
         std::find(radio_group_.begin(), radio_group_.end(), sender));
@@ -481,11 +507,6 @@ void ContentSettingBubbleContents::ButtonPressed(views::Button* sender,
 
 void ContentSettingBubbleContents::LinkClicked(views::Link* source,
                                                int event_flags) {
-  if (source == learn_more_link_) {
-    content_setting_bubble_model_->OnLearnMoreLinkClicked();
-    GetWidget()->Close();
-    return;
-  }
   if (source == custom_link_) {
     content_setting_bubble_model_->OnCustomLinkClicked();
     GetWidget()->Close();

chrome/browser/ui/views/content_setting_bubble_contents.h

@@ -28,6 +28,7 @@ class ContentSettingBubbleViewsBridge;
 }
 
 namespace views {
+class ImageButton;
 class RadioButton;
 class LabelButton;
 }
@@ -54,6 +55,7 @@ class ContentSettingBubbleContents : public content::WebContentsObserver,
       views::BubbleBorder::Arrow arrow);
   ~ContentSettingBubbleContents() override;
 
+  // views::BubbleDialogDelegateView:
   gfx::Size CalculatePreferredSize() const override;
 
  protected:
@@ -64,6 +66,7 @@ class ContentSettingBubbleContents : public content::WebContentsObserver,
   bool Close() override;
   int GetDialogButtons() const override;
   base::string16 GetDialogButtonLabel(ui::DialogButton button) const override;
+  void OnNativeThemeChanged(const ui::NativeTheme* theme) override;
 
  private:
   // A combobox model that builds the contents of the media capture devices menu
@@ -94,6 +97,9 @@ class ContentSettingBubbleContents : public content::WebContentsObserver,
 
   typedef std::map<views::Link*, int> ListItemLinks;
 
+  // Applies the colors appropriate for |theme| to the learn more button.
+  void StyleLearnMoreButton(const ui::NativeTheme* theme);
+
   // content::WebContentsObserver:
   void DidFinishNavigation(
       content::NavigationHandle* navigation_handle) override;
@@ -119,7 +125,7 @@ class ContentSettingBubbleContents : public content::WebContentsObserver,
   views::Link* manage_link_;
   views::LabelButton* manage_button_;
   views::Checkbox* manage_checkbox_;
-  views::Link* learn_more_link_;
+  views::ImageButton* learn_more_button_;
 
   // Combobox models the bubble owns.
   std::list<MediaComboboxModel> combobox_models_;

components/vector_icons/BUILD.gn

@@ -19,6 +19,7 @@ aggregate_vector_icons("components_vector_icons") {
     "error_circle.icon",
     "forward_arrow.1x.icon",
     "forward_arrow.icon",
+    "help_outline.icon",
     "info_outline.icon",
     "location_on.icon",
     "lock.icon",

components/vector_icons/help_outline.icon

+// Copyright 2017 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.
+
+CANVAS_DIMENSIONS, 24,
+MOVE_TO, 11, 18,
+R_H_LINE_TO, 2,
+R_V_LINE_TO, -2,
+R_H_LINE_TO, -2,
+R_V_LINE_TO, 2,
+CLOSE,
+R_MOVE_TO, 1, -16,
+CUBIC_TO, 6.48f, 2, 2, 6.48f, 2, 12,
+R_CUBIC_TO, 0, 5.52f, 4.48f, 10, 10, 10,
+R_CUBIC_TO, 5.52f, 0, 10, -4.48f, 10, -10,
+CUBIC_TO_SHORTHAND, 17.52f, 2, 12, 2,
+CLOSE,
+R_MOVE_TO, 0, 18,
+R_CUBIC_TO, -4.41f, 0, -8, -3.59f, -8, -8,
+R_CUBIC_TO, 0, -4.41f, 3.59f, -8, 8, -8,
+R_CUBIC_TO, 4.41f, 0, 8, 3.59f, 8, 8,
+R_CUBIC_TO, 0, 4.41f, -3.59f, 8, -8, 8,
+CLOSE,
+R_MOVE_TO, 0, -14,
+R_CUBIC_TO, -2.21f, 0, -4, 1.79f, -4, 4,
+R_H_LINE_TO, 2,
+R_CUBIC_TO, 0, -1.1f, 0.9f, -2, 2, -2,
+R_CUBIC_TO, 1.1f, 0, 2, 0.9f, 2, 2,
+R_CUBIC_TO, 0, 2, -3, 1.75f, -3, 5,
+R_H_LINE_TO, 2,
+R_CUBIC_TO, 0, -2.25f, 3, -2.5f, 3, -5,
+R_CUBIC_TO, 0, -2.21f, -1.79f, -4, -4, -4,
+CLOSE,
+END