Commit 0abf04ae authored by Akihiro Ota's avatar Akihiro Ota Committed by Commit Bot

ChromeVox: Keep track of the last spoken locale.

This change further refines the locale announcement given when
switching between locales within the same language. We
announce the locale once when switching to more specific locales,
e.g. en -> en-us. We never announce the locale when switching to less
specific locales, e.g. en-us -> en. Finally, all subsequent switches
from en -> en-us will not be announced. The above logic only applies
when we stay within the same language. If the language changes e.g.
en -> pt, then the above state will reset.

This change also adds a test to confirm behavior. We also removed a
test since it can be merged into the new test.

same language.

Bug: 1098548
Change-Id: Ie7015f68bc6c6c747065d815697584f5fd642092
AX-Relnotes: Refine voice switching behavior when switching within the
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2466497
Commit-Queue: Akihiro Ota <akihiroota@chromium.org>
Reviewed-by: default avatarDavid Tseng <dtseng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#816295}
parent c8387ef0
...@@ -23,6 +23,8 @@ LocaleOutputHelper = class { ...@@ -23,6 +23,8 @@ LocaleOutputHelper = class {
chrome.i18n.getUILanguage().toLowerCase(); chrome.i18n.getUILanguage().toLowerCase();
/** @private {string} */ /** @private {string} */
this.currentLocale_ = LocaleOutputHelper.BROWSER_UI_LOCALE_ || ''; this.currentLocale_ = LocaleOutputHelper.BROWSER_UI_LOCALE_ || '';
/** @private {string} */
this.lastSpokenLocale_ = this.currentLocale_;
/** /**
* Confidence threshold to meet before assigning sub-node language. * Confidence threshold to meet before assigning sub-node language.
* @const * @const
...@@ -64,6 +66,7 @@ LocaleOutputHelper = class { ...@@ -64,6 +66,7 @@ LocaleOutputHelper = class {
if (this.hasVoiceForLocale_(newLocale)) { if (this.hasVoiceForLocale_(newLocale)) {
this.setCurrentLocale_(newLocale); this.setCurrentLocale_(newLocale);
if (shouldAnnounce) { if (shouldAnnounce) {
this.lastSpokenLocale_ = newLocale;
// Prepend the human-readable locale to |outputString|. // Prepend the human-readable locale to |outputString|.
const displayLanguage = const displayLanguage =
chrome.accessibilityPrivate.getDisplayNameForLocale( chrome.accessibilityPrivate.getDisplayNameForLocale(
...@@ -141,21 +144,20 @@ LocaleOutputHelper = class { ...@@ -141,21 +144,20 @@ LocaleOutputHelper = class {
* @private * @private
*/ */
shouldAnnounceLocale_(newLocale) { shouldAnnounceLocale_(newLocale) {
// Note: currentLocale_ and newLocale are guaranteed to contain a language const [lastSpokenLanguage, lastSpokenCountry] =
// code. However, they might not contain a country code. this.lastSpokenLocale_.split('-');
const [currentLanguage, currentCountry] = this.currentLocale_.split('-');
const [newLanguage, newCountry] = newLocale.split('-'); const [newLanguage, newCountry] = newLocale.split('-');
if (currentLanguage !== newLanguage) { if (lastSpokenLanguage !== newLanguage) {
return true; return true;
} }
if (!currentCountry || !newCountry) { if (!newCountry) {
// If one of the countries is blank, then we don't want to announce the // If |newCountry| is undefined, then we don't want to announce the
// locale. For example, we don't want to announce 'en' -> 'en-us'. // locale. For example, we don't want to announce 'en-us' -> 'en'.
return false; return false;
} }
return currentCountry !== newCountry; return lastSpokenCountry !== newCountry;
} }
// =============== Static Methods ============== // =============== Static Methods ==============
......
...@@ -578,42 +578,31 @@ TEST_F( ...@@ -578,42 +578,31 @@ TEST_F(
}); });
}); });
TEST_F( // Tests logic in shouldAnnounceLocale_(). We only announce the locale once when
'ChromeVoxLocaleOutputHelperTest', 'DoNotAnnounceLocaleFirstCase', // transitioning to more specific locales, e.g. 'en' -> 'en-us'. Transitions to
function() { // less specific locales, e.g. 'en-us' -> 'en' should not be announced. Finally,
const mockFeedback = this.createMockFeedback(); // subsequent transitions to the same locale, e.g. 'en' -> 'en-us' should not be
this.runWithLoadedTree( // announced.
` TEST_F('ChromeVoxLocaleOutputHelperTest', 'MaybeAnnounceLocale', function() {
const mockFeedback = this.createMockFeedback();
this.runWithLoadedTree(
`
<p lang="en">Start</p> <p lang="en">Start</p>
<p lang="en-us">End</p> <p lang="en-ca">Middle</p>
<p lang="en">Penultimate</p>
<p lang="en-ca">End</p>
`, `,
function() { function() {
localStorage['languageSwitching'] = 'true'; localStorage['languageSwitching'] = 'true';
this.setAvailableVoices(); this.setAvailableVoices();
mockFeedback.call(doCmd('jumpToTop')) mockFeedback.call(doCmd('jumpToTop'))
.expectSpeechWithLocale('en', 'Start') .expectSpeechWithLocale('en', 'Start')
.call(doCmd('nextObject')) .call(doCmd('nextObject'))
.expectSpeechWithLocale('en-us', 'End') .expectSpeechWithLocale('en-ca', 'English (Canada): Middle')
.replay(); .call(doCmd('nextObject'))
}); .expectSpeechWithLocale('en', 'Penultimate')
}); .call(doCmd('nextObject'))
.expectSpeechWithLocale('en-ca', 'End')
TEST_F( .replay();
'ChromeVoxLocaleOutputHelperTest', 'DoNotAnnounceLocaleSecondCase', });
function() { });
const mockFeedback = this.createMockFeedback();
this.runWithLoadedTree(
`
<p lang="en-us">Start</p>
<p lang="en">End</p>
`,
function() {
localStorage['languageSwitching'] = 'true';
this.setAvailableVoices();
mockFeedback.call(doCmd('jumpToTop'))
.expectSpeechWithLocale('en-us', 'Start')
.call(doCmd('nextObject'))
.expectSpeechWithLocale('en', 'End')
.replay();
});
});
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