Commit 636e8ad5 authored by Akihiro Ota's avatar Akihiro Ota Committed by Chromium LUCI CQ

TTS: Perform case-insensitive comparison on locales when choosing voice.

Consider the following scenario:
Voice1.lang = "en-us" (all lower case)
Voice2.lang = "en-US" (country code is upper case)
Utterance.lang = "en-us" (all lower case)
The user has set Voice2 as their preferred voice for English.

The old code would choose voice1 as the best voice for the utterance
because voice1.lang == utterance.lang. voice2.lang != utterance.lang
because of the difference in casing. This was leading to issues in
ChromeVox's voice switching behavior, where eSpeak was sometimes being
used, even if the user had set Chrome OS US English as their preferred
voice.

The tangible effect of this change is that ChromeVox voice switching
will respect the user's preferred voices.

voices.
--gtest_filter="TtsControllerTest.TestGetMatchingVoice"'. Also manually
verify the bug no longer repros after the fix.

Fixed: 1121592
AX-Relnotes: ChromeVox voice switching now respects the user's preferred
Test: 'out/cros/content_unittests
Change-Id: I0588b50fbbc4ee7f0b71741b2bb312c9d6d5b14c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2617511
Commit-Queue: Akihiro Ota <akihiroota@chromium.org>
Reviewed-by: default avatarDavid Tseng <dtseng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#842270}
parent f5e4ae45
...@@ -15,6 +15,7 @@ ...@@ -15,6 +15,7 @@
#include "base/json/json_reader.h" #include "base/json/json_reader.h"
#include "base/metrics/histogram_macros.h" #include "base/metrics/histogram_macros.h"
#include "base/metrics/user_metrics.h" #include "base/metrics/user_metrics.h"
#include "base/strings/string_util.h"
#include "base/values.h" #include "base/values.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "build/chromeos_buildflags.h" #include "build/chromeos_buildflags.h"
...@@ -691,11 +692,13 @@ int TtsControllerImpl::GetMatchingVoice(TtsUtterance* utterance, ...@@ -691,11 +692,13 @@ int TtsControllerImpl::GetMatchingVoice(TtsUtterance* utterance,
// Prefer the utterance language. // Prefer the utterance language.
if (!voice.lang.empty() && !utterance->GetLang().empty()) { if (!voice.lang.empty() && !utterance->GetLang().empty()) {
// An exact language match is worth more than a partial match. // An exact locale match is worth more than a partial match.
if (voice.lang == utterance->GetLang()) { // Convert locales to lowercase to handle cases like "en-us" vs. "en-US".
if (base::EqualsCaseInsensitiveASCII(voice.lang, utterance->GetLang())) {
score += 128; score += 128;
} else if (l10n_util::GetLanguage(voice.lang) == } else if (base::EqualsCaseInsensitiveASCII(
l10n_util::GetLanguage(utterance->GetLang())) { l10n_util::GetLanguage(voice.lang),
l10n_util::GetLanguage(utterance->GetLang()))) {
score += 64; score += 64;
} }
} }
...@@ -739,8 +742,9 @@ int TtsControllerImpl::GetMatchingVoice(TtsUtterance* utterance, ...@@ -739,8 +742,9 @@ int TtsControllerImpl::GetMatchingVoice(TtsUtterance* utterance,
if (!voice.lang.empty()) { if (!voice.lang.empty()) {
if (voice.lang == app_lang) { if (voice.lang == app_lang) {
score += 2; score += 2;
} else if (l10n_util::GetLanguage(voice.lang) == } else if (base::EqualsCaseInsensitiveASCII(
l10n_util::GetLanguage(app_lang)) { l10n_util::GetLanguage(voice.lang),
l10n_util::GetLanguage(app_lang))) {
score += 1; score += 1;
} }
} }
......
...@@ -508,6 +508,50 @@ TEST_F(TtsControllerTest, TestGetMatchingVoice) { ...@@ -508,6 +508,50 @@ TEST_F(TtsControllerTest, TestGetMatchingVoice) {
// voice0 is matched against the pref over the system language. // voice0 is matched against the pref over the system language.
TestContentBrowserClient::GetInstance()->set_application_locale("en-US"); TestContentBrowserClient::GetInstance()->set_application_locale("en-US");
EXPECT_EQ(0, controller()->GetMatchingVoice(utterance.get(), voices)); EXPECT_EQ(0, controller()->GetMatchingVoice(utterance.get(), voices));
#endif
}
{
// This block ensures that voices can be matched, even if their locale's
// casing doesn't exactly match that of the utterance e.g. "en-us" will
// match with "en-US".
std::vector<VoiceData> voices;
VoiceData voice0;
voice0.engine_id = "id0";
voice0.name = "English voice";
voice0.lang = "en-US";
voices.push_back(voice0);
VoiceData voice1;
voice1.engine_id = "id0";
voice1.name = "French voice";
voice1.lang = "fr";
voices.push_back(voice1);
std::unique_ptr<TtsUtterance> utterance(TtsUtterance::Create());
utterance->SetLang("en-us");
EXPECT_EQ(0, controller()->GetMatchingVoice(utterance.get(), voices));
utterance->SetLang("en-US");
EXPECT_EQ(0, controller()->GetMatchingVoice(utterance.get(), voices));
utterance->SetLang("EN-US");
EXPECT_EQ(0, controller()->GetMatchingVoice(utterance.get(), voices));
#if BUILDFLAG(IS_CHROMEOS_ASH)
// Add another English voice.
VoiceData voice2;
voice2.engine_id = "id1";
voice2.name = "Another English voice";
voice2.lang = "en-us";
voices.push_back(voice2);
// Set voice2 as the preferred voice for English.
TtsControllerDelegate::PreferredVoiceIds preferred_voice_ids;
preferred_voice_ids.lang_voice_id.emplace(voice2.name, voice2.engine_id);
delegate()->SetPreferredVoiceIds(preferred_voice_ids);
// Ensure that voice2 is chosen over voice0, even though the locales don't
// match exactly. The utterance has a locale of "en-US", while voice2 has
// a locale of "en-us"; this shouldn't prevent voice2 from being used.
EXPECT_EQ(2, controller()->GetMatchingVoice(utterance.get(), voices));
#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