[Merge 109][LanguageSettings] Don't show unknown translate languages

Filters unknown languages out of the the displayed always and
never translate lists in the detailed language settings.

While it is currently impossible for unknown languages to get onto a
users always or never translate list it was possible in the past. This
means that there are many older profiles that have unknown languages
on theses lists.

Don't show these languages in the settings since they are ignored
by the translate pipelines and can break the UI if we do not have
display names for them.

This CL updates fake_language_settings_private so that the always
translate list (translate_allowlist) is a dict instead of a list to
replicate the actual preference. Before this CL the always and never
translate preferences were not actually accessed by the UI in tests.
This CL tests that unknown languages are not shown in the UI for both
lists.

(cherry picked from commit 83b77f3)

Fixed: 1382938
Change-Id: I7cab10a5d23845ebefa20872e2a51f5dad57caa3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4023911
Reviewed-by: Megan Jablonski <megjablon@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Commit-Queue: Trevor Perrier <perrier@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1071950}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4034295
Auto-Submit: Trevor Perrier <perrier@chromium.org>
Cr-Commit-Position: refs/branch-heads/5414@{#116}
Cr-Branched-From: 4417ee5-refs/heads/main@{#1070088}

chrome/browser/resources/settings/languages_page/languages.ts

@@ -457,7 +457,7 @@ class SettingsLanguagesElement extends SettingsLanguagesElementBase implements
     const alwaysTranslateCodes =
         Object.keys(this.getPref('translate_allowlists').value);
     const alwaysTranslateLanguages =
-        alwaysTranslateCodes.map(code => this.getLanguage(code));
+        alwaysTranslateCodes.map((code: string) => this.getLanguage(code));
     this.set('languages.alwaysTranslate', alwaysTranslateLanguages);
   }
 

chrome/browser/resources/settings/languages_page/translate_page.html

@@ -34,7 +34,7 @@ <h2 class="flex">$i18n{automaticallyTranslateLanguages}</h2>
   <div class="list-frame">
     <div id="alwaysTranslateList" class="vertical-list">
       <template is="dom-repeat" items="[[languages.alwaysTranslate]]"
-          sort="alphabeticalSort_">
+          sort="alphabeticalSort_" filter="hasDisplayName_">
         <div class="list-item">
           <div class="start cr-padded-text">[[item.displayName]]</div>
           <cr-icon-button class="icon-delete-gray"
@@ -58,7 +58,7 @@ <h2 class="flex">$i18n{neverTranslateLanguages}</h2>
   <div class="list-frame">
     <div id="neverTranslateList" class="vertical-list">
       <template is="dom-repeat" items="[[languages.neverTranslate]]"
-          sort="alphabeticalSort_">
+          sort="alphabeticalSort_" filter="hasDisplayName_">
         <div class="list-item">
           <div class="start cr-padded-text">[[item.displayName]]</div>
           <cr-icon-button class="icon-delete-gray" value="[[item.code]]"

chrome/browser/resources/settings/languages_page/translate_page.ts

@@ -114,6 +114,15 @@ export class SettingsTranslatePageElement extends
     return first.displayName.localeCompare(second.displayName);
   }
 
+  /**
+   * A filter function to return true if language is not undefined and has a
+   * displayName.
+   */
+  private hasDisplayName_(language: chrome.languageSettingsPrivate.Language|
+                          undefined): boolean {
+    return !!language && !!language!.displayName;
+  }
+
   /**
    * Stamps and opens the Add Languages dialog, registering a listener to
    * disable the dialog's dom-if again on close.

chrome/test/data/webui/settings/fake_language_settings_private.ts

@@ -216,7 +216,9 @@ export class FakeLanguageSettingsPrivate extends TestBrowserProxy {
    */
   getAlwaysTranslateLanguages(callback: StringArrayCallback) {
     setTimeout(() => {
-      callback(this.settingsPrefs_!.get('prefs.translate_allowlists.value'));
+      const alwaysTranslateMap =
+          this.settingsPrefs_!.get('prefs.translate_allowlists.value');
+      callback(Object.keys(alwaysTranslateMap));
     });
   }
 
@@ -225,21 +227,18 @@ export class FakeLanguageSettingsPrivate extends TestBrowserProxy {
    */
   setLanguageAlwaysTranslateState(
       languageCode: string, alwaysTranslate: boolean) {
-    const alwaysTranslateList =
-        this.settingsPrefs_!.get('prefs.translate_allowlists.value');
+    // Need to create a copy of the translate_allowlist object so that
+    // preference observers are notified during tests.
+    const alwaysTranslateMap = Object.assign(
+        {}, this.settingsPrefs_!.get('prefs.translate_allowlists.value'));
     if (alwaysTranslate) {
-      if (!alwaysTranslateList.includes(languageCode)) {
-        alwaysTranslateList.push(languageCode);
-      }
+      // The target language is not used in tests so set to 'en'.
+      alwaysTranslateMap[languageCode] = 'en';
     } else {
-      const index = alwaysTranslateList.indexOf(languageCode);
-      if (index === -1) {
-        return;
-      }
-      alwaysTranslateList.splice(index, 1);
+      delete alwaysTranslateMap[languageCode];
     }
     this.settingsPrefs_!.set(
-        'prefs.translate_allowlists.value', alwaysTranslateList);
+        'prefs.translate_allowlists.value', alwaysTranslateMap);
   }
 
   /**
@@ -465,19 +464,19 @@ export function getFakeLanguagePrefs() {
     },
     {
       key: 'translate_site_blocklist_with_time',
-      type: chrome.settingsPrivate.PrefType.LIST,
+      type: chrome.settingsPrivate.PrefType.DICTIONARY,
       value: {
         'ru.wikipedia.org': '13305315102292953',
         'de.wikipedia.org': '13305315083099649',
       },
     },
     // Note: The real implementation of this pref is actually a dictionary
     // of {always translate: target}, however only the keys are needed for
-    // testing.
+    // testing so target will always be 'en'.
     {
       key: 'translate_allowlists',
-      type: chrome.settingsPrivate.PrefType.LIST,
-      value: [],
+      type: chrome.settingsPrivate.PrefType.DICTIONARY,
+      value: {},
     },
     {
       key: 'translate_recent_target',

chrome/test/data/webui/settings/translate_page_tests.ts

@@ -104,6 +104,78 @@ suite('translate page settings', function() {
       assertEquals(translatePage.getPref(translateTarget).value, 'sw');
     });
 
+    test('test never translate display', function() {
+      // Disable a language not in fake_language_settings_private. The language
+      // should not be shown in the never translate list.
+      languageHelper.disableTranslateLanguage('eo');
+      flush();
+
+      const neverTranslateDiv =
+          translatePage.shadowRoot!.querySelector<HTMLElement>(
+              '#neverTranslateList');
+      assertTrue(!!neverTranslateDiv);
+
+      // Only one language should be shown in the UI.
+      let listItems =
+          neverTranslateDiv.querySelectorAll<HTMLElement>('.list-item');
+      assertEquals(1, listItems.length);
+
+      // But two should be in the preference (since en-US is the default).
+      assertDeepEquals(
+          ['en-US', 'eo'], translatePage.getPref(neverTranslatePref).value);
+
+      // Disable a language that is in fake_language_settings_private. The
+      // language should be shown in the never translate list.
+      languageHelper.disableTranslateLanguage('nb');
+      flush();
+
+      // Two items should now be shown.
+      listItems = neverTranslateDiv.querySelectorAll<HTMLElement>('.list-item');
+      assertEquals(2, listItems.length);
+
+      // But three should be on the never translate list
+      assertDeepEquals(
+          ['en-US', 'eo', 'nb'],
+          translatePage.getPref(neverTranslatePref).value);
+    });
+
+    test('test always translate display', function() {
+      // Add a language not in fake_language_settings_private. The language
+      // should not be shown in the always translate list.
+      languageHelper.setLanguageAlwaysTranslateState('eo', true);
+      flush();
+
+      const alwaysTranslateDiv =
+          translatePage.shadowRoot!.querySelector<HTMLElement>(
+              '#alwaysTranslateList');
+      assertTrue(!!alwaysTranslateDiv);
+
+      // No languages should be shown on the UI.
+      let listItems =
+          alwaysTranslateDiv.querySelectorAll<HTMLElement>('.list-item');
+      assertEquals(0, listItems.length);
+
+      // But one should be on the always translate list
+      assertDeepEquals(
+          ['eo'],
+          Object.keys(translatePage.getPref(alwaysTranslatePref).value));
+
+      // Add a language that is in fake_language_settings_private. The
+      // language should be shown in the always translate list.
+      languageHelper.setLanguageAlwaysTranslateState('nb', true);
+      flush();
+
+      // // There should now be only one item shown.
+      listItems =
+          alwaysTranslateDiv.querySelectorAll<HTMLElement>('.list-item');
+      assertEquals(1, listItems.length);
+
+      // But two should be on the always translate list
+      assertDeepEquals(
+          ['eo', 'nb'],
+          Object.keys(translatePage.getPref(alwaysTranslatePref).value));
+    });
+
     test('never translate remove icon enabled state', function() {
       // The icon should be disabled if there is only one element on the list
       // and enabled if there are more than one.
@@ -222,7 +294,8 @@ suite('translate page settings', function() {
           new CustomEvent('languages-added', {detail: ['en', 'no']}));
       dialog.$.dialog.close();
       assertDeepEquals(
-          ['en', 'no'], translatePage.getPref(alwaysTranslatePref).value);
+          ['en', 'no'],
+          Object.keys(translatePage.getPref(alwaysTranslatePref).value));
 
       return dialogClosedResolver.promise;
     });