Skip to content

Commit

Permalink
Update interstitials to match the new security indicators
Browse files Browse the repository at this point in the history
I've gated this change using the Feature API, behind the feature name
SecurityWarningIconUpdate. That way we can monitor the CTR impact and
roll back if necessary.

Squashed pngs using tools/resources/optimize-png-files.sh -o2.

Screenshots on the bug.

BUG=629140

Review-Url: https://codereview.chromium.org/2158113002
Cr-Commit-Position: refs/heads/master@{#406183}
  • Loading branch information
adrifelt authored and Commit bot committed Jul 19, 2016
1 parent c8551e3 commit bd00bd1
Show file tree
Hide file tree
Showing 12 changed files with 41 additions and 2 deletions.
3 changes: 3 additions & 0 deletions chrome/browser/safe_browsing/safe_browsing_blocking_page.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
#include "chrome/grit/locale_settings.h"
#include "components/google/core/browser/google_util.h"
#include "components/prefs/pref_service.h"
#include "components/security_interstitials/core/common_string_util.h"
#include "components/security_interstitials/core/controller_client.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/interstitial_page.h"
Expand Down Expand Up @@ -639,6 +640,8 @@ void SafeBrowsingBlockingPage::PopulateInterstitialStrings(
load_time_data->SetBoolean(
"overridable",
!IsPrefEnabled(prefs::kSafeBrowsingProceedAnywayDisabled));
security_interstitials::common_string_util::PopulateNewIconStrings(
load_time_data);

switch (interstitial_reason_) {
case SB_REASON_MALWARE:
Expand Down
3 changes: 3 additions & 0 deletions chrome/browser/ssl/captive_portal_blocking_page.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "chrome/browser/ssl/ssl_cert_reporter.h"
#include "components/captive_portal/captive_portal_detector.h"
#include "components/certificate_reporting/error_reporter.h"
#include "components/security_interstitials/core/common_string_util.h"
#include "components/security_interstitials/core/controller_client.h"
#include "components/url_formatter/url_formatter.h"
#include "components/wifi/wifi_service.h"
Expand Down Expand Up @@ -119,6 +120,8 @@ void CaptivePortalBlockingPage::PopulateInterstitialStrings(
load_time_data->SetString("iconClass", "icon-offline");
load_time_data->SetString("type", "CAPTIVE_PORTAL");
load_time_data->SetBoolean("overridable", false);
security_interstitials::common_string_util::PopulateNewIconStrings(
load_time_data);

// |IsWifiConnection| isn't accurate on some platforms, so always try to get
// the Wi-Fi SSID even if |IsWifiConnection| is false.
Expand Down
1 change: 1 addition & 0 deletions components/security_interstitials/core/bad_clock_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ void BadClockUI::PopulateStringsForHTML(base::DictionaryValue* load_time_data) {
common_string_util::PopulateSSLLayoutStrings(cert_error_, load_time_data);
common_string_util::PopulateSSLDebuggingStrings(ssl_info_, time_triggered_,
load_time_data);
common_string_util::PopulateNewIconStrings(load_time_data);

// Clock-specific strings.
PopulateClockStrings(load_time_data);
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,13 @@ input[type=checkbox]:focus ~ .checkbox {
display: none;
}

.safe-browsing .icon {
.safe-browsing .new-icons {
background-image: -webkit-image-set(
url(images/1x/triangle_white.png) 1x,
url(images/2x/triangle_white.png) 2x);
}

.safe-browsing .old-icons {
background-image: -webkit-image-set(
url(images/1x/stop_sign.png) 1x,
url(images/2x/stop_sign.png) 2x);
Expand All @@ -204,7 +210,13 @@ input[type=checkbox]:focus ~ .checkbox {
font-size: .875em;
}

.ssl .icon {
.ssl .new-icons {
background-image: -webkit-image-set(
url(images/1x/triangle_red.png) 1x,
url(images/2x/triangle_red.png) 2x);
}

.ssl .old-icons {
background-image: -webkit-image-set(
url(images/1x/brokenssl_red.png) 1x,
url(images/2x/brokenssl_red.png) 2x);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,11 @@ function setupEvents() {
$('body').classList.add('safe-browsing');
}

if (loadTimeData.getBoolean('iconUpdate') === true)
$('icon').classList.add('new-icons');
else
$('icon').classList.add('old-icons');

if (hidePrimaryButton) {
$('primary-button').classList.add('hidden');
} else {
Expand Down
11 changes: 11 additions & 0 deletions components/security_interstitials/core/common_string_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "components/security_interstitials/core/common_string_util.h"

#include "base/feature_list.h"
#include "base/i18n/rtl.h"
#include "base/i18n/time_formatting.h"
#include "base/strings/string_util.h"
Expand All @@ -12,6 +13,11 @@
#include "net/base/net_errors.h"
#include "ui/base/l10n/l10n_util.h"

namespace {
const base::Feature kSecurityWarningIconUpdate{
"SecurityWarningIconUpdate", base::FEATURE_DISABLED_BY_DEFAULT};
} // namespace

namespace security_interstitials {

namespace common_string_util {
Expand Down Expand Up @@ -50,6 +56,11 @@ void PopulateSSLDebuggingStrings(const net::SSLInfo ssl_info,
"pem", base::JoinString(encoded_chain, base::StringPiece()));
}

void PopulateNewIconStrings(base::DictionaryValue* load_time_data) {
load_time_data->SetBoolean(
"iconUpdate", base::FeatureList::IsEnabled(kSecurityWarningIconUpdate));
}

} // namespace common_string_util

} // namespace security_interstitials
3 changes: 3 additions & 0 deletions components/security_interstitials/core/common_string_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ void PopulateSSLDebuggingStrings(const net::SSLInfo ssl_info,
const base::Time time_triggered,
base::DictionaryValue* load_time_data);

// For determining whether to use the old or new icon sets.
void PopulateNewIconStrings(base::DictionaryValue* load_time_data);

} // common_string_util

} // namespace security_interstitials
Expand Down
1 change: 1 addition & 0 deletions components/security_interstitials/core/ssl_error_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ void SSLErrorUI::PopulateStringsForHTML(base::DictionaryValue* load_time_data) {
common_string_util::PopulateSSLLayoutStrings(cert_error_, load_time_data);
common_string_util::PopulateSSLDebuggingStrings(ssl_info_, time_triggered_,
load_time_data);
common_string_util::PopulateNewIconStrings(load_time_data);

// Shared values for both the overridable and non-overridable versions.
load_time_data->SetBoolean("bad_clock", false);
Expand Down

0 comments on commit bd00bd1

Please sign in to comment.