Commit 423bc8f5 authored by My Nguyen's avatar My Nguyen Committed by Commit Bot

[OsSettingsLanguages] Update spellcheck pref when dictionary is empty

The logic in settings side is added in https://crrev.com/c/1600384.
Main logic is to turn off kSpellcheckEnable when spell check is off
for the 1 supported language.

This is equivalent to turning off kSpellcheckEnable when
hun_dictionaries is empty as it has similar origin to spellCheckEnable.
They both come from users' spellcheck dictionaries pref, forced and
blocked dictionaries list.

More background: http://go/cros-lang-settings-spell-check-migrate

Update the test in languages_page_test.js to better reflect users's
action.

Side effect:
Originally, when users turn off spell check for all languages, spell
check is left on. Now, when they turn the last one off, spell check is
off as well.

Related CL: https://crrev.com/c/2360093
Drive-by: Remove unused SetSingleLanguageDictionary function in test.

Bug: 1113439
Change-Id: Id8700c3b357da53e22e5f79420b163c8ef50b2d8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2390300Reviewed-by: default avatarGuillaume Jenkins <gujen@google.com>
Reviewed-by: default avatarRouslan Solomakhin <rouslan@chromium.org>
Reviewed-by: default avatarKyle Horimoto <khorimoto@chromium.org>
Commit-Queue: My Nguyen <myy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#805734}
parent f9edd969
...@@ -106,9 +106,6 @@ Polymer({ ...@@ -106,9 +106,6 @@ Polymer({
return []; return [];
}, },
}, },
/** @private {string|undefined} */
languageSyncedWithBrowserEnableSpellchecking_: String,
// </if> // </if>
/** /**
...@@ -641,29 +638,12 @@ Polymer({ ...@@ -641,29 +638,12 @@ Polymer({
// Hide list of spell check languages if there is only 1 language // Hide list of spell check languages if there is only 1 language
// and we don't need to display any errors for that language // and we don't need to display any errors for that language
// TODO(crbug/1124888): Make hideSpellCheckLanugages_ a computed property
this.hideSpellCheckLanguages_ = !singleLanguage.isManaged && this.hideSpellCheckLanguages_ = !singleLanguage.isManaged &&
singleLanguage.downloadDictionaryFailureCount === 0; singleLanguage.downloadDictionaryFailureCount === 0;
// Turn off spell check if spell check for the 1 remaining language is
// off
if (!singleLanguage.spellCheckEnabled) {
this.setPrefValue('browser.enable_spellchecking', false);
this.languageSyncedWithBrowserEnableSpellchecking_ =
singleLanguage.language.code;
}
// Undo the sync if spell check appeared as turned off for the language
// because a download was still in progress. This only occurs when
// Settings is loaded for the very first time and dictionaries have not
// been downloaded yet.
if (this.languageSyncedWithBrowserEnableSpellchecking_ ===
singleLanguage.language.code &&
singleLanguage.spellCheckEnabled) {
this.setPrefValue('browser.enable_spellchecking', true);
}
} else { } else {
this.hideSpellCheckLanguages_ = false; this.hideSpellCheckLanguages_ = false;
this.languageSyncedWithBrowserEnableSpellchecking_ = undefined;
} }
}, },
......
...@@ -829,8 +829,15 @@ void SpellcheckService::OnSpellCheckDictionariesChanged() { ...@@ -829,8 +829,15 @@ void SpellcheckService::OnSpellCheckDictionariesChanged() {
// If there are no hunspell dictionaries to load, then immediately let the // If there are no hunspell dictionaries to load, then immediately let the
// renderers know the new state. // renderers know the new state.
if (hunspell_dictionaries_.empty()) if (hunspell_dictionaries_.empty()) {
#if !defined(OS_MAC)
// Only update non-MacOS platform because basic spell check on Mac OS
// is controlled by OS and doesn't depend on users' dictionaries pref
user_prefs::UserPrefs::Get(context_)->SetBoolean(
spellcheck::prefs::kSpellCheckEnable, false);
#endif // !defined(OS_MAC)
InitForAllRenderers(); InitForAllRenderers();
}
} }
void SpellcheckService::OnUseSpellingServiceChanged() { void SpellcheckService::OnUseSpellingServiceChanged() {
......
...@@ -149,11 +149,6 @@ class SpellcheckServiceBrowserTest : public InProcessBrowserTest, ...@@ -149,11 +149,6 @@ class SpellcheckServiceBrowserTest : public InProcessBrowserTest,
spellcheck->OnCustomDictionaryChanged(change); spellcheck->OnCustomDictionaryChanged(change);
} }
void SetSingleLanguageDictionary(const std::string& single_dictionary) {
prefs_->SetString(spellcheck::prefs::kSpellCheckDictionary,
single_dictionary);
}
void SetMultiLingualDictionaries(const std::string& multiple_dictionaries) { void SetMultiLingualDictionaries(const std::string& multiple_dictionaries) {
base::ListValue dictionaries_value; base::ListValue dictionaries_value;
dictionaries_value.AppendStrings( dictionaries_value.AppendStrings(
...@@ -335,6 +330,16 @@ IN_PROC_BROWSER_TEST_F(SpellcheckServiceBrowserTest, ...@@ -335,6 +330,16 @@ IN_PROC_BROWSER_TEST_F(SpellcheckServiceBrowserTest,
GetPrefs()->GetBoolean(spellcheck::prefs::kSpellCheckUseSpellingService)); GetPrefs()->GetBoolean(spellcheck::prefs::kSpellCheckUseSpellingService));
} }
#if !defined(OS_MAC)
IN_PROC_BROWSER_TEST_F(SpellcheckServiceBrowserTest,
DisableSpellcheckIfDictionaryIsEmpty) {
InitSpellcheck(true, "", "en-US");
SetMultiLingualDictionaries("");
EXPECT_FALSE(GetPrefs()->GetBoolean(spellcheck::prefs::kSpellCheckEnable));
}
#endif // !defined(OS_MAC)
// Removing a spellcheck language from accept languages should remove it from // Removing a spellcheck language from accept languages should remove it from
// spellcheck languages list as well. // spellcheck languages list as well.
IN_PROC_BROWSER_TEST_F(SpellcheckServiceBrowserTest, IN_PROC_BROWSER_TEST_F(SpellcheckServiceBrowserTest,
......
...@@ -631,27 +631,34 @@ suite('languages page', function() { ...@@ -631,27 +631,34 @@ suite('languages page', function() {
const list = languagesPage.$.spellCheckLanguagesList; const list = languagesPage.$.spellCheckLanguagesList;
assertFalse(list.hidden); assertFalse(list.hidden);
assertTrue(languagesPage.$$('#enableSpellcheckingToggle').checked);
assertDeepEquals(
['en-US'], languageHelper.getPref('spellcheck.dictionaries').value);
languageHelper.setPrefValue('intl.accept_languages', 'en-US'); // Update supported languages to just 1 language should hide list.
if (isChromeOS) { languageHelper.setPrefValue(languagesPref, 'en-US');
languageHelper.setPrefValue( flush();
'settings.language.preferred_languages', 'en-US'); assertTrue(list.hidden);
}
// Disable spell check should keep list hidden and remove the single
// language from dictionaries.
languagesPage.$$('#enableSpellcheckingToggle').click();
flush();
// Update supported languages to just 1 language English with spell
// check disabled for that language
languageHelper.setPrefValue('spellcheck.dictionaries', []);
assertTrue(list.hidden); assertTrue(list.hidden);
assertFalse(languageHelper.getPref('browser.enable_spellchecking').value); assertFalse(languagesPage.$$('#enableSpellcheckingToggle').checked);
assertDeepEquals(
[], languageHelper.getPref('spellcheck.dictionaries').value);
// Enable spell check should keep list hidden and add the single language
// to dictionaries.
languagesPage.$$('#enableSpellcheckingToggle').click();
flush();
// Update supported languages to just 1 language English that finished
// downloading and is now ready
languageHelper.setPrefValue('spellcheck.dictionaries', ['en-US']);
languageHelper.set('languages.enabled.0.downloadDictionaryStatus', {
isReady: true,
});
assertTrue(list.hidden); assertTrue(list.hidden);
assertTrue(languageHelper.getPref('browser.enable_spellchecking').value); assertTrue(languagesPage.$$('#enableSpellcheckingToggle').checked);
assertDeepEquals(
['en-US'], languageHelper.getPref('spellcheck.dictionaries').value);
}); });
test('no supported languages', () => { test('no supported languages', () => {
......
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