Commit 7c01188e authored by James Cook's avatar James Cook Committed by Commit Bot

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

This reverts commit b0f26697.

Reason for revert: After more PM/UX discussion, we want to restore
the search engine select menu in OS settings.

Original change's description:
> 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/+/1640601
> Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
> Commit-Queue: James Cook <jamescook@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#665641}

TBR=jamescook@chromium.org,xiyuan@chromium.org

Bug: 960937
Change-Id: I463e11f209afedc885fb07b1a12593f7450c1629
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1716968Reviewed-by: default avatarJames Cook <jamescook@chromium.org>
Commit-Queue: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#680586}
parent 5c177746
...@@ -14,6 +14,7 @@ js_library("os_search_page") { ...@@ -14,6 +14,7 @@ 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/md_select_css.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="../../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">
...@@ -17,6 +21,12 @@ ...@@ -17,6 +21,12 @@
<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;
} }
...@@ -24,10 +34,46 @@ ...@@ -24,10 +34,46 @@
<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 settings-box-text">
$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 first two-line" <div id="assistantSubpageTrigger" class="settings-box two-line"
on-click="onGoogleAssistantTap_" actionable> on-click="onGoogleAssistantTap_" actionable>
<div class="start"> <div class="start">
$i18n{searchGoogleAssistant} $i18n{searchGoogleAssistant}
...@@ -40,17 +86,7 @@ ...@@ -40,17 +86,7 @@
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,6 +14,20 @@ Polymer({ ...@@ -14,6 +14,20 @@ 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,
...@@ -26,8 +40,22 @@ Polymer({ ...@@ -26,8 +40,22 @@ 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(
...@@ -36,6 +64,18 @@ Polymer({ ...@@ -36,6 +64,18 @@ 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_);
...@@ -52,4 +92,22 @@ Polymer({ ...@@ -52,4 +92,22 @@ 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;
},
}); });
...@@ -27,6 +27,7 @@ ...@@ -27,6 +27,7 @@
#include "chrome/browser/ui/webui/settings/profile_info_handler.h" #include "chrome/browser/ui/webui/settings/profile_info_handler.h"
#include "chrome/browser/ui/webui/settings/protocol_handlers_handler.h" #include "chrome/browser/ui/webui/settings/protocol_handlers_handler.h"
#include "chrome/browser/ui/webui/settings/reset_settings_handler.h" #include "chrome/browser/ui/webui/settings/reset_settings_handler.h"
#include "chrome/browser/ui/webui/settings/search_engines_handler.h"
#include "chrome/browser/ui/webui/settings/settings_cookies_view_handler.h" #include "chrome/browser/ui/webui/settings/settings_cookies_view_handler.h"
#include "chrome/browser/ui/webui/settings/settings_localized_strings_provider.h" #include "chrome/browser/ui/webui/settings/settings_localized_strings_provider.h"
#include "chrome/browser/ui/webui/settings/settings_media_devices_selection_handler.h" #include "chrome/browser/ui/webui/settings/settings_media_devices_selection_handler.h"
...@@ -83,6 +84,8 @@ OSSettingsUI::OSSettingsUI(content::WebUI* web_ui) ...@@ -83,6 +84,8 @@ OSSettingsUI::OSSettingsUI(content::WebUI* web_ui)
std::make_unique<::settings::ProfileInfoHandler>(profile)); std::make_unique<::settings::ProfileInfoHandler>(profile));
AddSettingsPageUIHandler( AddSettingsPageUIHandler(
std::make_unique<::settings::ProtocolHandlersHandler>()); std::make_unique<::settings::ProtocolHandlersHandler>());
AddSettingsPageUIHandler(
std::make_unique<::settings::SearchEnginesHandler>(profile));
html_source->AddBoolean("unifiedConsentEnabled", html_source->AddBoolean("unifiedConsentEnabled",
unified_consent::IsUnifiedConsentFeatureEnabled()); unified_consent::IsUnifiedConsentFeatureEnabled());
......
...@@ -65,7 +65,6 @@ ...@@ -65,7 +65,6 @@
#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"
...@@ -82,7 +81,6 @@ ...@@ -82,7 +81,6 @@
#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"
...@@ -2502,12 +2500,6 @@ void AddSearchStrings(content::WebUIDataSource* html_source, Profile* profile) { ...@@ -2502,12 +2500,6 @@ 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
} }
......
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