Bug 1506102 - Don't count lastFallbackLocale as installed when just fluent is included r=zbraniecki,Gijs a=lizzard
authorMark Striemer <mstriemer@mozilla.com>
Tue, 12 Feb 2019 23:28:27 +0000
changeset 515945 720c87f798e3
parent 515944 99fa26897e41
child 515946 35e4f7994c01
push id1953
push userffxbld-merge
push dateMon, 11 Mar 2019 12:10:20 +0000
treeherdermozilla-release@9c35dcbaa899 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerszbraniecki, Gijs, lizzard
bugs1506102
milestone66.0
Bug 1506102 - Don't count lastFallbackLocale as installed when just fluent is included r=zbraniecki,Gijs a=lizzard Differential Revision: https://phabricator.services.mozilla.com/D18386
browser/components/preferences/browserLanguages.js
browser/components/preferences/in-content/main.js
browser/components/preferences/in-content/preferences.js
browser/components/preferences/in-content/tests/browser_browser_languages_subdialog.js
--- a/browser/components/preferences/browserLanguages.js
+++ b/browser/components/preferences/browserLanguages.js
@@ -1,16 +1,19 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 /* import-globals-from ../../../toolkit/content/preferencesBindings.js */
 
 ChromeUtils.import("resource://gre/modules/Services.jsm");
 
+// This is exported by preferences.js but we can't import that in a subdialog.
+let {getAvailableLocales} = window.top;
+
 ChromeUtils.defineModuleGetter(this, "AddonManager",
                                "resource://gre/modules/AddonManager.jsm");
 ChromeUtils.defineModuleGetter(this, "AddonRepository",
                                "resource://gre/modules/addons/AddonRepository.jsm");
 ChromeUtils.defineModuleGetter(this, "RemoteSettings",
                                "resource://services-settings/remote-settings.js");
 ChromeUtils.defineModuleGetter(this, "SelectionChangedMenulist",
                                "resource:///modules/SelectionChangedMenulist.jsm");
@@ -273,18 +276,18 @@ class SortedItemSelectList {
   enableWithMessageId(messageId) {
     this.menulist.setAttribute("data-l10n-id", messageId);
     this.menulist.removeAttribute("image");
     this.menulist.disabled = this.menulist.itemCount == 0;
     this.button.disabled = !this.menulist.selectedItem;
   }
 }
 
-function getLocaleDisplayInfo(localeCodes) {
-  let availableLocales = new Set(Services.locale.availableLocales);
+async function getLocaleDisplayInfo(localeCodes) {
+  let availableLocales = new Set(await getAvailableLocales());
   let packagedLocales = new Set(Services.locale.packagedLocales);
   let localeNames = Services.intl.getLocaleDisplayNames(undefined, localeCodes);
   return localeCodes.map((code, i) => {
     return {
       id: "locale-" + code,
       label: localeNames[i],
       value: code,
       canRemove: !packagedLocales.has(code),
@@ -345,41 +348,41 @@ var gBrowserLanguagesDialog = {
     this.selectedLocales = selected;
 
     // This is a list of available locales that the user selected. It's more
     // restricted than the Intl notion of `requested` as it only contains
     // locale codes for which we have matching locales available.
     // The first time this dialog is opened, populate with appLocalesAsBCP47.
     let selectedLocales = this.selectedLocales || Services.locale.appLocalesAsBCP47;
     let selectedLocaleSet = new Set(selectedLocales);
-    let available = Services.locale.availableLocales;
+    let available = await getAvailableLocales();
     let availableSet = new Set(available);
 
     // Filter selectedLocales since the user may select a locale when it is
     // available and then disable it.
     selectedLocales = selectedLocales.filter(locale => availableSet.has(locale));
     // Nothing in available should be in selectedSet.
     available = available.filter(locale => !selectedLocaleSet.has(locale));
 
-    this.initSelectedLocales(selectedLocales);
+    await this.initSelectedLocales(selectedLocales);
     await this.initAvailableLocales(available, search);
 
     this.initialized = true;
   },
 
-  initSelectedLocales(selectedLocales) {
+  async initSelectedLocales(selectedLocales) {
     this._selectedLocales = new OrderedListBox({
       richlistbox: document.getElementById("selectedLocales"),
       upButton: document.getElementById("up"),
       downButton: document.getElementById("down"),
       removeButton: document.getElementById("remove"),
       onRemove: (item) => this.selectedLocaleRemoved(item),
       onReorder: () => this.recordTelemetry("reorder"),
     });
-    this._selectedLocales.setItems(getLocaleDisplayInfo(selectedLocales));
+    this._selectedLocales.setItems(await getLocaleDisplayInfo(selectedLocales));
   },
 
   async initAvailableLocales(available, search) {
     this._availableLocales = new SortedItemSelectList({
       menulist: document.getElementById("availableLocales"),
       button: document.getElementById("add"),
       compareFn: compareItems,
       onSelect: (item) => this.availableLanguageSelected(item),
@@ -426,23 +429,23 @@ var gBrowserLanguagesDialog = {
 
     // Store the available langpack info for later use.
     this.availableLangpacks = new Map();
     for (let {target_locale, url, hash} of availableLangpacks) {
       this.availableLangpacks.set(target_locale, {url, hash});
     }
 
     // Remove the installed locales from the available ones.
-    let installedLocales = new Set(Services.locale.availableLocales);
+    let installedLocales = new Set(await getAvailableLocales());
     let notInstalledLocales = availableLangpacks
       .filter(({target_locale}) => !installedLocales.has(target_locale))
       .map(lang => lang.target_locale);
 
     // Create the rows for the remote locales.
-    let availableItems = getLocaleDisplayInfo(notInstalledLocales);
+    let availableItems = await getLocaleDisplayInfo(notInstalledLocales);
     availableItems.push({
       label: await document.l10n.formatValue("browser-languages-available-label"),
       className: "label-item",
       disabled: true,
       installed: false,
     });
 
     // Remove the search option and add the remote locales.
@@ -453,46 +456,46 @@ var gBrowserLanguagesDialog = {
     // Update the dropdown and enable it again.
     this._availableLocales.setItems(items);
     this._availableLocales.enableWithMessageId("browser-languages-select-language");
   },
 
   async loadLocalesFromInstalled(available) {
     let items;
     if (available.length > 0) {
-      items = getLocaleDisplayInfo(available);
+      items = await getLocaleDisplayInfo(available);
       items.push(await this.createInstalledLabel());
     } else {
       items = [];
     }
     if (this.downloadEnabled) {
       items.push({
         label: await document.l10n.formatValue("browser-languages-search"),
         value: "search",
       });
     }
     this._availableLocales.setItems(items);
   },
 
   async availableLanguageSelected(item) {
-    if (Services.locale.availableLocales.includes(item.value)) {
+    if ((await getAvailableLocales()).includes(item.value)) {
       this.recordTelemetry("add");
-      this.requestLocalLanguage(item);
+      await this.requestLocalLanguage(item);
     } else if (this.availableLangpacks.has(item.value)) {
       // Telemetry is tracked in requestRemoteLanguage.
       await this.requestRemoteLanguage(item);
     } else {
       this.showError();
     }
   },
 
-  requestLocalLanguage(item, available) {
+  async requestLocalLanguage(item, available) {
     this._selectedLocales.addItem(item);
     let selectedCount = this._selectedLocales.items.length;
-    let availableCount = Services.locale.availableLocales.length;
+    let availableCount = (await getAvailableLocales()).length;
     if (selectedCount == availableCount) {
       // Remove the installed label, they're all installed.
       this._availableLocales.items.shift();
       this._availableLocales.setItems(this._availableLocales.items);
     }
     // The label isn't always reset when the selected item is removed, so set it again.
     this._availableLocales.enableWithMessageId("browser-languages-select-language");
   },
--- a/browser/components/preferences/in-content/main.js
+++ b/browser/components/preferences/in-content/main.js
@@ -778,17 +778,17 @@ var gMainPane = {
   },
 
   /**
    * Update the available list of locales and select the locale that the user
    * is "selecting". This could be the currently requested locale or a locale
    * that the user would like to switch to after confirmation.
    */
   async setBrowserLocales(selected) {
-    let available = Services.locale.availableLocales;
+    let available = await getAvailableLocales();
     let localeNames = Services.intl.getLocaleDisplayNames(undefined, available);
     let locales = available.map((code, i) => ({code, name: localeNames[i]}));
     locales.sort((a, b) => a.name > b.name);
 
     let fragment = document.createDocumentFragment();
     for (let {code, name} of locales) {
       let menuitem = document.createXULElement("menuitem");
       menuitem.setAttribute("value", code);
--- a/browser/components/preferences/in-content/preferences.js
+++ b/browser/components/preferences/in-content/preferences.js
@@ -421,8 +421,30 @@ function appendSearchKeywords(aId, keywo
   element.setAttribute("searchkeywords", keywords.join(" "));
 }
 
 function maybeDisplayPoliciesNotice() {
   if (Services.policies.status == Services.policies.ACTIVE) {
     document.getElementById("policies-container").removeAttribute("hidden");
   }
 }
+
+/**
+ * Filter the lastFallbackLocale from availableLocales if it doesn't have all
+ * of the needed strings.
+ *
+ * When the lastFallbackLocale isn't the defaultLocale, then by default only
+ * fluent strings are included. To fully use that locale you need the langpack
+ * to be installed, so if it isn't installed remove it from availableLocales.
+ */
+async function getAvailableLocales() {
+  let {availableLocales, defaultLocale, lastFallbackLocale} = Services.locale;
+  // If defaultLocale isn't lastFallbackLocale, then we still need the langpack
+  // for lastFallbackLocale for it to be useful.
+  if (defaultLocale != lastFallbackLocale) {
+    let lastFallbackId = `langpack-${lastFallbackLocale}@firefox.mozilla.org`;
+    let lastFallbackInstalled = await AddonManager.getAddonByID(lastFallbackId);
+    if (!lastFallbackInstalled) {
+      return availableLocales.filter(locale => locale != lastFallbackLocale);
+    }
+  }
+  return availableLocales;
+}
--- a/browser/components/preferences/in-content/tests/browser_browser_languages_subdialog.js
+++ b/browser/components/preferences/in-content/tests/browser_browser_languages_subdialog.js
@@ -165,21 +165,32 @@ function assertTelemetryRecorded(events)
   let relatedEvents = snapshot.parent
     .filter(([timestamp, category]) => category == TELEMETRY_CATEGORY)
     .map(relatedEvent => relatedEvent.slice(2, 6));
 
   // Events are now an array of: method, object[, value[, extra]] as expected.
   Assert.deepEqual(relatedEvents, events, "The events are recorded correctly");
 }
 
-function selectLocale(localeCode, available, dialogDoc) {
+async function selectLocale(localeCode, available, selected, dialogDoc) {
   let [locale] = Array.from(available.firstElementChild.children)
     .filter(item => item.value == localeCode);
   available.selectedItem = locale;
+
+  // Get ready for the selected list to change.
+  let added = waitForMutation(
+    selected,
+    {childList: true},
+    target => Array.from(target.children).some(el => el.value == localeCode));
+
+  // Add the locale.
   dialogDoc.getElementById("add").doCommand();
+
+  // Wait for the list to update.
+  await added;
 }
 
 async function openDialog(doc, search = false) {
   let dialogLoaded = promiseLoadSubDialog(BROWSER_LANGUAGES_URL);
   if (search) {
     doc.getElementById("defaultBrowserLanguageSearch").doCommand();
     doc.getElementById("defaultBrowserLanguage").firstElementChild.hidePopup();
   } else {
@@ -250,39 +261,32 @@ add_task(async function testDisabledBrow
     target =>
       Array.from(available.firstElementChild.children)
         .some(locale => locale.value == "pl"));
 
   // pl is now available since it is available remotely.
   assertAvailableLocales(available, ["fr", "pl"]);
 
   // Add pl.
-  selectLocale("pl", available, dialogDoc);
-
-  // Wait for pl to be added, this should upgrade and enable the existing langpack.
-  await waitForMutation(
-    selected,
-    {childList: true},
-    target => selected.itemCount == 3);
+  await selectLocale("pl", available, selected, dialogDoc);
   assertLocaleOrder(selected, "pl,en-US,he");
 
   // Find pl again since it's been upgraded.
   pl = await AddonManager.getAddonByID(langpackId("pl"));
   is(pl.userDisabled, false, "pl is now enabled");
   is(pl.version, "2.0", "pl is upgraded to version 2.0");
 
   let dialogId = getDialogId(dialogDoc);
   ok(dialogId, "There's a dialogId");
   let {installId} = pl.install;
   ok(installId, "There's an installId");
 
   await Promise.all(addons.map(addon => addon.uninstall()));
   BrowserTestUtils.removeTab(gBrowser.selectedTab);
 
-  // FIXME: Also here
   assertTelemetryRecorded([
     ["manage", "main", dialogId],
     ["search", "dialog", dialogId],
     ["add", "dialog", dialogId, {installId}],
 
     // Cancel is recorded when the tab is closed.
     ["cancel", "dialog", dialogId],
   ]);
@@ -397,30 +401,30 @@ add_task(async function testAddAndRemove
   let {dialog, dialogDoc, available, selected} = await openDialog(doc);
   let dialogId = getDialogId(dialogDoc);
 
   // The initial order is set by the pref.
   assertLocaleOrder(selected, "en-US");
   assertAvailableLocales(available, ["fr", "pl", "he"]);
 
   // Add pl and fr to selected.
-  selectLocale("pl", available, dialogDoc);
-  selectLocale("fr", available, dialogDoc);
+  await selectLocale("pl", available, selected, dialogDoc);
+  await selectLocale("fr", available, selected, dialogDoc);
 
   assertLocaleOrder(selected, "fr,pl,en-US");
   assertAvailableLocales(available, ["he"]);
 
   // Remove pl and fr from selected.
   dialogDoc.getElementById("remove").doCommand();
   dialogDoc.getElementById("remove").doCommand();
   assertLocaleOrder(selected, "en-US");
   assertAvailableLocales(available, ["fr", "pl", "he"]);
 
   // Add he to selected.
-  selectLocale("he", available, dialogDoc);
+  await selectLocale("he", available, selected, dialogDoc);
   assertLocaleOrder(selected, "he,en-US");
   assertAvailableLocales(available, ["pl", "fr"]);
 
   // Accepting the change shows the confirm message bar.
   let dialogClosed = BrowserTestUtils.waitForEvent(dialogDoc.documentElement, "dialogclosing");
   dialog.acceptDialog();
   await dialogClosed;
 
@@ -497,24 +501,17 @@ add_task(async function testInstallFromA
   is(Services.locale.availableLocales.join(","),
      "en-US", "There is only one installed locale");
 
   // Verify that there are no extra dictionaries.
   let dicts = await AddonManager.getAddonsByTypes(["dictionary"]);
   is(dicts.length, 0, "There are no installed dictionaries");
 
   // Add Polish, this will install the langpack.
-  selectLocale("pl", available, dialogDoc);
-
-  // Wait for the langpack to install and be added to the list.
-  let selectedLocales = dialogDoc.getElementById("selectedLocales");
-  await waitForMutation(
-    selectedLocales,
-    {childList: true},
-    target => selectedLocales.itemCount == 2);
+  await selectLocale("pl", available, selected, dialogDoc);
 
   let langpack = await AddonManager.getAddonByID(langpackId("pl"));
   Assert.deepEqual(
     langpack.installTelemetryInfo,
     {source: "about:preferences"},
     "The source is set to preferences");
 
   // Verify the list is correct.
@@ -564,17 +561,16 @@ add_task(async function testInstallFromA
   // Uninstall the langpack and dictionary.
   let installs = await AddonManager.getAddonsByTypes(["locale", "dictionary"]);
   is(installs.length, 2, "There is one langpack and one dictionary installed");
   await Promise.all(installs.map(item => item.uninstall()));
 
   BrowserTestUtils.removeTab(gBrowser.selectedTab);
 
   ok(installId, "The langpack has an installId");
-  // FIXME: Most are here
   assertTelemetryRecorded([
     // First dialog installs a locale and accepts.
     ["search", "main", firstDialogId],
     // It has an installId since it was downloaded.
     ["add", "dialog", firstDialogId, {installId}],
     // It got moved down to avoid errors with finding translations.
     ["reorder", "dialog", firstDialogId],
     ["accept", "dialog", firstDialogId],