Commit b0f26697 authored by James Cook's avatar James Cook Committed by Commit Bot

SplitSettings: Change OS search engine menu to link to browser setting

After PM/UX discussion, we don't want to duplicate the search engine
drop-down between browser and OS settings. Instead, OS settings uses
an info label with a link to browser settings.

Screenshots on bug:
https://bugs.chromium.org/p/chromium/issues/detail?id=960937#c7

Bug: 960937
Change-Id: I622558c4c7238dc8f3d36d3a63305e7abc0f473a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1640601Reviewed-by: default avatarXiyuan Xia <xiyuan@chromium.org>
Commit-Queue: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#665641}
parent 52f0071c
...@@ -14,5 +14,10 @@ ...@@ -14,5 +14,10 @@
<message name="IDS_OS_SETTINGS_OPEN_WALLPAPER_APP" desc="Sub-label about opening the wallpaper app."> <message name="IDS_OS_SETTINGS_OPEN_WALLPAPER_APP" desc="Sub-label about opening the wallpaper app.">
Open the wallpaper app Open the wallpaper app
</message> </message>
<!-- Search and Assistant section. -->
<message name="IDS_OS_SETTINGS_SEARCH_ENGINE_LABEL" desc="Label in OS settings describing search engine behavior.">
Searches from the app launcher use your browser <ph name="BEGIN_LINK">&lt;a target="_blank" href="$1"&gt;</ph>search engine setting<ph name="END_LINK">&lt;/a&gt;</ph>.
</message>
</if> </if>
</grit-part> </grit-part>
...@@ -14,7 +14,6 @@ js_library("os_search_page") { ...@@ -14,7 +14,6 @@ js_library("os_search_page") {
deps = [ deps = [
"../..:route", "../..:route",
"../../prefs", "../../prefs",
"../../search_engines_page:search_engines_browser_proxy",
"../../settings_page:settings_animated_pages", "../../settings_page:settings_animated_pages",
"//ui/webui/resources/js:cr", "//ui/webui/resources/js:cr",
"//ui/webui/resources/js:i18n_behavior", "//ui/webui/resources/js:i18n_behavior",
......
<link rel="import" href="chrome://resources/html/polymer.html"> <link rel="import" href="chrome://resources/html/polymer.html">
<link rel="import" href="chrome://resources/cr_elements/cr_icon_button/cr_icon_button.html"> <link rel="import" href="chrome://resources/cr_elements/cr_icon_button/cr_icon_button.html">
<link rel="import" href="chrome://resources/cr_elements/policy/cr_policy_pref_indicator.html">
<link rel="import" href="chrome://resources/cr_elements/shared_vars_css.html"> <link rel="import" href="chrome://resources/cr_elements/shared_vars_css.html">
<link rel="import" href="chrome://resources/html/assert.html"> <link rel="import" href="chrome://resources/html/assert.html">
<link rel="import" href="chrome://resources/html/i18n_behavior.html"> <link rel="import" href="chrome://resources/html/i18n_behavior.html">
<link rel="import" href="chrome://resources/html/md_select_css.html">
<link rel="import" href="../../controls/extension_controlled_indicator.html">
<link rel="import" href="../../google_assistant_page/google_assistant_page.html"> <link rel="import" href="../../google_assistant_page/google_assistant_page.html">
<link rel="import" href="../../google_assistant_page/google_assistant_browser_proxy.html"> <link rel="import" href="../../google_assistant_page/google_assistant_browser_proxy.html">
<link rel="import" href="../../icons.html"> <link rel="import" href="../../icons.html">
<link rel="import" href="../../i18n_setup.html"> <link rel="import" href="../../i18n_setup.html">
<link rel="import" href="../../route.html"> <link rel="import" href="../../route.html">
<link rel="import" href="../../search_engines_page/search_engines_browser_proxy.html">
<link rel="import" href="../../settings_page/settings_animated_pages.html"> <link rel="import" href="../../settings_page/settings_animated_pages.html">
<link rel="import" href="../../settings_page/settings_subpage.html"> <link rel="import" href="../../settings_page/settings_subpage.html">
<link rel="import" href="../../settings_shared_css.html"> <link rel="import" href="../../settings_shared_css.html">
...@@ -21,12 +17,6 @@ ...@@ -21,12 +17,6 @@
<dom-module id="os-settings-search-page"> <dom-module id="os-settings-search-page">
<template> <template>
<style include="settings-shared md-select"> <style include="settings-shared md-select">
#search-wrapper {
align-items: center;
display: flex;
min-height: var(--settings-row-min-height);
}
iron-icon { iron-icon {
padding-inline-end: 16px; padding-inline-end: 16px;
} }
...@@ -34,46 +24,10 @@ ...@@ -34,46 +24,10 @@
<settings-animated-pages id="pages" section="search" <settings-animated-pages id="pages" section="search"
focus-config="[[focusConfig_]]"> focus-config="[[focusConfig_]]">
<div route-path="default"> <div route-path="default">
<!-- Omnibox and launcher search engine. This shares the same pref with
browser search engine because users probably don't want one engine
in the omnibox and a different one in the launcher. -->
<div class="settings-box first block">
<div id="search-wrapper">
<div id="searchExplanation" class="start">
$i18nRaw{searchExplanation}
</div>
<template is="dom-if" if="[[isDefaultSearchControlledByPolicy_(
prefs.default_search_provider_data.template_url_data)]]">
<cr-policy-pref-indicator pref="[[
prefs.default_search_provider_data.template_url_data]]">
</cr-policy-pref-indicator>
</template>
<select class="md-select" on-change="onChange_"
aria-labelledby="searchExplanation"
disabled$="[[isDefaultSearchEngineEnforced_(
prefs.default_search_provider_data.template_url_data)]]">
<template is="dom-repeat" items="[[searchEngines_]]">
<option selected="[[item.default]]">[[item.name]]</option>
</template>
</select>
</div>
<template is="dom-if"
if="[[prefs.default_search_provider_data.template_url_data.extensionId]]">
<extension-controlled-indicator
extension-id="[[
prefs.default_search_provider_data.template_url_data.extensionId]]"
extension-name="[[
prefs.default_search_provider_data.template_url_data.controlledByName]]"
extension-can-be-disabled="[[
prefs.default_search_provider_data.template_url_data.extensionCanBeDisabled]]"
on-disable-extension="onDisableExtension_">
</extension-controlled-indicator>
</template>
</div>
<!-- Google Assistant --> <!-- Google Assistant -->
<template is="dom-if" if="[[isAssistantAllowed_]]"> <template is="dom-if" if="[[isAssistantAllowed_]]">
<div id="assistantSubpageTrigger" class="settings-box two-line" <div id="assistantSubpageTrigger" class="settings-box first two-line"
on-click="onGoogleAssistantTap_" actionable> on-click="onGoogleAssistantTap_" actionable>
<div class="start"> <div class="start">
$i18n{searchGoogleAssistant} $i18n{searchGoogleAssistant}
...@@ -86,7 +40,17 @@ ...@@ -86,7 +40,17 @@
aria-label="$i18n{searchGoogleAssistant}"></cr-icon-button> aria-label="$i18n{searchGoogleAssistant}"></cr-icon-button>
</div> </div>
</template> </template>
<!-- Label explaining that the app launcher search box uses the
browser's search engine. -->
<div class="settings-box">
<div class="start">
<iron-icon icon="cr:info-outline"></iron-icon>
$i18nRaw{osSearchEngineLabel}
</div>
</div>
</div> </div>
<template is="dom-if" if="[[isAssistantAllowed_]]"> <template is="dom-if" if="[[isAssistantAllowed_]]">
<template is="dom-if" route-path="/googleAssistant"> <template is="dom-if" route-path="/googleAssistant">
<settings-subpage <settings-subpage
......
...@@ -14,20 +14,6 @@ Polymer({ ...@@ -14,20 +14,6 @@ Polymer({
properties: { properties: {
prefs: Object, prefs: Object,
/**
* List of default search engines available.
* @private {!Array<!SearchEngine>}
*/
searchEngines_: {
type: Array,
value: function() {
return [];
}
},
/** @private Filter applied to search engines. */
searchEnginesFilter_: String,
/** @type {?Map<string, string>} */ /** @type {?Map<string, string>} */
focusConfig_: Object, focusConfig_: Object,
...@@ -40,22 +26,8 @@ Polymer({ ...@@ -40,22 +26,8 @@ Polymer({
}, },
}, },
/** @private {?settings.SearchEnginesBrowserProxy} */
browserProxy_: null,
/** @override */
created: function() {
this.browserProxy_ = settings.SearchEnginesBrowserProxyImpl.getInstance();
},
/** @override */ /** @override */
ready: function() { ready: function() {
const updateSearchEngines = searchEngines => {
this.set('searchEngines_', searchEngines.defaults);
};
this.browserProxy_.getSearchEnginesList().then(updateSearchEngines);
cr.addWebUIListener('search-engines-changed', updateSearchEngines);
this.focusConfig_ = new Map(); this.focusConfig_ = new Map();
if (settings.routes.GOOGLE_ASSISTANT) { if (settings.routes.GOOGLE_ASSISTANT) {
this.focusConfig_.set( this.focusConfig_.set(
...@@ -64,18 +36,6 @@ Polymer({ ...@@ -64,18 +36,6 @@ Polymer({
} }
}, },
/** @private */
onChange_: function() {
const select = /** @type {!HTMLSelectElement} */ (this.$$('select'));
const searchEngine = this.searchEngines_[select.selectedIndex];
this.browserProxy_.setDefaultSearchEngine(searchEngine.modelIndex);
},
/** @private */
onDisableExtension_: function() {
this.fire('refresh-pref', 'default_search_provider.enabled');
},
/** @private */ /** @private */
onGoogleAssistantTap_: function() { onGoogleAssistantTap_: function() {
assert(this.isAssistantAllowed_); assert(this.isAssistantAllowed_);
...@@ -92,22 +52,4 @@ Polymer({ ...@@ -92,22 +52,4 @@ Polymer({
toggleValue ? 'searchGoogleAssistantEnabled' : toggleValue ? 'searchGoogleAssistantEnabled' :
'searchGoogleAssistantDisabled'); 'searchGoogleAssistantDisabled');
}, },
/**
* @param {!chrome.settingsPrivate.PrefObject} pref
* @return {boolean}
* @private
*/
isDefaultSearchControlledByPolicy_: function(pref) {
return pref.controlledBy == chrome.settingsPrivate.ControlledBy.USER_POLICY;
},
/**
* @param {!chrome.settingsPrivate.PrefObject} pref
* @return {boolean}
* @private
*/
isDefaultSearchEngineEnforced_: function(pref) {
return pref.enforcement == chrome.settingsPrivate.Enforcement.ENFORCED;
},
}); });
...@@ -63,6 +63,7 @@ ...@@ -63,6 +63,7 @@
#if defined(OS_CHROMEOS) #if defined(OS_CHROMEOS)
#include "ash/public/cpp/ash_switches.h" #include "ash/public/cpp/ash_switches.h"
#include "ash/public/interfaces/voice_interaction_controller.mojom.h" #include "ash/public/interfaces/voice_interaction_controller.mojom.h"
#include "base/strings/strcat.h"
#include "base/system/sys_info.h" #include "base/system/sys_info.h"
#include "chrome/browser/chromeos/account_manager/account_manager_util.h" #include "chrome/browser/chromeos/account_manager/account_manager_util.h"
#include "chrome/browser/chromeos/arc/arc_util.h" #include "chrome/browser/chromeos/arc/arc_util.h"
...@@ -77,6 +78,7 @@ ...@@ -77,6 +78,7 @@
#include "chrome/browser/ui/webui/chromeos/network_element_localized_strings_provider.h" #include "chrome/browser/ui/webui/chromeos/network_element_localized_strings_provider.h"
#include "chrome/browser/ui/webui/chromeos/smb_shares/smb_shares_localized_strings_provider.h" #include "chrome/browser/ui/webui/chromeos/smb_shares/smb_shares_localized_strings_provider.h"
#include "chrome/common/pref_names.h" #include "chrome/common/pref_names.h"
#include "chrome/common/webui_url_constants.h"
#include "chromeos/constants/chromeos_features.h" #include "chromeos/constants/chromeos_features.h"
#include "chromeos/constants/chromeos_switches.h" #include "chromeos/constants/chromeos_switches.h"
#include "chromeos/services/assistant/public/features.h" #include "chromeos/services/assistant/public/features.h"
...@@ -2296,6 +2298,12 @@ void AddSearchStrings(content::WebUIDataSource* html_source, Profile* profile) { ...@@ -2296,6 +2298,12 @@ void AddSearchStrings(content::WebUIDataSource* html_source, Profile* profile) {
base::ASCIIToUTF16(chrome::kOmniboxLearnMoreURL)); base::ASCIIToUTF16(chrome::kOmniboxLearnMoreURL));
html_source->AddString("searchExplanation", search_explanation_text); html_source->AddString("searchExplanation", search_explanation_text);
#if defined(OS_CHROMEOS) #if defined(OS_CHROMEOS)
std::string settings_search_url =
base::StrCat({chrome::kChromeUISettingsURL, chrome::kSearchSubPage});
html_source->AddString(
"osSearchEngineLabel",
l10n_util::GetStringFUTF16(IDS_OS_SETTINGS_SEARCH_ENGINE_LABEL,
base::ASCIIToUTF16(settings_search_url)));
html_source->AddBoolean("isAssistantAllowed", is_assistant_allowed); html_source->AddBoolean("isAssistantAllowed", is_assistant_allowed);
#endif #endif
} }
......
...@@ -348,6 +348,7 @@ const char kPaymentsSubPage[] = "payments"; ...@@ -348,6 +348,7 @@ const char kPaymentsSubPage[] = "payments";
const char kPrintingSettingsSubPage[] = "printing"; const char kPrintingSettingsSubPage[] = "printing";
const char kPrivacySubPage[] = "privacy"; const char kPrivacySubPage[] = "privacy";
const char kResetProfileSettingsSubPage[] = "resetProfileSettings"; const char kResetProfileSettingsSubPage[] = "resetProfileSettings";
const char kSearchSubPage[] = "search";
const char kSearchEnginesSubPage[] = "searchEngines"; const char kSearchEnginesSubPage[] = "searchEngines";
const char kSignOutSubPage[] = "signOut"; const char kSignOutSubPage[] = "signOut";
const char kSyncSetupSubPage[] = "syncSetup"; const char kSyncSetupSubPage[] = "syncSetup";
......
...@@ -310,6 +310,7 @@ extern const char kPeopleSubPage[]; ...@@ -310,6 +310,7 @@ extern const char kPeopleSubPage[];
extern const char kPrintingSettingsSubPage[]; extern const char kPrintingSettingsSubPage[];
extern const char kPrivacySubPage[]; extern const char kPrivacySubPage[];
extern const char kResetProfileSettingsSubPage[]; extern const char kResetProfileSettingsSubPage[];
extern const char kSearchSubPage[];
extern const char kSearchEnginesSubPage[]; extern const char kSearchEnginesSubPage[];
extern const char kSignOutSubPage[]; extern const char kSignOutSubPage[];
extern const char kSyncSetupSubPage[]; extern const char kSyncSetupSubPage[];
......
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