Make 'Load full site' a button instead of link.

chrome/browser/ui/views/content_setting_bubble_contents.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 "chrome/browser/ui/views/content_setting_bubble_contents.h"
 
 #include <algorithm>
 #include <set>
 #include <string>
 #include <vector>
@@ skipping 11 matching lines @@
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "components/strings/grit/components_strings.h"
 #include "content/public/browser/plugin_service.h"
 #include "content/public/browser/web_contents.h"
 #include "ui/base/cursor/cursor.h"
 #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/font_list.h"
 #include "ui/gfx/text_utils.h"
+#include "ui/views/controls/button/label_button.h"

[msw, 2016/09/14 17:01:34]

nit: This include is redundant

[melandory, 2016/09/14 19:30:12]

Done.

+#include "ui/views/controls/button/md_text_button.h"
 #include "ui/views/controls/button/menu_button.h"
 #include "ui/views/controls/button/radio_button.h"
 #include "ui/views/controls/combobox/combobox.h"
 #include "ui/views/controls/image_view.h"
 #include "ui/views/controls/label.h"
 #include "ui/views/controls/link.h"
 #include "ui/views/controls/menu/menu_config.h"
 #include "ui/views/controls/menu/menu_runner.h"
 #include "ui/views/controls/separator.h"
 #include "ui/views/layout/grid_layout.h"
@@ skipping 112 matching lines @@
 ContentSettingBubbleContents::ContentSettingBubbleContents(
     ContentSettingBubbleModel* content_setting_bubble_model,
     content::WebContents* web_contents,
     views::View* anchor_view,
     views::BubbleBorder::Arrow arrow)
     : content::WebContentsObserver(web_contents),
       BubbleDialogDelegateView(anchor_view, arrow),
       content_setting_bubble_model_(content_setting_bubble_model),
       custom_link_(NULL),
       manage_link_(NULL),
+      manage_button_(NULL),

[msw, 2016/09/14 17:01:34]

nit: nullptr here and elsewhere

[melandory, 2016/09/14 19:30:12]

Done.

       learn_more_link_(NULL) {
   // 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));
 }
 
 ContentSettingBubbleContents::~ContentSettingBubbleContents() {
   // Must remove the children here so the comboboxes get destroyed before
   // their associated models.
   RemoveAllChildViews(true);
@@ skipping 200 matching lines @@
   if (!bubble_content_empty) {
     layout->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
     layout->StartRow(0, kSingleColumnSetId);
     layout->AddView(new views::Separator(views::Separator::HORIZONTAL), 1, 1,
                     GridLayout::FILL, GridLayout::FILL);
     layout->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
   }
 }
 
 views::View* ContentSettingBubbleContents::CreateExtraView() {
-  manage_link_ = new views::Link(base::UTF8ToUTF16(
-      content_setting_bubble_model_->bubble_content().manage_link));
-  manage_link_->set_listener(this);
-  return manage_link_;
+  if (content_setting_bubble_model_->bubble_content().manage_link_as_button) {

[msw, 2016/09/14 17:01:34]

Why make this an option? Why not just always use a button?

[melandory, 2016/09/14 19:30:12]

It was set as a requirement for our particular bubble, that it "manage link"
should be button, because it effectively reloads the page. For other content
setting the link might be more appropriate, because clicking on it opens setting
page.
Although it looks like button on OSX. So I don't really have strong opinion. I
can ask our UX designer to raise this point and maybe change them all to be
buttons.
For now I prefer to distinguish link and button and do not wait until decision
about all bubbles is made. 

[msw, 2016/09/14 19:50:24]

Acknowledged.

+    manage_button_ =  views::MdTextButton::CreateSecondaryUiButton(
+              this, base::UTF8ToUTF16(
+                  content_setting_bubble_model_->bubble_content().manage_link));

[msw, 2016/09/14 17:01:34]

nit: rename |BubbleContent::manage_link| to |manage_text| or similar.

[melandory, 2016/09/14 19:30:12]

Done.

+    return manage_button_;
+  } else {
+    manage_link_ = new views::Link(base::UTF8ToUTF16(
+        content_setting_bubble_model_->bubble_content().manage_link));
+    manage_link_->set_listener(this);
+    return manage_link_;
+  }
 }
 
 bool ContentSettingBubbleContents::Accept() {
   content_setting_bubble_model_->OnDoneClicked();
   return true;
 }
 
 bool ContentSettingBubbleContents::Close() {
   return true;
 }
@@ skipping 10 matching lines @@
 void ContentSettingBubbleContents::DidNavigateMainFrame(
     const content::LoadCommittedDetails& details,
     const content::FrameNavigateParams& params) {
   // Content settings are based on the main frame, so if it switches then
   // close up shop.
   GetWidget()->Close();
 }
 
 void ContentSettingBubbleContents::ButtonPressed(views::Button* sender,
                                                  const ui::Event& event) {
-  RadioGroup::const_iterator i(
-      std::find(radio_group_.begin(), radio_group_.end(), sender));
-  DCHECK(i != radio_group_.end());
-  content_setting_bubble_model_->OnRadioClicked(i - radio_group_.begin());
+  if (manage_button_ == sender) {
+    GetWidget()->Close();
+    content_setting_bubble_model_->OnManageLinkClicked();
+  } else {
+    RadioGroup::const_iterator i(
+        std::find(radio_group_.begin(), radio_group_.end(), sender));
+    DCHECK(i != radio_group_.end());
+    content_setting_bubble_model_->OnRadioClicked(i - radio_group_.begin());
+  }
 }
 
 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_) {
@@ skipping 13 matching lines @@
   DCHECK(i != list_item_links_.end());
   content_setting_bubble_model_->OnListItemClicked(i->second);
 }
 
 void ContentSettingBubbleContents::OnPerformAction(views::Combobox* combobox) {
   MediaComboboxModel* model =
       static_cast<MediaComboboxModel*>(combobox->model());
   content_setting_bubble_model_->OnMediaMenuClicked(
       model->type(), model->GetDevices()[combobox->selected_index()].id);
 }