Commit 6deaae48 authored by Raymes Khoury's avatar Raymes Khoury Committed by Commit Bot

Update the UI to reflect the notifications permission for the DSE

This updates 2 UI surfaces to reflect notifications permissions set for
the DSE. Specifically:
1) In android Site Details, when toggle the DSE permission for
notififications, it will show the options "Allow/Block for the current
search engine". 2 cases are handled here: one when that setting links
directly into Notification Channels on Android O+ and the other in the
case where the permission is handled directly in Chrome.

2) In Settings>Search Engines, underneath the currently selected search
engine, it will display a link indicating whether
notifications/geolocation are allowed. The link will take the user to
the Site Details for the search engine. Several changes have been made
to the behavior here:
-The link used to indicate if location was disabled. Now, if
notifications are location are disabled, nothing will be shown in the
text for those permissions.
-The link used to take users directly to Android Location Settings when
the text indicated that system location was disabled. But now due to the
link handling combined permissions, it always takes the user directly to
Site Details for the search engine, where the user can follow a link
into Android Location Settings.

Bug: 780344
Change-Id: I41e0dd1028ad289141d1632875d30373b528f17c
Reviewed-on: https://chromium-review.googlesource.com/799534
Commit-Queue: Raymes Khoury <raymes@chromium.org>
Reviewed-by: default avatarBen Wells <benwells@chromium.org>
Reviewed-by: default avatarTed Choc <tedchoc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523606}
parent 0f49dec7
...@@ -7,9 +7,9 @@ package org.chromium.chrome.browser.preferences; ...@@ -7,9 +7,9 @@ package org.chromium.chrome.browser.preferences;
import android.content.Context; import android.content.Context;
import android.content.Intent; import android.content.Intent;
import android.content.res.Resources; import android.content.res.Resources;
import android.net.Uri;
import android.os.Build; import android.os.Build;
import android.os.Bundle; import android.os.Bundle;
import android.support.annotation.StringRes;
import android.text.SpannableString; import android.text.SpannableString;
import android.text.TextUtils; import android.text.TextUtils;
import android.text.style.ForegroundColorSpan; import android.text.style.ForegroundColorSpan;
...@@ -29,11 +29,13 @@ import org.chromium.base.Log; ...@@ -29,11 +29,13 @@ import org.chromium.base.Log;
import org.chromium.base.VisibleForTesting; import org.chromium.base.VisibleForTesting;
import org.chromium.base.metrics.RecordUserAction; import org.chromium.base.metrics.RecordUserAction;
import org.chromium.chrome.R; import org.chromium.chrome.R;
import org.chromium.chrome.browser.UrlConstants; import org.chromium.chrome.browser.ContentSettingsType;
import org.chromium.chrome.browser.locale.LocaleManager; import org.chromium.chrome.browser.locale.LocaleManager;
import org.chromium.chrome.browser.preferences.website.ContentSetting; import org.chromium.chrome.browser.preferences.website.ContentSetting;
import org.chromium.chrome.browser.preferences.website.GeolocationInfo; import org.chromium.chrome.browser.preferences.website.GeolocationInfo;
import org.chromium.chrome.browser.preferences.website.NotificationInfo;
import org.chromium.chrome.browser.preferences.website.SingleWebsitePreferences; import org.chromium.chrome.browser.preferences.website.SingleWebsitePreferences;
import org.chromium.chrome.browser.preferences.website.WebsitePreferenceBridge;
import org.chromium.chrome.browser.search_engines.TemplateUrlService; import org.chromium.chrome.browser.search_engines.TemplateUrlService;
import org.chromium.chrome.browser.search_engines.TemplateUrlService.TemplateUrl; import org.chromium.chrome.browser.search_engines.TemplateUrlService.TemplateUrl;
import org.chromium.components.location.LocationUtils; import org.chromium.components.location.LocationUtils;
...@@ -336,36 +338,13 @@ public class SearchEngineAdapter extends BaseAdapter ...@@ -336,36 +338,13 @@ public class SearchEngineAdapter extends BaseAdapter
}); });
TextView link = (TextView) view.findViewById(R.id.location_permission); TextView link = (TextView) view.findViewById(R.id.location_permission);
link.setVisibility(selected ? View.VISIBLE : View.GONE); link.setVisibility(View.GONE);
if (TemplateUrlService.getInstance().getSearchEngineUrlFromTemplateUrl( if (TemplateUrlService.getInstance().getSearchEngineUrlFromTemplateUrl(
templateUrl.getKeyword()) == null) { templateUrl.getKeyword()) == null) {
Log.e(TAG, "Invalid template URL found: %s", templateUrl); Log.e(TAG, "Invalid template URL found: %s", templateUrl);
assert false; assert false;
link.setVisibility(View.GONE);
} else if (selected) { } else if (selected) {
if (!locationShouldBeShown(templateUrl)) { setupPermissionsLink(templateUrl, link);
link.setVisibility(View.GONE);
} else {
ForegroundColorSpan linkSpan = new ForegroundColorSpan(
ApiCompatibilityUtils.getColor(resources, R.color.google_blue_700));
// If location is allowed but system location is off, show a message explaining how
// to turn location on.
if (locationEnabled(templateUrl)
&& !LocationUtils.getInstance().isSystemLocationSettingEnabled()) {
link.setText(SpanApplier.applySpans(
mContext.getString(R.string.android_location_off),
new SpanInfo("<link>", "</link>", linkSpan)));
} else {
String message = mContext.getString(locationEnabled(templateUrl)
? R.string.search_engine_location_allowed
: R.string.search_engine_location_blocked);
SpannableString messageWithLink = new SpannableString(message);
messageWithLink.setSpan(linkSpan, 0, messageWithLink.length(), 0);
link.setText(messageWithLink);
}
link.setOnClickListener(this);
}
} }
return view; return view;
...@@ -390,7 +369,7 @@ public class SearchEngineAdapter extends BaseAdapter ...@@ -390,7 +369,7 @@ public class SearchEngineAdapter extends BaseAdapter
@Override @Override
public void onClick(View view) { public void onClick(View view) {
if (view.getTag() == null) { if (view.getTag() == null) {
onLocationLinkClicked(); onPermissionsLinkClicked();
} else { } else {
searchEngineSelected((int) view.getTag()); searchEngineSelected((int) view.getTag());
} }
...@@ -413,15 +392,19 @@ public class SearchEngineAdapter extends BaseAdapter ...@@ -413,15 +392,19 @@ public class SearchEngineAdapter extends BaseAdapter
return keyword; return keyword;
} }
private void onLocationLinkClicked() { private void onPermissionsLinkClicked() {
mIsLocationPermissionChanged = true; mIsLocationPermissionChanged = true;
if (!LocationUtils.getInstance().isSystemLocationSettingEnabled()) { String url = TemplateUrlService.getInstance().getSearchEngineUrlFromTemplateUrl(
toKeyword(mSelectedSearchEnginePosition));
int linkBeingShown = getPermissionsLinkMessage(url);
assert linkBeingShown != 0;
// If notifications are off and location is on but system location is off, it's a special
// case where we link directly to Android Settings.
if (linkBeingShown == R.string.search_engine_system_location_disabled) {
mContext.startActivity(LocationUtils.getInstance().getSystemLocationSettingsIntent()); mContext.startActivity(LocationUtils.getInstance().getSystemLocationSettingsIntent());
} else { } else {
Intent settingsIntent = PreferencesLauncher.createIntentForSettingsPage( Intent settingsIntent = PreferencesLauncher.createIntentForSettingsPage(
mContext, SingleWebsitePreferences.class.getName()); mContext, SingleWebsitePreferences.class.getName());
String url = TemplateUrlService.getInstance().getSearchEngineUrlFromTemplateUrl(
toKeyword(mSelectedSearchEnginePosition));
Bundle fragmentArgs = SingleWebsitePreferences.createFragmentArgsForSite(url); Bundle fragmentArgs = SingleWebsitePreferences.createFragmentArgsForSite(url);
settingsIntent.putExtra(Preferences.EXTRA_SHOW_FRAGMENT_ARGUMENTS, fragmentArgs); settingsIntent.putExtra(Preferences.EXTRA_SHOW_FRAGMENT_ARGUMENTS, fragmentArgs);
mContext.startActivity(settingsIntent); mContext.startActivity(settingsIntent);
...@@ -446,18 +429,69 @@ public class SearchEngineAdapter extends BaseAdapter ...@@ -446,18 +429,69 @@ public class SearchEngineAdapter extends BaseAdapter
return url; return url;
} }
private boolean locationShouldBeShown(TemplateUrl templateUrl) { @StringRes
String url = getSearchEngineUrl(templateUrl); private int getPermissionsLinkMessage(String url) {
if (url.isEmpty()) return false; if (url == null) return 0;
// Do not show location if the scheme isn't HTTPS. NotificationInfo notificationSettings = new NotificationInfo(url, null, false);
Uri uri = Uri.parse(url); boolean notificationsAllowed =
if (!UrlConstants.HTTPS_SCHEME.equals(uri.getScheme())) return false; notificationSettings.getContentSetting() == ContentSetting.ALLOW
&& WebsitePreferenceBridge.isPermissionControlledByDSE(
ContentSettingsType.CONTENT_SETTINGS_TYPE_NOTIFICATIONS, url, false);
// Only show the location setting if it is explicitly enabled or disabled.
GeolocationInfo locationSettings = new GeolocationInfo(url, null, false); GeolocationInfo locationSettings = new GeolocationInfo(url, null, false);
ContentSetting locationPermission = locationSettings.getContentSetting(); boolean locationAllowed = locationSettings.getContentSetting() == ContentSetting.ALLOW
return locationPermission != ContentSetting.ASK; && WebsitePreferenceBridge.isPermissionControlledByDSE(
ContentSettingsType.CONTENT_SETTINGS_TYPE_GEOLOCATION, url, false);
boolean systemLocationAllowed =
LocationUtils.getInstance().isSystemLocationSettingEnabled();
// Cases where location is fully enabled.
if (locationAllowed && systemLocationAllowed) {
if (notificationsAllowed) {
return R.string.search_engine_location_and_notifications_allowed;
} else {
return R.string.search_engine_location_allowed;
}
}
// Cases where the user has allowed location for the site but it's disabled at the system
// level.
if (locationAllowed) {
if (notificationsAllowed) {
return R.string.search_engine_notifications_allowed_system_location_disabled;
} else {
return R.string.search_engine_system_location_disabled;
}
}
// Cases where the user has not allowed location.
if (notificationsAllowed) return R.string.search_engine_notifications_allowed;
return 0;
}
private void setupPermissionsLink(TemplateUrl templateUrl, TextView link) {
int message = getPermissionsLinkMessage(getSearchEngineUrl(templateUrl));
if (message == 0) return;
ForegroundColorSpan linkSpan = new ForegroundColorSpan(
ApiCompatibilityUtils.getColor(mContext.getResources(), R.color.google_blue_700));
link.setVisibility(View.VISIBLE);
link.setOnClickListener(this);
// If notifications are off and location is on but system location is off, it's a special
// case where we link directly to Android Settings. So we only highlight that part of the
// link to make it clearer.
if (message == R.string.search_engine_system_location_disabled) {
link.setText(SpanApplier.applySpans(
mContext.getString(message), new SpanInfo("<link>", "</link>", linkSpan)));
} else {
SpannableString messageWithLink = new SpannableString(mContext.getString(message));
messageWithLink.setSpan(linkSpan, 0, messageWithLink.length(), 0);
link.setText(messageWithLink);
}
} }
private boolean locationEnabled(TemplateUrl templateUrl) { private boolean locationEnabled(TemplateUrl templateUrl) {
......
...@@ -368,9 +368,9 @@ public class SingleWebsitePreferences extends PreferenceFragment ...@@ -368,9 +368,9 @@ public class SingleWebsitePreferences extends PreferenceFragment
} }
private void setUpNotificationsPreference(Preference preference) { private void setUpNotificationsPreference(Preference preference) {
final ContentSetting value = mSite.getNotificationPermission();
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O
&& ChromeFeatureList.isEnabled(ChromeFeatureList.SITE_NOTIFICATION_CHANNELS)) { && ChromeFeatureList.isEnabled(ChromeFeatureList.SITE_NOTIFICATION_CHANNELS)) {
final ContentSetting value = mSite.getNotificationPermission();
if (!(value == ContentSetting.ALLOW || value == ContentSetting.BLOCK)) { if (!(value == ContentSetting.ALLOW || value == ContentSetting.BLOCK)) {
// TODO(crbug.com/735110): Figure out if this is the correct thing to do, for values // TODO(crbug.com/735110): Figure out if this is the correct thing to do, for values
// that are non-null, but not ALLOW or BLOCK either. (In setupListPreference we // that are non-null, but not ALLOW or BLOCK either. (In setupListPreference we
...@@ -384,8 +384,16 @@ public class SingleWebsitePreferences extends PreferenceFragment ...@@ -384,8 +384,16 @@ public class SingleWebsitePreferences extends PreferenceFragment
newPreference.setKey(preference.getKey()); newPreference.setKey(preference.getKey());
setUpPreferenceCommon(newPreference); setUpPreferenceCommon(newPreference);
newPreference.setSummary( if (isPermissionControlledByDSE(
getResources().getString(ContentSettingsResources.getSiteSummary(value))); ContentSettingsType.CONTENT_SETTINGS_TYPE_NOTIFICATIONS)) {
newPreference.setSummary(getResources().getString(value == ContentSetting.ALLOW
? R.string.website_settings_permissions_allow_dse
: R.string.website_settings_permissions_block_dse));
} else {
newPreference.setSummary(
getResources().getString(ContentSettingsResources.getSiteSummary(value)));
}
newPreference.setDefaultValue(value); newPreference.setDefaultValue(value);
// This preference is read-only so should not attempt to persist to shared prefs. // This preference is read-only so should not attempt to persist to shared prefs.
...@@ -410,7 +418,11 @@ public class SingleWebsitePreferences extends PreferenceFragment ...@@ -410,7 +418,11 @@ public class SingleWebsitePreferences extends PreferenceFragment
getPreferenceScreen().removePreference(preference); getPreferenceScreen().removePreference(preference);
getPreferenceScreen().addPreference(newPreference); getPreferenceScreen().addPreference(newPreference);
} else { } else {
setUpListPreference(preference, mSite.getNotificationPermission()); setUpListPreference(preference, value);
if (isPermissionControlledByDSE(ContentSettingsType.CONTENT_SETTINGS_TYPE_NOTIFICATIONS)
&& value != null) {
updatePreferenceForDSESetting(preference);
}
} }
} }
...@@ -620,7 +632,7 @@ public class SingleWebsitePreferences extends PreferenceFragment ...@@ -620,7 +632,7 @@ public class SingleWebsitePreferences extends PreferenceFragment
setUpListPreference(preference, permission); setUpListPreference(preference, permission);
if (isPermissionControlledByDSE(ContentSettingsType.CONTENT_SETTINGS_TYPE_GEOLOCATION) if (isPermissionControlledByDSE(ContentSettingsType.CONTENT_SETTINGS_TYPE_GEOLOCATION)
&& permission != null) { && permission != null) {
updateLocationPreferenceForDSESetting(preference); updatePreferenceForDSESetting(preference);
} }
} }
...@@ -684,7 +696,7 @@ public class SingleWebsitePreferences extends PreferenceFragment ...@@ -684,7 +696,7 @@ public class SingleWebsitePreferences extends PreferenceFragment
* for searches that happen from the omnibox. * for searches that happen from the omnibox.
* @param preference The Location preference to modify. * @param preference The Location preference to modify.
*/ */
private void updateLocationPreferenceForDSESetting(Preference preference) { private void updatePreferenceForDSESetting(Preference preference) {
ListPreference listPreference = (ListPreference) preference; ListPreference listPreference = (ListPreference) preference;
Resources res = getResources(); Resources res = getResources();
listPreference.setEntries(new String[] { listPreference.setEntries(new String[] {
......
...@@ -305,12 +305,22 @@ CHAR-LIMIT guidelines: ...@@ -305,12 +305,22 @@ CHAR-LIMIT guidelines:
<message name="IDS_PREFS_SEARCH_ENGINE" desc="Title for Search Engine preferences. [CHAR-LIMIT=32]"> <message name="IDS_PREFS_SEARCH_ENGINE" desc="Title for Search Engine preferences. [CHAR-LIMIT=32]">
Search engine Search engine
</message> </message>
<message name="IDS_SEARCH_ENGINE_LOCATION_ALLOWED" desc="The text of a link displayed when location permission for a particular search engine is allowed."> <message name="IDS_SEARCH_ENGINE_LOCATION_AND_NOTIFICATIONS_ALLOWED" desc="The text of a link displayed when location and notifications permissions for a particular search engine are allowed.">
Location and notifications are allowed
</message>
<message name="IDS_SEARCH_ENGINE_LOCATION_ALLOWED" desc="The text of a link displayed when the location permission for a particular search engine is allowed.">
Location is allowed Location is allowed
</message> </message>
<message name="IDS_SEARCH_ENGINE_LOCATION_BLOCKED" desc="The text of a link displayed when location permission for a particular search engine is blocked."> <message name="IDS_SEARCH_ENGINE_NOTIFICATIONS_ALLOWED" desc="The text of a link displayed when the notifications permission for a particular search engine is allowed.">
Location is blocked Notifications are allowed
</message>
<message name="IDS_SEARCH_ENGINE_NOTIFICATIONS_ALLOWED_SYSTEM_LOCATION_DISABLED" desc="The text of a link displayed when the location and notifications permissions for a particular search engine are allowed but system location location is turned off. Contains a link to the settings menu to change it.">
Notifications are allowed. Location is off; turn it on in Android Settings.
</message>
<message name="IDS_SEARCH_ENGINE_SYSTEM_LOCATION_DISABLED" desc="The text of a link displayed when the user has granted geolocation to a particular search engine but system location is turned off. Contains a link to the settings menu to change it.">
Location is off; turn it on in <ph name="BEGIN_LINK">&lt;link&gt;</ph>Android Settings<ph name="END_LINK">&lt;/link&gt;</ph>.
</message> </message>
<message name="IDS_SEARCH_ENGINE_RECENTLY_VISITED" desc="Header for the list of recently visited search engines."> <message name="IDS_SEARCH_ENGINE_RECENTLY_VISITED" desc="Header for the list of recently visited search engines.">
Recently visited Recently visited
</message> </message>
...@@ -954,9 +964,6 @@ Your Google account may have other forms of browsing history like searches and a ...@@ -954,9 +964,6 @@ Your Google account may have other forms of browsing history like searches and a
<message name="IDS_GEOLOCATION_PERMISSION_TITLE" desc="Title for the permission of accessing the current location of a device [CHAR-LIMIT=32]"> <message name="IDS_GEOLOCATION_PERMISSION_TITLE" desc="Title for the permission of accessing the current location of a device [CHAR-LIMIT=32]">
Location access Location access
</message> </message>
<message name="IDS_ANDROID_LOCATION_OFF" desc="Text in the search engine picker dialog, explaining to the user that the location service in Android is turned off. Contains a link to the settings menu to change it.">
Turn on location in <ph name="BEGIN_LINK">&lt;link&gt;</ph>Android Settings<ph name="END_LINK">&lt;/link&gt;</ph>.
</message>
<message name="IDS_ANDROID_PERMISSION_OFF_PLURAL" desc="Text at the top of the Website list, explaining to the user that multiple permissions, such as the location service, are turned off. Contains a link to the settings menu to change it."> <message name="IDS_ANDROID_PERMISSION_OFF_PLURAL" desc="Text at the top of the Website list, explaining to the user that multiple permissions, such as the location service, are turned off. Contains a link to the settings menu to change it.">
Turn on permissions for Chrome in <ph name="BEGIN_LINK">&lt;link&gt;</ph>Android Settings<ph name="END_LINK">&lt;/link&gt;</ph>. Turn on permissions for Chrome in <ph name="BEGIN_LINK">&lt;link&gt;</ph>Android Settings<ph name="END_LINK">&lt;/link&gt;</ph>.
</message> </message>
......
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