Skip to content

Commit

Permalink
Bug 1740067 - Rename language switching variables to be more explicit…
Browse files Browse the repository at this point in the history 
…; r=mstriemer,platform-i18n-reviewers,preferences-reviewers,nordzilla

These were all areas that were confusing me when I was onboarding on to
this code, so I tried to make the terminology less ambiguous and more
precise.

The default language is now the primary language.

UI is now appended to words that are dealing with DOM elements rather
than data stores.

Differential Revision: https://phabricator.services.mozilla.com/D136019
  • Loading branch information
@gregtatum
gregtatum committed Jan 20, 2022
1 parent 5146e0f commit 75db126
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 49 deletions.
56 changes: 30 additions & 26 deletions browser/components/preferences/dialogs/browserLanguages.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -360,10 +360,10 @@ function compareItems(a, b) {
}

var gBrowserLanguagesDialog = {
telemetryId: null,
_telemetryId: null,
accepted: false,
_availableLocales: null,
_selectedLocales: null,
_availableLocalesUI: null,
_selectedLocalesUI: null,
selectedLocales: null,

get downloadEnabled() {
Expand All @@ -376,7 +376,7 @@ var gBrowserLanguagesDialog = {
"intl.ui.browserLanguage",
method,
"dialog",
this.telemetryId,
this._telemetryId,
extra
);
},
Expand All @@ -392,7 +392,7 @@ var gBrowserLanguagesDialog = {
.addEventListener("beforeaccept", () => this.beforeAccept());
// Maintain the previously selected locales even if we cancel out.
let { telemetryId, selected, search } = window.arguments[0];
this.telemetryId = telemetryId;
this._telemetryId = telemetryId;
this.selectedLocales = selected;

// This is a list of available locales that the user selected. It's more
Expand Down Expand Up @@ -420,19 +420,21 @@ var gBrowserLanguagesDialog = {
},

async initSelectedLocales(selectedLocales) {
this._selectedLocales = new OrderedListBox({
this._selectedLocalesUI = 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(await getLocaleDisplayInfo(selectedLocales));
this._selectedLocalesUI.setItems(
await getLocaleDisplayInfo(selectedLocales)
);
},

async initAvailableLocales(available, search) {
this._availableLocales = new SortedItemSelectList({
this._availableLocalesUI = new SortedItemSelectList({
menulist: document.getElementById("availableLocales"),
button: document.getElementById("add"),
compareFn: compareItems,
Expand Down Expand Up @@ -467,7 +469,9 @@ var gBrowserLanguagesDialog = {
}

// Disable the dropdown while we hit the network.
this._availableLocales.disableWithMessageId("browser-languages-searching");
this._availableLocalesUI.disableWithMessageId(
"browser-languages-searching"
);

// Fetch the available langpacks from AMO.
let availableLangpacks;
Expand Down Expand Up @@ -502,13 +506,13 @@ var gBrowserLanguagesDialog = {
});

// Remove the search option and add the remote locales.
let items = this._availableLocales.items;
let items = this._availableLocalesUI.items;
items.pop();
items = items.concat(availableItems);

// Update the dropdown and enable it again.
this._availableLocales.setItems(items);
this._availableLocales.enableWithMessageId(
this._availableLocalesUI.setItems(items);
this._availableLocalesUI.enableWithMessageId(
"browser-languages-select-language"
);
},
Expand All @@ -527,7 +531,7 @@ var gBrowserLanguagesDialog = {
value: "search",
});
}
this._availableLocales.setItems(items);
this._availableLocalesUI.setItems(items);
},

async availableLanguageSelected(item) {
Expand All @@ -543,22 +547,22 @@ var gBrowserLanguagesDialog = {
},

async requestLocalLanguage(item, available) {
this._selectedLocales.addItem(item);
let selectedCount = this._selectedLocales.items.length;
this._selectedLocalesUI.addItem(item);
let selectedCount = this._selectedLocalesUI.items.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);
this._availableLocalesUI.items.shift();
this._availableLocalesUI.setItems(this._availableLocalesUI.items);
}
// The label isn't always reset when the selected item is removed, so set it again.
this._availableLocales.enableWithMessageId(
this._availableLocalesUI.enableWithMessageId(
"browser-languages-select-language"
);
},

async requestRemoteLanguage(item) {
this._availableLocales.disableWithMessageId(
this._availableLocalesUI.disableWithMessageId(
"browser-languages-downloading"
);

Expand All @@ -580,8 +584,8 @@ var gBrowserLanguagesDialog = {
}

item.installed = true;
this._selectedLocales.addItem(item);
this._availableLocales.enableWithMessageId(
this._selectedLocalesUI.addItem(item);
this._availableLocalesUI.enableWithMessageId(
"browser-languages-select-language"
);

Expand All @@ -605,7 +609,7 @@ var gBrowserLanguagesDialog = {

showError() {
document.getElementById("warning-message").hidden = false;
this._availableLocales.enableWithMessageId(
this._availableLocalesUI.enableWithMessageId(
"browser-languages-select-language"
);

Expand All @@ -624,17 +628,17 @@ var gBrowserLanguagesDialog = {
},

getSelectedLocales() {
return this._selectedLocales.items.map(item => item.value);
return this._selectedLocalesUI.items.map(item => item.value);
},

async selectedLocaleRemoved(item) {
this.recordTelemetry("remove");

this._availableLocales.addItem(item);
this._availableLocalesUI.addItem(item);

// If the item we added is at the top of the list, it needs the label.
if (this._availableLocales.items[0] == item) {
this._availableLocales.addItem(await this.createInstalledLabel());
if (this._availableLocalesUI.items[0] == item) {
this._availableLocalesUI.addItem(await this.createInstalledLabel());
}
},

Expand Down
2 changes: 1 addition & 1 deletion browser/components/preferences/main.inc.xhtml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@
<vbox id="browserLanguagesBox" align="start" hidden="true">
<description flex="1" controls="chooseBrowserLanguage" data-l10n-id="choose-browser-language-description"/>
<hbox>
<menulist id="defaultBrowserLanguage">
<menulist id="primaryBrowserLocale">
<menupopup/>
</menulist>
<button id="manageBrowserLanguagesButton"
Expand Down
36 changes: 20 additions & 16 deletions browser/components/preferences/main.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ var gMainPane = {
initializeProxyUI(gMainPane);

if (Services.prefs.getBoolPref("intl.multilingual.enabled")) {
gMainPane.initBrowserLocale();
gMainPane.initPrimaryBrowserLanguageUI();
}

// We call `initDefaultZoomValues` to set and unhide the
Expand Down Expand Up @@ -396,7 +396,7 @@ var gMainPane = {
Services.prefs.clearUserPref("browser.ctrlTab.migrated");
});
setEventListener("manageBrowserLanguagesButton", "command", function() {
gMainPane.showBrowserLanguages({ search: false });
gMainPane.showBrowserLanguagesSubDialog({ search: false });
});
if (AppConstants.MOZ_UPDATER) {
// These elements are only compiled in when the updater is enabled
Expand Down Expand Up @@ -986,28 +986,28 @@ var gMainPane = {
document.getElementById("zoomBox").hidden = false;
},

initBrowserLocale() {
initPrimaryBrowserLanguageUI() {
// Enable telemetry.
Services.telemetry.setEventRecordingEnabled(
"intl.ui.browserLanguage",
true
);

// This will register the "command" listener.
let menulist = document.getElementById("defaultBrowserLanguage");
let menulist = document.getElementById("primaryBrowserLocale");
new SelectionChangedMenulist(menulist, event => {
gMainPane.onBrowserLanguageChange(event);
gMainPane.onPrimaryBrowserLanguageMenuChange(event);
});

gMainPane.setBrowserLocales(Services.locale.appLocaleAsBCP47);
gMainPane.updatePrimaryBrowserLanguageUI(Services.locale.appLocaleAsBCP47);
},

/**
* 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) {
async updatePrimaryBrowserLanguageUI(selected) {
let available = await getAvailableLocales();
let localeNames = Services.intl.getLocaleDisplayNames(
undefined,
Expand All @@ -1028,7 +1028,7 @@ var gMainPane = {
// Add an option to search for more languages if downloading is supported.
if (Services.prefs.getBoolPref("intl.multilingual.downloadEnabled")) {
let menuitem = document.createXULElement("menuitem");
menuitem.id = "defaultBrowserLanguageSearch";
menuitem.id = "primaryBrowserLocaleSearch";
menuitem.setAttribute(
"label",
await document.l10n.formatValue("browser-languages-search")
Expand All @@ -1037,7 +1037,7 @@ var gMainPane = {
fragment.appendChild(menuitem);
}

let menulist = document.getElementById("defaultBrowserLanguage");
let menulist = document.getElementById("primaryBrowserLocale");
let menupopup = menulist.querySelector("menupopup");
menupopup.textContent = "";
menupopup.appendChild(fragment);
Expand Down Expand Up @@ -1114,7 +1114,7 @@ var gMainPane = {
}

messageBar.hidden = false;
gMainPane.selectedLocales = locales;
gMainPane.selectedLocalesForRestart = locales;
},

hideConfirmLanguageChangeMessageBar() {
Expand Down Expand Up @@ -1156,11 +1156,11 @@ var gMainPane = {
},

/* Show or hide the confirm change message bar based on the new locale. */
onBrowserLanguageChange(event) {
onPrimaryBrowserLanguageMenuChange(event) {
let locale = event.target.value;

if (locale == "search") {
gMainPane.showBrowserLanguages({ search: true });
gMainPane.showBrowserLanguagesSubDialog({ search: true });
return;
} else if (locale == Services.locale.appLocaleAsBCP47) {
this.hideConfirmLanguageChangeMessageBar();
Expand Down Expand Up @@ -1341,7 +1341,7 @@ var gMainPane = {
);
},

showBrowserLanguages({ search }) {
showBrowserLanguagesSubDialog({ search }) {
// Record the telemetry event with an id to associate related actions.
let telemetryId = parseInt(
Services.telemetry.msSinceProcessStart(),
Expand All @@ -1350,7 +1350,11 @@ var gMainPane = {
let method = search ? "search" : "manage";
gMainPane.recordBrowserLanguagesTelemetry(method, telemetryId);

let opts = { selected: gMainPane.selectedLocales, search, telemetryId };
let opts = {
selectedLocalesForRestart: gMainPane.selectedLocalesForRestart,
search,
telemetryId,
};
gSubDialog.open(
"chrome://browser/content/preferences/dialogs/browserLanguages.xhtml",
{ closingCallback: this.browserLanguagesClosed },
Expand All @@ -1370,12 +1374,12 @@ var gMainPane = {
// Prepare for changing the locales if they are different than the current locales.
if (selected && selected.join(",") != active.join(",")) {
gMainPane.showConfirmLanguageChangeMessageBar(selected);
gMainPane.setBrowserLocales(selected[0]);
gMainPane.updatePrimaryBrowserLanguageUI(selected[0]);
return;
}

// They matched, so we can reset the UI.
gMainPane.setBrowserLocales(Services.locale.appLocaleAsBCP47);
gMainPane.updatePrimaryBrowserLanguageUI(Services.locale.appLocaleAsBCP47);
gMainPane.hideConfirmLanguageChangeMessageBar();
},

Expand Down
10 changes: 5 additions & 5 deletions browser/components/preferences/tests/browser_browser_languages_subdialog.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -223,8 +223,8 @@ async function selectLocale(localeCode, available, selected, dialogDoc) {
async function openDialog(doc, search = false) {
let dialogLoaded = promiseLoadSubDialog(BROWSER_LANGUAGES_URL);
if (search) {
doc.getElementById("defaultBrowserLanguageSearch").doCommand();
doc.getElementById("defaultBrowserLanguage").menupopup.hidePopup();
doc.getElementById("primaryBrowserLocaleSearch").doCommand();
doc.getElementById("primaryBrowserLocale").menupopup.hidePopup();
} else {
doc.getElementById("manageBrowserLanguagesButton").doCommand();
}
Expand Down Expand Up @@ -703,7 +703,7 @@ add_task(async function testDownloadEnabled() {
});
let doc = gBrowser.contentDocument;

let defaultMenulist = doc.getElementById("defaultBrowserLanguage");
let defaultMenulist = doc.getElementById("primaryBrowserLocale");
ok(
hasSearchOption(defaultMenulist.menupopup),
"There's a search option in the General pane"
Expand Down Expand Up @@ -731,7 +731,7 @@ add_task(async function testDownloadDisabled() {
});
let doc = gBrowser.contentDocument;

let defaultMenulist = doc.getElementById("defaultBrowserLanguage");
let defaultMenulist = doc.getElementById("primaryBrowserLocale");
ok(
!hasSearchOption(defaultMenulist.menupopup),
"There's no search option in the General pane"
Expand Down Expand Up @@ -775,7 +775,7 @@ add_task(async function testReorderMainPane() {
let messageBar = doc.getElementById("confirmBrowserLanguage");
is(messageBar.hidden, true, "The message bar is hidden at first");

let available = doc.getElementById("defaultBrowserLanguage");
let available = doc.getElementById("primaryBrowserLocale");
let availableLocales = Array.from(available.menupopup.children);
let availableCodes = availableLocales
.map(item => item.value)
Expand Down
2 changes: 1 addition & 1 deletion browser/themes/shared/preferences/preferences.inc.css
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1072,7 +1072,7 @@ menulist[indicator=true] > menupopup menuitem[indicator=true]:not([image]) > .me
margin-inline-start: 4px;
}

#defaultBrowserLanguage {
#primaryBrowserLocale {
margin-inline-start: 0;
min-width: 20em;
}
Expand Down

0 comments on commit 75db126

Please sign in to comment.