Commit 03c64fa5 authored by Jarryd's avatar Jarryd Committed by Commit Bot

SiteSettings: Display origin consistently between pages.

Match origin string representations on the SiteDetails pages with
how they are represented on the AllSites page by displaying host
only, except in cases where a non-default scheme or port is used.

Bug: 997258
Change-Id: I17fda8124f9d4ac3dade15889f65e0a32f111b62
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2043862
Commit-Queue: Jarryd Goodman <jarrydg@chromium.org>
Reviewed-by: default avatarEsmael Elmoslimany <aee@chromium.org>
Reviewed-by: default avatarDemetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#741680}
parent 6167430d
...@@ -3858,10 +3858,10 @@ ...@@ -3858,10 +3858,10 @@
Offline data in installed apps will also be cleared Offline data in installed apps will also be cleared
</message> </message>
<message name="IDS_SETTINGS_SITE_SETTINGS_ORIGIN_DELETE_CONFIRMATION" desc="Text for the dialog that warns about clearing all storage data for an origin."> <message name="IDS_SETTINGS_SITE_SETTINGS_ORIGIN_DELETE_CONFIRMATION" desc="Text for the dialog that warns about clearing all storage data for an origin.">
This will clear all data and cookies stored by <ph name="ORIGIN_NAME">$1<ex>https://www.google.co.uk</ex></ph>. This will clear all data and cookies stored by <ph name="ORIGIN_NAME">$1<ex>www.example.com</ex></ph>.
</message> </message>
<message name="IDS_SETTINGS_SITE_SETTINGS_ORIGIN_DELETE_CONFIRMATION_INSTALLED" desc="Text for the dialog that warns about clearing all storage data for an origin that has an installed app."> <message name="IDS_SETTINGS_SITE_SETTINGS_ORIGIN_DELETE_CONFIRMATION_INSTALLED" desc="Text for the dialog that warns about clearing all storage data for an origin that has an installed app.">
This will clear all data and cookies stored by <ph name="ORIGIN_NAME">$1<ex>https://www.google.co.uk</ex></ph> and its installed app. This will clear all data and cookies stored by <ph name="ORIGIN_NAME">$1<ex>www.example.com</ex></ph> and its installed app.
</message> </message>
<message name="IDS_SETTINGS_SITE_SETTINGS_COOKIE_REMOVE_MULTIPLE" desc="Text for the dialog that warns about deleting all site data."> <message name="IDS_SETTINGS_SITE_SETTINGS_COOKIE_REMOVE_MULTIPLE" desc="Text for the dialog that warns about deleting all site data.">
This will delete any data stored on your device for all the sites shown. Do you want to continue? This will delete any data stored on your device for all the sites shown. Do you want to continue?
......
...@@ -497,7 +497,8 @@ Polymer({ ...@@ -497,7 +497,8 @@ Polymer({
const messageId = isInstalled ? const messageId = isInstalled ?
'siteSettingsOriginDeleteConfirmationInstalled' : 'siteSettingsOriginDeleteConfirmationInstalled' :
'siteSettingsOriginDeleteConfirmation'; 'siteSettingsOriginDeleteConfirmation';
return loadTimeData.substituteString(this.i18n(messageId), origin); return loadTimeData.substituteString(
this.i18n(messageId), this.originRepresentation(origin));
} else { } else {
// Clear SiteGroup // Clear SiteGroup
let messageId; let messageId;
...@@ -513,8 +514,10 @@ Polymer({ ...@@ -513,8 +514,10 @@ Polymer({
} else { } else {
messageId = 'siteSettingsSiteGroupDeleteConfirmationNew'; messageId = 'siteSettingsSiteGroupDeleteConfirmationNew';
} }
return loadTimeData.substituteString( const displayName = this.actionMenuModel_.item.etldPlus1 ||
this.i18n(messageId), this.actionMenuModel_.item.etldPlus1); this.originRepresentation(
this.actionMenuModel_.item.origins[0].origin);
return loadTimeData.substituteString(this.i18n(messageId), displayName);
} }
} else { } else {
// Storage Pressure UI disabled // Storage Pressure UI disabled
...@@ -539,11 +542,13 @@ Polymer({ ...@@ -539,11 +542,13 @@ Polymer({
if (this.actionMenuModel_.actionScope === 'origin') { if (this.actionMenuModel_.actionScope === 'origin') {
return loadTimeData.substituteString( return loadTimeData.substituteString(
this.i18n('siteSettingsSiteResetConfirmation'), this.i18n('siteSettingsSiteResetConfirmation'),
this.actionMenuModel_.origin); this.originRepresentation(this.actionMenuModel_.origin));
} }
return loadTimeData.substituteString( return loadTimeData.substituteString(
this.i18n('siteSettingsSiteGroupResetConfirmation'), this.i18n('siteSettingsSiteGroupResetConfirmation'),
this.actionMenuModel_.item.etldPlus1); this.actionMenuModel_.item.etldPlus1 ||
this.originRepresentation(
this.actionMenuModel_.item.origins[0].origin));
}, },
/** /**
* Get the appropriate label for the clear all data confirmation * Get the appropriate label for the clear all data confirmation
......
...@@ -205,7 +205,8 @@ Polymer({ ...@@ -205,7 +205,8 @@ Polymer({
// The displayName won't change, so just use the first // The displayName won't change, so just use the first
// exception. // exception.
assert(exceptionList.length > 0); assert(exceptionList.length > 0);
this.pageTitle = exceptionList[0].displayName; this.pageTitle =
this.originRepresentation(exceptionList[0].displayName);
}); });
}, },
......
...@@ -100,7 +100,7 @@ ...@@ -100,7 +100,7 @@
<div class="site-representation middle text-elide"> <div class="site-representation middle text-elide">
<span id="originSiteRepresentation" <span id="originSiteRepresentation"
class="url-directionality"> class="url-directionality">
[[originRepresentation_(item)]] [[originRepresentation(item.origin)]]
</span> </span>
<span class="secondary" <span class="secondary"
hidden$="[[!originScheme_(item)]]"> hidden$="[[!originScheme_(item)]]">
......
...@@ -165,18 +165,7 @@ Polymer({ ...@@ -165,18 +165,7 @@ Polymer({
// Fall back onto using the host of the first origin, if no eTLD+1 name // Fall back onto using the host of the first origin, if no eTLD+1 name
// was computed. // was computed.
} }
return this.originRepresentation_(siteGroup.origins[0]); return this.originRepresentation(siteGroup.origins[0].origin);
},
/**
* Returns a user-friendly name for the origin.
* @param {OriginInfo} origin
* @return {string} The user-friendly name.
* @private
*/
originRepresentation_(origin) {
const url = this.toUrl(origin.origin);
return url.host;
}, },
/** /**
......
...@@ -121,6 +121,21 @@ const SiteSettingsBehaviorImpl = { ...@@ -121,6 +121,21 @@ const SiteSettingsBehaviorImpl = {
return new URL(this.ensureUrlHasScheme(originOrPattern)); return new URL(this.ensureUrlHasScheme(originOrPattern));
}, },
/**
* Returns a user-friendly name for the origin.
* @param {string} origin
* @return {string} The user-friendly name.
* @protected
*/
originRepresentation(origin) {
try {
const url = this.toUrl(origin);
return url ? (url.host || url.origin) : '';
} catch (error) {
return '';
}
},
/** /**
* Convert an exception (received from the C++ handler) to a full * Convert an exception (received from the C++ handler) to a full
* SiteException. * SiteException.
......
...@@ -1083,6 +1083,8 @@ TEST_F(SiteSettingsHandlerTest, DefaultSettingSource) { ...@@ -1083,6 +1083,8 @@ TEST_F(SiteSettingsHandlerTest, DefaultSettingSource) {
// Use a non-default port to verify the display name does not strip this off. // Use a non-default port to verify the display name does not strip this off.
const std::string google("https://www.google.com:183"); const std::string google("https://www.google.com:183");
const std::string expected_display_name("www.google.com:183");
ContentSettingSourceSetter source_setter(profile(), ContentSettingSourceSetter source_setter(profile(),
ContentSettingsType::NOTIFICATIONS); ContentSettingsType::NOTIFICATIONS);
...@@ -1095,7 +1097,7 @@ TEST_F(SiteSettingsHandlerTest, DefaultSettingSource) { ...@@ -1095,7 +1097,7 @@ TEST_F(SiteSettingsHandlerTest, DefaultSettingSource) {
// Test Chrome built-in defaults are marked as default. // Test Chrome built-in defaults are marked as default.
handler()->HandleGetOriginPermissions(&get_origin_permissions_args); handler()->HandleGetOriginPermissions(&get_origin_permissions_args);
ValidateOrigin(google, google, google, CONTENT_SETTING_ASK, ValidateOrigin(google, google, expected_display_name, CONTENT_SETTING_ASK,
site_settings::SiteSettingSource::kDefault, 1U); site_settings::SiteSettingSource::kDefault, 1U);
base::ListValue default_value_args; base::ListValue default_value_args;
...@@ -1105,7 +1107,7 @@ TEST_F(SiteSettingsHandlerTest, DefaultSettingSource) { ...@@ -1105,7 +1107,7 @@ TEST_F(SiteSettingsHandlerTest, DefaultSettingSource) {
handler()->HandleSetDefaultValueForContentType(&default_value_args); handler()->HandleSetDefaultValueForContentType(&default_value_args);
// A user-set global default should also show up as default. // A user-set global default should also show up as default.
handler()->HandleGetOriginPermissions(&get_origin_permissions_args); handler()->HandleGetOriginPermissions(&get_origin_permissions_args);
ValidateOrigin(google, google, google, CONTENT_SETTING_BLOCK, ValidateOrigin(google, google, expected_display_name, CONTENT_SETTING_BLOCK,
site_settings::SiteSettingSource::kDefault, 3U); site_settings::SiteSettingSource::kDefault, 3U);
base::ListValue set_notification_pattern_args; base::ListValue set_notification_pattern_args;
...@@ -1119,7 +1121,7 @@ TEST_F(SiteSettingsHandlerTest, DefaultSettingSource) { ...@@ -1119,7 +1121,7 @@ TEST_F(SiteSettingsHandlerTest, DefaultSettingSource) {
&set_notification_pattern_args); &set_notification_pattern_args);
// A user-set pattern should not show up as default. // A user-set pattern should not show up as default.
handler()->HandleGetOriginPermissions(&get_origin_permissions_args); handler()->HandleGetOriginPermissions(&get_origin_permissions_args);
ValidateOrigin(google, google, google, CONTENT_SETTING_ALLOW, ValidateOrigin(google, google, expected_display_name, CONTENT_SETTING_ALLOW,
site_settings::SiteSettingSource::kPreference, 5U); site_settings::SiteSettingSource::kPreference, 5U);
base::ListValue set_notification_origin_args; base::ListValue set_notification_origin_args;
...@@ -1133,20 +1135,20 @@ TEST_F(SiteSettingsHandlerTest, DefaultSettingSource) { ...@@ -1133,20 +1135,20 @@ TEST_F(SiteSettingsHandlerTest, DefaultSettingSource) {
&set_notification_origin_args); &set_notification_origin_args);
// A user-set per-origin permission should not show up as default. // A user-set per-origin permission should not show up as default.
handler()->HandleGetOriginPermissions(&get_origin_permissions_args); handler()->HandleGetOriginPermissions(&get_origin_permissions_args);
ValidateOrigin(google, google, google, CONTENT_SETTING_BLOCK, ValidateOrigin(google, google, expected_display_name, CONTENT_SETTING_BLOCK,
site_settings::SiteSettingSource::kPreference, 7U); site_settings::SiteSettingSource::kPreference, 7U);
// Enterprise-policy set defaults should not show up as default. // Enterprise-policy set defaults should not show up as default.
source_setter.SetPolicyDefault(CONTENT_SETTING_ALLOW); source_setter.SetPolicyDefault(CONTENT_SETTING_ALLOW);
handler()->HandleGetOriginPermissions(&get_origin_permissions_args); handler()->HandleGetOriginPermissions(&get_origin_permissions_args);
ValidateOrigin(google, google, google, CONTENT_SETTING_ALLOW, ValidateOrigin(google, google, expected_display_name, CONTENT_SETTING_ALLOW,
site_settings::SiteSettingSource::kPolicy, 8U); site_settings::SiteSettingSource::kPolicy, 8U);
} }
TEST_F(SiteSettingsHandlerTest, GetAndSetOriginPermissions) { TEST_F(SiteSettingsHandlerTest, GetAndSetOriginPermissions) {
const std::string origin_with_port("https://www.example.com:443"); const std::string origin_with_port("https://www.example.com:443");
// The display name won't show the port if it's default for that scheme. // The display name won't show the port if it's default for that scheme.
const std::string origin("https://www.example.com"); const std::string origin("www.example.com");
base::ListValue get_args; base::ListValue get_args;
get_args.AppendString(kCallbackId); get_args.AppendString(kCallbackId);
get_args.AppendString(origin_with_port); get_args.AppendString(origin_with_port);
......
...@@ -11,6 +11,7 @@ ...@@ -11,6 +11,7 @@
#include "base/feature_list.h" #include "base/feature_list.h"
#include "base/stl_util.h" #include "base/stl_util.h"
#include "base/strings/utf_string_conversions.h"
#include "base/values.h" #include "base/values.h"
#include "chrome/browser/content_settings/host_content_settings_map_factory.h" #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
#include "chrome/browser/hid/hid_chooser_context.h" #include "chrome/browser/hid/hid_chooser_context.h"
...@@ -30,6 +31,7 @@ ...@@ -30,6 +31,7 @@
#include "components/permissions/permission_result.h" #include "components/permissions/permission_result.h"
#include "components/prefs/pref_service.h" #include "components/prefs/pref_service.h"
#include "components/subresource_filter/core/browser/subresource_filter_features.h" #include "components/subresource_filter/core/browser/subresource_filter_features.h"
#include "components/url_formatter/url_formatter.h"
#include "extensions/browser/extension_registry.h" #include "extensions/browser/extension_registry.h"
#include "extensions/common/constants.h" #include "extensions/common/constants.h"
#include "url/origin.h" #include "url/origin.h"
...@@ -426,9 +428,14 @@ std::string GetDisplayNameForGURL( ...@@ -426,9 +428,14 @@ std::string GetDisplayNameForGURL(
if (!display_name.empty()) if (!display_name.empty())
return display_name; return display_name;
// Note that using Serialize() here will chop off default port numbers and auto url_16 = url_formatter::FormatUrl(
// percent encode the origin. url,
return origin.Serialize(); url_formatter::kFormatUrlOmitDefaults |
url_formatter::kFormatUrlOmitHTTPS |
url_formatter::kFormatUrlOmitTrailingSlashOnBareHostname,
net::UnescapeRule::NONE, nullptr, nullptr, nullptr);
auto url_string = base::UTF16ToUTF8(url_16);
return url_string;
} }
// If the given |pattern| represents an individual origin or extension, retrieve // If the given |pattern| represents an individual origin or extension, retrieve
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment