Skip to content

Commit

Permalink
Implement red interstitial facelift color and icon changes
Browse files Browse the repository at this point in the history
This first CL for implementing the red interstitial facelift changes the
color scheme to the GM3 standard error/primary colors. It also
implements the main interstitial page icon and Desktop omnibox/page info
icon changes behind the feature flag.

Bug: 1469647
Change-Id: Ib2e7b5e8970eb47160e87ad3948acda0abde0e70
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4799310
Reviewed-by: Daniel Rubery <drubery@chromium.org>
Reviewed-by: Christian Dullweber <dullweber@chromium.org>
Commit-Queue: Sarah Krakowiak <skrakowi@chromium.org>
Reviewed-by: Moe Ahmadi <mahmadi@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1188300}
  • Loading branch information
Sarah Criel authored and Chromium LUCI CQ committed Aug 25, 2023
1 parent 6625c11 commit 5fcb41f
Show file tree
Hide file tree
Showing 18 changed files with 131 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "chrome/browser/ui/views/chrome_layout_provider.h"
#include "chrome/browser/ui/views/chrome_typography.h"
#include "chrome/browser/ui/views/page_info/page_info_view_factory.h"
#include "components/safe_browsing/core/common/features.h"
#include "components/strings/grit/components_strings.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/base/ui_base_features.h"
Expand Down Expand Up @@ -38,7 +39,19 @@ void PageInfoSecurityContentView::SetIdentityInfo(
security_description_type_ = security_description->type;

if (security_description->summary_style == SecuritySummaryColor::RED) {
security_view_->SetIcon(PageInfoViewFactory::GetConnectionNotSecureIcon());
if (base::FeatureList::IsEnabled(safe_browsing::kRedInterstitialFacelift) &&
(identity_info.safe_browsing_status ==
PageInfo::SAFE_BROWSING_STATUS_MALWARE ||
identity_info.safe_browsing_status ==
PageInfo::SAFE_BROWSING_STATUS_SOCIAL_ENGINEERING ||
identity_info.safe_browsing_status ==
PageInfo::SAFE_BROWSING_STATUS_UNWANTED_SOFTWARE)) {
security_view_->SetIcon(
PageInfoViewFactory::GetConnectionDangerousIcon());
} else {
security_view_->SetIcon(
PageInfoViewFactory::GetConnectionNotSecureIcon());
}
security_view_->SetSummary(security_description->summary, STYLE_RED);
} else {
security_view_->SetIcon(PageInfoViewFactory::GetConnectionSecureIcon());
Expand Down
6 changes: 6 additions & 0 deletions chrome/browser/ui/views/page_info/page_info_view_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -594,6 +594,12 @@ const ui::ImageModel PageInfoViewFactory::GetConnectionNotSecureIcon() {
GetIconSize());
}

// static
const ui::ImageModel PageInfoViewFactory::GetConnectionDangerousIcon() {
return ui::ImageModel::FromVectorIcon(
vector_icons::kDangerousIcon, ui::kColorAlertHighSeverity, GetIconSize());
}

// static
const ui::ImageModel PageInfoViewFactory::GetConnectionSecureIcon() {
return GetImageModel(features::IsChromeRefresh2023()
Expand Down
3 changes: 3 additions & 0 deletions chrome/browser/ui/views/page_info/page_info_view_factory.h
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,9 @@ class PageInfoViewFactory {
// Returns the not secure state icon for the SecurityInformationView.
static const ui::ImageModel GetConnectionNotSecureIcon();

// Returns the dangerous icon for the SecurityInformationView.
static const ui::ImageModel GetConnectionDangerousIcon();

// Returns the icon for the secure connection button.
static const ui::ImageModel GetConnectionSecureIcon();

Expand Down
2 changes: 2 additions & 0 deletions components/omnibox/browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,7 @@ static_library("location_bar") {
":buildflags",
"//build:chromeos_buildflags",
"//components/dom_distiller/core:core",
"//components/safe_browsing/core/common:common",
"//components/search_engines",
"//components/strings",
"//components/url_formatter",
Expand Down Expand Up @@ -842,6 +843,7 @@ source_set("unit_tests") {
"//components/optimization_guide/core:test_support",
"//components/optimization_guide/proto:optimization_guide_proto",
"//components/prefs:test_support",
"//components/safe_browsing/core/common:common",
"//components/search",
"//components/search_engines",
"//components/search_engines:test_support",
Expand Down
1 change: 1 addition & 0 deletions components/omnibox/browser/DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ include_rules = [
"+components/prefs",
"+components/query_parser",
"+components/query_tiles",
"+components/safe_browsing/core/common",
"+components/search",
"+components/search_engines",
"+components/security_state",
Expand Down
3 changes: 2 additions & 1 deletion components/omnibox/browser/location_bar_model_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,8 @@ const gfx::VectorIcon& LocationBarModelImpl::GetVectorIcon() const {

return location_bar_model::GetSecurityVectorIcon(
GetSecurityLevel(),
delegate_->ShouldUseUpdatedConnectionSecurityIndicators());
delegate_->ShouldUseUpdatedConnectionSecurityIndicators(),
delegate_->GetVisibleSecurityState()->malicious_content_status);
}

std::u16string LocationBarModelImpl::GetSecureDisplayText() const {
Expand Down
16 changes: 15 additions & 1 deletion components/omnibox/browser/location_bar_model_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "build/build_config.h"
#include "components/omnibox/browser/buildflags.h"
#include "components/omnibox/common/omnibox_features.h"
#include "components/safe_browsing/core/common/features.h"
#include "ui/base/ui_base_features.h"
#include "ui/gfx/vector_icon_types.h"

Expand All @@ -21,7 +22,8 @@ namespace location_bar_model {

const gfx::VectorIcon& GetSecurityVectorIcon(
security_state::SecurityLevel security_level,
bool use_updated_connection_security_indicators) {
bool use_updated_connection_security_indicators,
security_state::MaliciousContentStatus malicious_content_status) {
#if (!BUILDFLAG(IS_ANDROID) || BUILDFLAG(ENABLE_VR)) && !BUILDFLAG(IS_IOS)
switch (security_level) {
case security_state::NONE:
Expand All @@ -36,10 +38,22 @@ const gfx::VectorIcon& GetSecurityVectorIcon(
? vector_icons::kBusinessChromeRefreshIcon
: vector_icons::kBusinessIcon;
case security_state::WARNING:
return IsChromeRefreshIconsEnabled()
? vector_icons::kNotSecureWarningChromeRefreshIcon
: vector_icons::kNotSecureWarningIcon;
case security_state::DANGEROUS:
if (base::FeatureList::IsEnabled(
safe_browsing::kRedInterstitialFacelift) &&
malicious_content_status !=
security_state::MALICIOUS_CONTENT_STATUS_BILLING) {
return IsChromeRefreshIconsEnabled()
? vector_icons::kDangerousChromeRefreshIcon
: vector_icons::kDangerousIcon;
}
return IsChromeRefreshIconsEnabled()
? vector_icons::kNotSecureWarningChromeRefreshIcon
: vector_icons::kNotSecureWarningIcon;

case security_state::SECURITY_LEVEL_COUNT:
NOTREACHED();
return IsChromeRefreshIconsEnabled() ? omnibox::kHttpChromeRefreshIcon
Expand Down
3 changes: 2 additions & 1 deletion components/omnibox/browser/location_bar_model_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ namespace location_bar_model {
// indicates a secure connection.
const gfx::VectorIcon& GetSecurityVectorIcon(
security_state::SecurityLevel security_level,
bool use_updated_connection_security_indicators);
bool use_updated_connection_security_indicators,
security_state::MaliciousContentStatus malicious_content_status);

// Helper function to enable the omnibox chrome refresh icons based on the
// feature flags turned on. This is a duplicate of
Expand Down
35 changes: 30 additions & 5 deletions components/omnibox/browser/location_bar_model_util_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,43 +4,68 @@

#include "components/omnibox/browser/location_bar_model_util.h"

#include "base/test/scoped_feature_list.h"
#include "components/omnibox/browser/vector_icons.h"
#include "components/safe_browsing/core/common/features.h"
#include "components/vector_icons/vector_icons.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "ui/gfx/vector_icon_types.h"

TEST(LocationBarModelUtilTest, GetSecurityVectorIconWithNoneLevel) {
const gfx::VectorIcon& icon = location_bar_model::GetSecurityVectorIcon(
security_state::SecurityLevel::NONE,
/*use_updated_connection_security_indicators=*/false);
/*use_updated_connection_security_indicators=*/false,
/*malicious_content_status=*/
security_state::MALICIOUS_CONTENT_STATUS_NONE);
EXPECT_EQ(icon.name, omnibox::kHttpIcon.name);
}

TEST(LocationBarModelUtilTest, GetSecurityVectorIconWithSecureLevel) {
const gfx::VectorIcon& icon = location_bar_model::GetSecurityVectorIcon(
security_state::SecurityLevel::SECURE,
/*use_updated_connection_security_indicators=*/false);
/*use_updated_connection_security_indicators=*/false,
/*malicious_content_status=*/
security_state::MALICIOUS_CONTENT_STATUS_NONE);
EXPECT_EQ(icon.name, vector_icons::kHttpsValidIcon.name);
}

TEST(LocationBarModelUtilTest,
GetSecurityVectorIconWithSecureWithPolicyInstalledCertLevel) {
const gfx::VectorIcon& icon = location_bar_model::GetSecurityVectorIcon(
security_state::SecurityLevel::SECURE_WITH_POLICY_INSTALLED_CERT,
/*use_updated_connection_security_indicators=*/false);
/*use_updated_connection_security_indicators=*/false,
/*malicious_content_status=*/
security_state::MALICIOUS_CONTENT_STATUS_NONE);
EXPECT_EQ(icon.name, vector_icons::kBusinessIcon.name);
}

TEST(LocationBarModelUtilTest, GetSecurityVectorIconWithDangerousLevel) {
base::test::ScopedFeatureList scoped_feature_list_;
scoped_feature_list_.InitAndEnableFeature(
safe_browsing::kRedInterstitialFacelift);
const gfx::VectorIcon& icon = location_bar_model::GetSecurityVectorIcon(
security_state::SecurityLevel::DANGEROUS,
/*use_updated_connection_security_indicators=*/false);
/*use_updated_connection_security_indicators=*/false,
/*malicious_content_status=*/
security_state::MALICIOUS_CONTENT_STATUS_SOCIAL_ENGINEERING);
EXPECT_EQ(icon.name, vector_icons::kDangerousIcon.name);
}

TEST(LocationBarModelUtilTest,
GetSecurityVectorIconBillingInterstitialWithDangerousLevel) {
const gfx::VectorIcon& icon = location_bar_model::GetSecurityVectorIcon(
security_state::SecurityLevel::DANGEROUS,
/*use_updated_connection_security_indicators=*/false,
/*malicious_content_status=*/
security_state::MALICIOUS_CONTENT_STATUS_BILLING);
EXPECT_EQ(icon.name, vector_icons::kNotSecureWarningIcon.name);
}

TEST(LocationBarModelUtilTest, GetSecurityVectorIconWithWarningLevel) {
const gfx::VectorIcon& icon = location_bar_model::GetSecurityVectorIcon(
security_state::SecurityLevel::WARNING,
/*use_updated_connection_security_indicators=*/false);
/*use_updated_connection_security_indicators=*/false,
/*malicious_content_status=*/
security_state::MALICIOUS_CONTENT_STATUS_SOCIAL_ENGINEERING);
EXPECT_EQ(icon.name, vector_icons::kNotSecureWarningIcon.name);
}
5 changes: 5 additions & 0 deletions components/safe_browsing/core/common/features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,10 @@ BASE_FEATURE(kRealTimeUrlFilteringForEnterprise,
"RealTimeUrlFilteringForEnterprise",
base::FEATURE_ENABLED_BY_DEFAULT);

BASE_FEATURE(kRedInterstitialFacelift,
"RedInterstitialFacelift",
base::FEATURE_DISABLED_BY_DEFAULT);

BASE_FEATURE(kReferrerChainParameters,
"SafeBrowsingReferrerChainParameters",
base::FEATURE_DISABLED_BY_DEFAULT);
Expand Down Expand Up @@ -391,6 +395,7 @@ constexpr struct {
{&kMmapSafeBrowsingDatabase, true},
{&kNestedArchives, true},
{&kRealTimeUrlFilteringForEnterprise, true},
{&kRedInterstitialFacelift, false},
{&kSafeBrowsingCsbrrNewDownloadTrigger, true},
{&kSafeBrowsingLookupMechanismExperiment, true},
{&kSafeBrowsingRemoveCookiesInAuthRequests, true},
Expand Down
3 changes: 3 additions & 0 deletions components/safe_browsing/core/common/features.h
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,9 @@ BASE_DECLARE_FEATURE(kNestedArchives);
// managed browsers.
BASE_DECLARE_FEATURE(kRealTimeUrlFilteringForEnterprise);

// Controls whether we are using red interstitial facelift updates.
BASE_DECLARE_FEATURE(kRedInterstitialFacelift);

// Enables modifying key parameters on the navigation event collection used to
// populate referrer chains.
BASE_DECLARE_FEATURE(kReferrerChainParameters);
Expand Down
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
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 @@ -101,6 +101,10 @@ function setupEvents() {
const hidePrimaryButton = loadTimeData.getBoolean('hide_primary_button');
const showRecurrentErrorParagraph = loadTimeData.getBoolean(
'show_recurrent_error_paragraph');
const shouldUseNewDangerIcon =
loadTimeData.valueExists('shouldUseNewDangerIcon') ?
loadTimeData.getBoolean('shouldUseNewDangerIcon') :
false;

const body = document.querySelector('#body');
if (ssl || blockedInterception) {
Expand Down Expand Up @@ -281,6 +285,13 @@ function setupEvents() {
console.warn(loadTimeData.getString('lookalikeConsoleMessage'));
}

if (shouldUseNewDangerIcon) {
// If red interstitial facelift is enabled, use new stop sign icons.
if (document.getElementById('icon')) {
document.getElementById('icon').classList.add('new-icon');
}
}

preventDefaultOnPoundLinkClicks();
setupExtendedReportingCheckbox();
setupEnhancedProtectionMessage();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,13 @@ body.safe-browsing {
--google-red-600: rgb(217, 48, 37);
--google-red-500: rgb(234, 67, 53);
--google-red-100: rgb(250, 210, 207);
--google-red-90: rgb(249, 222, 220);
--google-red-50: rgb(252, 232, 230);
background-color: var(--google-red-600);
--google-red-40: rgb(179, 38, 30);
--google-red-10: rgb(65, 14, 11);
--google-primary-40: rgb(11, 87, 208);
--google-primary-90: rgb(211, 227, 253);
background-color: var(--google-red-40);
color: white;
}

Expand All @@ -21,7 +26,7 @@ body.safe-browsing {
.safe-browsing button {
background: white;
border-color: white;
color: var(--google-red-700);
color: var(--google-primary-40);
}

.safe-browsing button:active {
Expand All @@ -30,12 +35,12 @@ body.safe-browsing {
}

.safe-browsing button:hover {
background: var(--google-red-50);
background: var(--google-primary-90);
}

.safe-browsing .secondary-button {
background-color: var(--google-red-600);
border-color: var(--google-red-100);
background-color: var(--google-red-40);
border-color: white;
color: white;
}

Expand All @@ -56,15 +61,23 @@ body.safe-browsing {
background-image: -webkit-image-set(
url(images/1x/triangle_white.png) 1x,
url(images/2x/triangle_white.png) 2x);
color: var(--google-red-40);
}

.safe-browsing .new-icon {
background-image: -webkit-image-set(
url(images/1x/stop_sign_white.svg) 1x,
url(images/2x/stop_sign_white.svg) 2x);
color: var(--google-red-40);
}

@media (min-width: 240px) and (max-width: 420px) and
(min-height: 401px),
(min-width: 421px) and (min-height: 240px) and
(max-height: 560px) {
body.safe-browsing .nav-wrapper {
background: var(--google-red-600);
box-shadow: 0 -12px 24px var(--google-red-600);
background: var(--google-red-40);
box-shadow: 0 -12px 24px var(--google-red-40);
}
}

Expand All @@ -75,10 +88,10 @@ body.safe-browsing {
}

.safe-browsing-enhanced-protection-message {
background-color: rgba(0, 0, 0, 0.2); /* black 20% opacity */
border: 1px solid rgba(0, 0, 0, 0.3); /* black 30% opacity */
background-color: var(--google-red-90);
color: var(--google-red-10);
}

.safe-browsing-enhanced-protection-message .icon {
background-image: url(images/light_bulb_white.svg);
background-image: url(images/light_bulb_red.svg);
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

button {
border: 0;
border-radius: 4px;
border-radius: 20px;
box-sizing: border-box;
color: var(--primary-button-text-color);
cursor: pointer;
Expand Down Expand Up @@ -145,12 +145,16 @@ input[type=checkbox]:focus ~ .checkbox::after {
}

#enhanced-protection-message {
border-radius: 4px;
border-radius: 20px;
font-size: 1em;
margin-top: 32px;
padding: 10px 5px;
}

#enhanced-protection-message a {
color: var(--google-red-10);
}

#enhanced-protection-message label {
display: grid;
grid-template-columns: 2.5em 1fr;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,11 @@ void SafeBrowsingLoudErrorUI::PopulateStringsForHtml(
break;
}

// Change UI based on whether the facelift feature is enabled.
load_time_data.Set(
"shouldUseNewDangerIcon",
base::FeatureList::IsEnabled(safe_browsing::kRedInterstitialFacelift));

// Not used by this interstitial.
load_time_data.Set("recurrentErrorParagraph", "");
load_time_data.Set("show_recurrent_error_paragraph", false);
Expand Down

0 comments on commit 5fcb41f

Please sign in to comment.