Skip to content

GitLab

T
tangled
Commit 9cb3dee4 authored by Daniel Classon's avatar Daniel Classon Committed by Commit Bot
Browse files
Options

[SearchPage] Fix "Ok Google" search tags from not showing up 

The "Ok Google" tags should show up all the time, not just when Ok
Google is enabled. While this fixed the bug with the tags not showing
up, the search tag priority is still really low. In my testing,
"Hey goog" and "Okay goog" led to the search results being shown, but as
soon as Google is added, other Google related search items take
precedence. Fix this by changing search priority from Low to Medium.

Fixed: 1136629
Change-Id: Id13ae98b5a699a0853382413730c8079cdf658fc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2476521
Commit-Queue: Daniel Classon <dclasson@google.com>
Reviewed-by: default avatarKyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#818172}
parent 899e736f
Hide whitespace changes
Inline Side-by-side
Showing with 97 additions and 160 deletions
chrome/browser/resources/settings/chromeos/google_assistant_page/google_assistant_page.html
View file @ 9cb3dee4
......@@ -71,48 +71,51 @@
</settings-toggle-button>
</div>
</template>
<settings-toggle-button id="google-assistant-hotword-enable"
class="hr"
pref="{{prefs.settings.voice_interaction.hotword.enabled}}"
label="$i18n{googleAssistantEnableHotword}"
sub-label="$i18n{googleAssistantEnableHotwordDescription}"
on-settings-boolean-control-change="onEnableHotwordChange_"
hidden="[[!hotwordDspAvailable_]]"
deep-link-focus-id$="[[Setting.kAssistantOkGoogle]]">
</settings-toggle-button>
<div class="settings-box" id="dsp-hotword-container"
hidden="[[hotwordDspAvailable_]]">
<div class="start text-area settings-box-text" aria-hidden="true">
<div class="label" id="googleAssistantEnableHotword">
$i18n{googleAssistantEnableHotword}
</div>
<div class="secondary label"
id="googleAssistantEnableHotwordDescription">
$i18n{googleAssistantEnableHotwordWithoutDspDescription}
<template is="dom-if" if="[[hotwordDspAvailable_]]" restamp>
<settings-toggle-button id="google-assistant-hotword-enable"
class="hr"
pref="{{prefs.settings.voice_interaction.hotword.enabled}}"
label="$i18n{googleAssistantEnableHotword}"
sub-label="$i18n{googleAssistantEnableHotwordDescription}"
on-settings-boolean-control-change="onEnableHotwordChange_"
deep-link-focus-id$="[[Setting.kAssistantOkGoogle]]">
</settings-toggle-button>
</template>
<template is="dom-if" if="[[!hotwordDspAvailable_]]" restamp>
<div class="settings-box" id="dsp-hotword-container">
<div class="start text-area settings-box-text" aria-hidden="true">
<div class="label" id="googleAssistantEnableHotword">
$i18n{googleAssistantEnableHotword}
</div>
<div class="secondary label"
id="googleAssistantEnableHotwordDescription">
$i18n{googleAssistantEnableHotwordWithoutDspDescription}
</div>
</div>
</div>
<template is="dom-if" if="[[hotwordEnforced_]]" restamp>
<cr-policy-pref-indicator id="hotword-policy-pref-indicator"
pref="{{prefs.settings.voice_interaction.hotword.enabled}}">
</cr-policy-pref-indicator>
</template>
<!-- We don't use the settings_dropdown_menu for the dspHotwordState
drop down, because the dspHotwordState manages 2 preferences.
The settings_dropdown_menu is designed to manage only a single
preference. -->
<select id="dsp-hotword-state" class="md-select"
aria-labelledby="googleAssistantEnableHotword"
aria-describedby="googleAssistantEnableHotwordDescription"
on-change="onDspHotwordStateChange_"
disabled="[[hotwordEnforced_]]">
<template is="dom-repeat" items="[[hotwordDropdownList_]]">
<option value="[[item.value]]"
selected="[[isDspHotwordStateMatch_(item.value)]]">
[[item.name]]
</option>
<template is="dom-if" if="[[hotwordEnforced_]]" restamp>
<cr-policy-pref-indicator id="hotword-policy-pref-indicator"
pref="{{prefs.settings.voice_interaction.hotword.enabled}}">
</cr-policy-pref-indicator>
</template>
</select>
</div>
<!-- We don't use the settings_dropdown_menu for the dspHotwordState
drop down, because the dspHotwordState manages 2 preferences.
The settings_dropdown_menu is designed to manage only a single
preference. -->
<select id="dsp-hotword-state" class="md-select"
aria-labelledby="googleAssistantEnableHotword"
aria-describedby="googleAssistantEnableHotwordDescription"
on-change="onDspHotwordStateChange_"
disabled="[[hotwordEnforced_]]"
deep-link-focus-id$="[[Setting.kAssistantOkGoogle]]">
<template is="dom-repeat" items="[[hotwordDropdownList_]]">
<option value="[[item.value]]"
selected="[[isDspHotwordStateMatch_(item.value)]]">
[[item.name]]
</option>
</template>
</select>
</div>
</template>
<template is="dom-if" if="[[shouldShowVoiceMatchSettings_]]">
<div class="settings-box continuation embedded">
<div class="start text-area settings-box-text">
......
chrome/browser/ui/webui/settings/chromeos/search_section.cc
View file @ 9cb3dee4
......@@ -56,6 +56,15 @@ const std::vector<SearchConcept>& GetAssistantSearchConcepts() {
mojom::SearchResultDefaultRank::kMedium,
mojom::SearchResultType::kSubpage,
{.subpage = mojom::Subpage::kAssistant}},
{IDS_OS_SETTINGS_TAG_ASSISTANT_OK_GOOGLE,
mojom::kAssistantSubpagePath,
mojom::SearchResultIcon::kAssistant,
mojom::SearchResultDefaultRank::kMedium,
mojom::SearchResultType::kSetting,
{.setting = mojom::Setting::kAssistantOkGoogle},
{IDS_OS_SETTINGS_TAG_ASSISTANT_OK_GOOGLE_ALT1,
IDS_OS_SETTINGS_TAG_ASSISTANT_OK_GOOGLE_ALT2,
SearchConcept::kAltTagEnd}},
});
return *tags;
}
......@@ -117,21 +126,6 @@ const std::vector<SearchConcept>& GetAssistantQuickAnswersSearchConcepts() {
return *tags;
}
const std::vector<SearchConcept>& GetAssistantHotwordDspSearchConcepts() {
static const base::NoDestructor<std::vector<SearchConcept>> tags({
{IDS_OS_SETTINGS_TAG_ASSISTANT_OK_GOOGLE,
mojom::kAssistantSubpagePath,
mojom::SearchResultIcon::kAssistant,
mojom::SearchResultDefaultRank::kLow,
mojom::SearchResultType::kSetting,
{.setting = mojom::Setting::kAssistantOkGoogle},
{IDS_OS_SETTINGS_TAG_ASSISTANT_OK_GOOGLE_ALT1,
IDS_OS_SETTINGS_TAG_ASSISTANT_OK_GOOGLE_ALT2,
SearchConcept::kAltTagEnd}},
});
return *tags;
}
const std::vector<SearchConcept>& GetAssistantVoiceMatchSearchConcepts() {
static const base::NoDestructor<std::vector<SearchConcept>> tags({
{IDS_OS_SETTINGS_TAG_ASSISTANT_TRAIN_VOICE_MODEL,
......@@ -341,7 +335,6 @@ void SearchSection::UpdateAssistantSearchTags() {
updater.RemoveSearchTags(GetAssistantOnSearchConcepts());
updater.RemoveSearchTags(GetAssistantOffSearchConcepts());
updater.RemoveSearchTags(GetAssistantQuickAnswersSearchConcepts());
updater.RemoveSearchTags(GetAssistantHotwordDspSearchConcepts());
updater.RemoveSearchTags(GetAssistantVoiceMatchSearchConcepts());
ash::AssistantState* assistant_state = ash::AssistantState::Get();
......@@ -361,9 +354,6 @@ void SearchSection::UpdateAssistantSearchTags() {
updater.AddSearchTags(GetAssistantQuickAnswersSearchConcepts());
}
if (IsHotwordDspAvailable())
updater.AddSearchTags(GetAssistantHotwordDspSearchConcepts());
if (IsVoiceMatchAllowed() && assistant_state->hotword_enabled() &&
assistant_state->hotword_enabled().value() &&
assistant_state->consent_status() &&
......
chrome/test/data/webui/settings/chromeos/google_assistant_page_test.js
View file @ 9cb3dee4
......@@ -170,49 +170,6 @@ suite('GoogleAssistantHandler', function() {
button = page.$$('#google-assistant-hotword-enable');
assertTrue(!!button);
assertFalse(button.hasAttribute('hidden'));
});
test('hotwordToggleEnabled', function() {
let button = page.$$('#google-assistant-hotword-enable');
let indicator = page.$$('#hotword-policy-pref-indicator');
assertFalse(!!button);
assertFalse(!!indicator);
page.setPrefValue('settings.voice_interaction.enabled', true);
page.set('prefs.settings.voice_interaction.hotword.enabled', {
enforcement: chrome.settingsPrivate.Enforcement.RECOMMENDED,
value: true,
});
Polymer.dom.flush();
button = page.$$('#dsp-hotword-state');
indicator = page.$$('#hotword-policy-pref-indicator');
assertTrue(!!button);
assertFalse(!!indicator);
assertFalse(button.hasAttribute('disabled'));
});
test('hotwordToggleDisabled', function() {
let button = page.$$('#google-assistant-hotword-enable');
let indicator = page.$$('#hotword-policy-pref-indicator');
assertFalse(!!button);
assertFalse(!!indicator);
page.setPrefValue('settings.voice_interaction.enabled', true);
page.set('prefs.settings.voice_interaction.hotword.enabled', {
enforcement: chrome.settingsPrivate.Enforcement.ENFORCED,
value: true,
});
Polymer.dom.flush();
button = page.$$('#dsp-hotword-state');
indicator = page.$$('#hotword-policy-pref-indicator');
assertTrue(!!button);
assertTrue(!!indicator);
assertTrue(button.hasAttribute('disabled'));
});
test('tapOnRetrainVoiceModel', function() {
......@@ -327,52 +284,6 @@ suite('GoogleAssistantHandler', function() {
return browserProxy.whenCalled('showGoogleAssistantSettings');
});
test('dspHotwordDropdownEnabled', function() {
let dropdown = page.$$('#dsp-hotword-state');
assertFalse(!!dropdown);
page.setPrefValue('settings.voice_interaction.enabled', true);
page.set('prefs.settings.voice_interaction.hotword.enabled', {
enforcement: chrome.settingsPrivate.Enforcement.RECOMMENDED,
value: true,
});
Polymer.dom.flush();
dropdown = page.$$('#dsp-hotword-state');
assertTrue(!!dropdown);
assertFalse(dropdown.hasAttribute('disabled'));
});
test('dspHotwordDropdownDisabled', function() {
let dropdown = page.$$('#dsp-hotword-state');
assertFalse(!!dropdown);
page.setPrefValue('settings.voice_interaction.enabled', true);
page.set('prefs.settings.voice_interaction.hotword.enabled', {
enforcement: chrome.settingsPrivate.Enforcement.ENFORCED,
value: true,
});
Polymer.dom.flush();
dropdown = page.$$('#dsp-hotword-state');
assertTrue(!!dropdown);
assertTrue(dropdown.hasAttribute('disabled'));
});
test('dspHotwordDropdownVisibility', function() {
let dropdown = page.$$('#dsp-hotword-container');
assertFalse(!!dropdown);
page.setPrefValue('settings.voice_interaction.enabled', true);
Polymer.dom.flush();
dropdown = page.$$('#dsp-hotword-container');
assertTrue(!!dropdown);
assertTrue(dropdown.hasAttribute('hidden'));
});
test('assistantDisabledByPolicy', function() {
let button = page.$$('#google-assistant-enable');
assertTrue(!!button);
......@@ -393,7 +304,7 @@ suite('GoogleAssistantHandler', function() {
});
});
suite('GoogleAssistantHandlerWihtNoDspHotword', function() {
suite('GoogleAssistantHandlerWithNoDspHotword', function() {
/** @type {SettingsGoogleAssistantPageElement} */
let page = null;
......@@ -420,6 +331,7 @@ suite('GoogleAssistantHandlerWihtNoDspHotword', function() {
page = document.createElement('settings-google-assistant-page');
page.prefs = prefElement.prefs;
document.body.appendChild(page);
Polymer.dom.flush();
});
});
......@@ -437,30 +349,62 @@ suite('GoogleAssistantHandlerWihtNoDspHotword', function() {
Polymer.dom.flush();
}
test('hotwordToggleVisibilityWihtNoDspHotword', function() {
let button = page.$$('#google-assistant-hotword-enable');
assertFalse(!!button);
test('hotwordToggleVisibilityWithNoDspHotword', function() {
let toggle = page.$$('#google-assistant-hotword-enable');
assertFalse(!!toggle);
page.setPrefValue('settings.voice_interaction.enabled', true);
Polymer.dom.flush();
button = page.$$('#google-assistant-hotword-enable');
assertTrue(!!button);
assertTrue(button.hasAttribute('hidden'));
toggle = page.$$('#google-assistant-hotword-enable');
assertFalse(!!toggle);
});
test('dspHotwordDropdownVisibilityWihtNoDspHotword', function() {
test('dspHotwordDropdownVisibilityWithNoDspHotword', function() {
let container = page.$$('#dsp-hotword-container');
assertFalse(!!container);
page.setPrefValue('settings.voice_interaction.enabled', true);
loadTimeData.overrideValues({
hotwordDspAvailable: true,
});
Polymer.dom.flush();
container = page.$$('#dsp-hotword-container');
assertFalse(container.hasAttribute('hidden'));
assertTrue(!!container);
});
test('dspHotwordDropdownIndicatorEnabled', function() {
let indicator = page.$$('#hotword-policy-pref-indicator');
assertFalse(!!indicator);
page.setPrefValue('settings.voice_interaction.enabled', true);
page.set('prefs.settings.voice_interaction.hotword.enabled', {
enforcement: chrome.settingsPrivate.Enforcement.RECOMMENDED,
value: true,
});
Polymer.dom.flush();
const dropdown = page.$$('#dsp-hotword-state');
indicator = page.$$('#hotword-policy-pref-indicator');
assertTrue(!!dropdown);
assertFalse(!!indicator);
assertFalse(dropdown.hasAttribute('disabled'));
});
test('dspHotwordDropdownIndicatorDisabled', function() {
let indicator = page.$$('#hotword-policy-pref-indicator');
assertFalse(!!indicator);
page.setPrefValue('settings.voice_interaction.enabled', true);
page.set('prefs.settings.voice_interaction.hotword.enabled', {
enforcement: chrome.settingsPrivate.Enforcement.ENFORCED,
value: true,
});
Polymer.dom.flush();
const dropdown = page.$$('#dsp-hotword-state');
indicator = page.$$('#hotword-policy-pref-indicator');
assertTrue(!!dropdown);
assertTrue(!!indicator);
assertTrue(dropdown.hasAttribute('disabled'));
});
test('dspHotwordDropdownSelection', function() {
......
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