Page Info (Views): Change "Details" link (sec. panel) to "Learn more" (help center).

chrome/browser/ui/views/website_settings/website_settings_popup_view.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/website_settings/website_settings_popup_view.h"
 
 #include <stddef.h>
 
 #include <algorithm>
 #include <vector>
 
 #include "base/i18n/rtl.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/strings/string16.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/certificate_viewer.h"
 #include "chrome/browser/devtools/devtools_toggle_action.h"

[msw, 2016/11/15 01:56:07]

nit: remove devtools includes

[lgarron, 2016/11/29 02:12:02]

Done.

 #include "chrome/browser/devtools/devtools_window.h"
 #include "chrome/browser/infobars/infobar_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_dialogs.h"
 #include "chrome/browser/ui/layout_constants.h"
 #include "chrome/browser/ui/views/collected_cookies_views.h"
 #include "chrome/browser/ui/views/website_settings/chosen_object_row.h"
 #include "chrome/browser/ui/views/website_settings/permission_selector_row.h"
 #include "chrome/browser/ui/website_settings/website_settings.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/grit/chromium_strings.h"
 #include "chrome/grit/generated_resources.h"
 #include "chrome/grit/theme_resources.h"
 #include "components/content_settings/core/common/content_settings_types.h"
 #include "components/prefs/pref_service.h"

[msw, 2016/11/15 01:56:07]

nit: remove prefs includes

[lgarron, 2016/11/29 02:12:02]

Done.

 #include "components/strings/grit/components_chromium_strings.h"
 #include "components/strings/grit/components_strings.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/user_metrics.h"
 #include "extensions/common/constants.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/material_design/material_design_controller.h"
 #include "ui/base/models/simple_menu_model.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/gfx/canvas.h"
@@ skipping 76 matching lines @@
 class PopupHeaderView : public views::View {
  public:
   explicit PopupHeaderView(views::ButtonListener* button_listener,
                            views::StyledLabelListener* styled_label_listener);
   ~PopupHeaderView() override;
 
   // Sets the security summary for the current page.
   void SetSummary(const base::string16& summary_text);
 
   // Sets the security details for the current page.
-  void SetDetails(const base::string16& details_text,
-                  bool include_details_link);
+  void SetDetails(const base::string16& details_text);
 
   void AddResetDecisionsLabel();
 
  private:
   // The listener for the styled labels in this view.
   views::StyledLabelListener* styled_label_listener_;
 
   // The label that displays security summary for the current page.
   views::Label* summary_label_;
 
   // The label that displays the status of the identity check for this site.
-  // Includes a link to open the DevTools Security panel.
+  // Includes a link to open the Chrome Help Center article about connection
+  // security.
   views::StyledLabel* details_label_;
 
   // A container for the styled label with a link for resetting cert decisions.
   // This is only shown sometimes, so we use a container to keep track of
   // where to place it (if needed).
   views::View* reset_decisions_label_container_;
   views::StyledLabel* reset_decisions_label_;
 
   DISALLOW_COPY_AND_ASSIGN(PopupHeaderView);
 };
@@ skipping 109 matching lines @@
 
   layout->AddPaddingRow(1, kHeaderPaddingBottom);
 }
 
 PopupHeaderView::~PopupHeaderView() {}
 
 void PopupHeaderView::SetSummary(const base::string16& summary_text) {
   summary_label_->SetText(summary_text);
 }
 
-void PopupHeaderView::SetDetails(const base::string16& details_text,
-                                 bool include_details_label_link) {
-  if (include_details_label_link) {
-    base::string16 details_link_text =
-        l10n_util::GetStringUTF16(IDS_WEBSITE_SETTINGS_DETAILS_LINK);
+void PopupHeaderView::SetDetails(const base::string16& details_text) {
+  base::string16 learn_more_text =
+      l10n_util::GetStringUTF16(IDS_PAGE_INFO_LEARN_MORE);

[msw, 2016/11/15 01:56:07]

nit: inline below, nix |learn_more_text|

[lgarron, 2016/11/29 02:12:02]

Done.

 
-    std::vector<base::string16> subst;
-    subst.push_back(details_text);
-    subst.push_back(details_link_text);
+  std::vector<base::string16> subst;
+  subst.push_back(details_text);
+  subst.push_back(learn_more_text);
 
-    std::vector<size_t> offsets;
+  std::vector<size_t> offsets;
 
-    base::string16 text = base::ReplaceStringPlaceholders(
-        base::ASCIIToUTF16("$1 $2"), subst, &offsets);
-    details_label_->SetText(text);
-    gfx::Range details_range(offsets[1], text.length());
+  base::string16 text = base::ReplaceStringPlaceholders(
+      base::ASCIIToUTF16("$1 $2"), subst, &offsets);
+  details_label_->SetText(text);
+  gfx::Range details_range(offsets[1], text.length());
 
-    views::StyledLabel::RangeStyleInfo link_style =
-        views::StyledLabel::RangeStyleInfo::CreateForLink();
-    if (!ui::MaterialDesignController::IsSecondaryUiMaterial())
-      link_style.font_style |= gfx::Font::FontStyle::UNDERLINE;
-    link_style.disable_line_wrapping = false;
+  views::StyledLabel::RangeStyleInfo link_style =
+      views::StyledLabel::RangeStyleInfo::CreateForLink();
+  if (!ui::MaterialDesignController::IsSecondaryUiMaterial())
+    link_style.font_style |= gfx::Font::FontStyle::UNDERLINE;
+  link_style.disable_line_wrapping = false;
 
-    details_label_->AddStyleRange(details_range, link_style);
-  } else {
-    details_label_->SetText(details_text);
-  }
+  details_label_->AddStyleRange(details_range, link_style);
 }
 
 void PopupHeaderView::AddResetDecisionsLabel() {
   std::vector<base::string16> subst;
   subst.push_back(
       l10n_util::GetStringUTF16(IDS_PAGEINFO_INVALID_CERTIFICATE_DESCRIPTION));
   subst.push_back(l10n_util::GetStringUTF16(
       IDS_PAGEINFO_RESET_INVALID_CERTIFICATE_DECISIONS_BUTTON));
 
   std::vector<size_t> offsets;
@@ skipping 145 matching lines @@
       profile_(profile),
       header_(nullptr),
       separator_(nullptr),
       site_settings_view_(nullptr),
       cookies_view_(nullptr),
       cookie_dialog_link_(nullptr),
       permissions_view_(nullptr),
       weak_factory_(this) {
   g_shown_popup_type = POPUP_WEBSITE_SETTINGS;
   set_parent_window(parent_window);
   is_devtools_disabled_ =

[msw, 2016/11/15 01:56:07]

nit: remove this.

[lgarron, 2016/11/29 02:12:02]

Done.

       profile->GetPrefs()->GetBoolean(prefs::kDevToolsDisabled);
 
   // 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));
 
   views::GridLayout* layout = new views::GridLayout(this);
   SetLayoutManager(layout);
   const int content_column = 0;
   views::ColumnSet* column_set = layout->AddColumnSet(content_column);
@@ skipping 238 matching lines @@
 
   header_->SetSummary(security_description->summary);
 
   if (identity_info.certificate) {
     certificate_ = identity_info.certificate;
 
     if (identity_info.show_ssl_decision_revoke_button)
       header_->AddResetDecisionsLabel();
   }
 
-  bool include_details_link = !is_devtools_disabled_ || certificate_;
-
-  header_->SetDetails(security_description->details, include_details_link);
+  header_->SetDetails(security_description->details);
 
   Layout();
   SizeToContents();
 }
 
 void WebsiteSettingsPopupView::SetSelectedTab(TabId tab_id) {
   // TODO(lgarron): Remove this method. (https://crbug.com/571533)
 }
 
 views::View* WebsiteSettingsPopupView::CreateSiteSettingsView() {
@@ skipping 39 matching lines @@
     default:
       NOTREACHED();
   }
 }
 
 void WebsiteSettingsPopupView::StyledLabelLinkClicked(views::StyledLabel* label,
                                                       const gfx::Range& range,
                                                       int event_flags) {
   switch (label->id()) {
     case STYLED_LABEL_SECURITY_DETAILS:
+      web_contents()->OpenURL(content::OpenURLParams(
+          GURL(chrome::kPageInfoHelpCenterURL), content::Referrer(),
+          WindowOpenDisposition::NEW_FOREGROUND_TAB, ui::PAGE_TRANSITION_LINK,
+          false));
       presenter_->RecordWebsiteSettingsAction(
-          WebsiteSettings::WEBSITE_SETTINGS_SECURITY_DETAILS_OPENED);

[msw, 2016/11/15 01:56:07]

Deprecate this and WEBSITE_SETTINGS_CERTIFICATE_DIALOG_OPENED if they'll be
removed on Cocoa too.

[lgarron, 2016/11/29 02:12:02]

I've made a separate CL for this: https://codereview.chromium.org/2521173004

-
-      if (is_devtools_disabled_) {
-        DCHECK(certificate_);
-        gfx::NativeWindow parent =
-            anchor_widget() ? anchor_widget()->GetNativeWindow() : nullptr;
-        presenter_->RecordWebsiteSettingsAction(
-            WebsiteSettings::WEBSITE_SETTINGS_CERTIFICATE_DIALOG_OPENED);
-        ShowCertificateViewer(web_contents(), parent, certificate_.get());
-      } else {
-        DevToolsWindow::OpenDevToolsWindow(
-            web_contents(), DevToolsToggleAction::ShowSecurityPanel());
-      }
+          WebsiteSettings::WEBSITE_SETTINGS_CONNECTION_HELP_OPENED);
       break;
     case STYLED_LABEL_RESET_CERTIFICATE_DECISIONS:
       presenter_->OnRevokeSSLErrorBypassButtonPressed();
       GetWidget()->Close();
       break;
     default:
       NOTREACHED();
   }
 }