Skip to content

Commit

Permalink
Bug 1844678 - Remove beta language labels from Translations r=gregtat…
Browse files Browse the repository at this point in the history
…um,fluent-reviewers,flod

Removes the labeling of individual languages as being
in beta or not, from the UI components of translations.

Differential Revision: https://phabricator.services.mozilla.com/D184177
  • Loading branch information
nordzilla committed Jul 24, 2023
1 parent 04110d0 commit 68a39b4
Show file tree
Hide file tree
Showing 18 changed files with 82 additions and 189 deletions.
24 changes: 4 additions & 20 deletions browser/components/translations/content/translationsPanel.js
Original file line number Diff line number Diff line change
Expand Up @@ -439,35 +439,19 @@ var TranslationsPanel = new (class {
);

for (const popup of fromPopups) {
for (const { langTag, isBeta, displayName } of fromLanguages) {
for (const { langTag, displayName } of fromLanguages) {
const fromMenuItem = document.createXULElement("menuitem");
fromMenuItem.setAttribute("value", langTag);
if (isBeta) {
document.l10n.setAttributes(
fromMenuItem,
"translations-panel-displayname-beta",
{ language: displayName }
);
} else {
fromMenuItem.setAttribute("label", displayName);
}
fromMenuItem.setAttribute("label", displayName);
popup.appendChild(fromMenuItem);
}
}

for (const popup of toPopups) {
for (const { langTag, isBeta, displayName } of toLanguages) {
for (const { langTag, displayName } of toLanguages) {
const toMenuItem = document.createXULElement("menuitem");
toMenuItem.setAttribute("value", langTag);
if (isBeta) {
document.l10n.setAttributes(
toMenuItem,
"translations-panel-displayname-beta",
{ language: displayName }
);
} else {
toMenuItem.setAttribute("label", displayName);
}
toMenuItem.setAttribute("label", displayName);
popup.appendChild(toMenuItem);
}
}
Expand Down
1 change: 0 additions & 1 deletion browser/components/translations/tests/browser/browser.ini
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ support-files =
[browser_translations_panel_app_menu_never_translate_site.js]
[browser_translations_panel_auto_offer.js]
[browser_translations_panel_basics.js]
[browser_translations_panel_beta_langs.js]
[browser_translations_panel_button.js]
[browser_translations_panel_cancel.js]
[browser_translations_panel_engine_unsupported.js]
Expand Down
12 changes: 6 additions & 6 deletions browser/components/translations/tests/browser/head.js
Original file line number Diff line number Diff line change
Expand Up @@ -387,10 +387,10 @@ const SPANISH_PAGE_URL = TRANSLATIONS_TESTER_ES;
const SPANISH_PAGE_URL_2 = TRANSLATIONS_TESTER_ES_2;
const SPANISH_PAGE_URL_DOT_ORG = TRANSLATIONS_TESTER_ES_DOT_ORG;
const LANGUAGE_PAIRS = [
{ fromLang: "es", toLang: "en", isBeta: false },
{ fromLang: "en", toLang: "es", isBeta: false },
{ fromLang: "fr", toLang: "en", isBeta: false },
{ fromLang: "en", toLang: "fr", isBeta: false },
{ fromLang: "en", toLang: "uk", isBeta: true },
{ fromLang: "uk", toLang: "en", isBeta: true },
{ fromLang: "es", toLang: "en" },
{ fromLang: "en", toLang: "es" },
{ fromLang: "fr", toLang: "en" },
{ fromLang: "en", toLang: "fr" },
{ fromLang: "en", toLang: "uk" },
{ fromLang: "uk", toLang: "en" },
];
6 changes: 0 additions & 6 deletions browser/locales/en-US/browser/translations.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,6 @@ urlbar-translations-button-loading =
translations-panel-settings-button =
.aria-label = Manage translation settings
# Text displayed on a language dropdown when the language is in beta
# Variables:
# $language (string) - The localized display name of the detected language
translations-panel-displayname-beta =
.label = { $language } BETA
## Options in the Firefox Translations settings.

translations-panel-settings-manage-languages =
Expand Down
45 changes: 13 additions & 32 deletions toolkit/components/translations/actors/TranslationsParent.sys.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -910,11 +910,10 @@ export class TranslationsParent extends JSWindowActorParent {
TranslationsParent.#getTranslationModelRecords().then(records => {
const languagePairMap = new Map();

for (const { fromLang, toLang, version } of records.values()) {
const isBeta = Services.vc.compare(version, "1.0") < 0;
for (const { fromLang, toLang } of records.values()) {
const key = TranslationsParent.languagePairKey(fromLang, toLang);
if (!languagePairMap.has(key)) {
languagePairMap.set(key, { fromLang, toLang, isBeta });
languagePairMap.set(key, { fromLang, toLang });
}
}
return Array.from(languagePairMap.values());
Expand All @@ -936,31 +935,14 @@ export class TranslationsParent extends JSWindowActorParent {
static async getSupportedLanguages() {
const languagePairs = await TranslationsParent.getLanguagePairs();

/** @type {Map<string, boolean>} */
const fromLanguages = new Map();
/** @type {Map<string, boolean>} */
const toLanguages = new Map();

for (const { fromLang, toLang, isBeta } of languagePairs) {
// [BetaLanguage, BetaLanguage] => isBeta == true,
// [BetaLanguage, NonBetaLanguage] => isBeta == true,
// [NonBetaLanguage, BetaLanguage] => isBeta == true,
// [NonBetaLanguage, NonBetaLanguage] => isBeta == false,
if (isBeta) {
// If these languages are part of a beta languagePair, at least one of them is a beta language
// but the other may not be, so only tentatively mark them as beta if there is no entry.
if (!fromLanguages.has(fromLang)) {
fromLanguages.set(fromLang, isBeta);
}
if (!toLanguages.has(toLang)) {
toLanguages.set(toLang, isBeta);
}
} else {
// If these languages are part of a non-beta languagePair, then they are both
// guaranteed to be non-beta languages. Idempotently overwrite any previous entry.
fromLanguages.set(fromLang, isBeta);
toLanguages.set(toLang, isBeta);
}
/** @type {Set<string>} */
const fromLanguages = new Set();
/** @type {Set<string>} */
const toLanguages = new Set();

for (const { fromLang, toLang } of languagePairs) {
fromLanguages.add(fromLang);
toLanguages.add(toLang);
}

// Build a map of the langTag to the display name.
Expand All @@ -981,20 +963,19 @@ export class TranslationsParent extends JSWindowActorParent {
}
}

const addDisplayName = ([langTag, isBeta]) => ({
const addDisplayName = langTag => ({
langTag,
isBeta,
displayName: displayNames.get(langTag),
});

const sort = (a, b) => a.displayName.localeCompare(b.displayName);

return {
languagePairs,
fromLanguages: Array.from(fromLanguages.entries())
fromLanguages: Array.from(fromLanguages.keys())
.map(addDisplayName)
.sort(sort),
toLanguages: Array.from(toLanguages.entries())
toLanguages: Array.from(toLanguages.keys())
.map(addDisplayName)
.sort(sort),
};
Expand Down
42 changes: 8 additions & 34 deletions toolkit/components/translations/content/translations.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -286,9 +286,9 @@ class TranslationsState {
({ langTag: existingTag }) => existingTag === langTag
);
if (entry) {
const { displayName, isBeta } = entry;
const { displayName } = entry;
await this.setFromLanguage(langTag);
this.ui.setDetectOptionTextContent(displayName, isBeta);
this.ui.setDetectOptionTextContent(displayName);
}
}

Expand Down Expand Up @@ -390,41 +390,17 @@ class TranslationsUI {
const supportedLanguages = await this.state.supportedLanguages;

// Update the DOM elements with the display names.
for (const {
langTag,
isBeta,
displayName,
} of supportedLanguages.toLanguages) {
for (const { langTag, displayName } of supportedLanguages.toLanguages) {
const option = document.createElement("option");
option.value = langTag;
if (isBeta) {
document.l10n.setAttributes(
option,
"about-translations-displayname-beta",
{ language: displayName }
);
} else {
option.text = displayName;
}
option.text = displayName;
this.languageTo.add(option);
}

for (const {
langTag,
isBeta,
displayName,
} of supportedLanguages.fromLanguages) {
for (const { langTag, displayName } of supportedLanguages.fromLanguages) {
const option = document.createElement("option");
option.value = langTag;
if (isBeta) {
document.l10n.setAttributes(
option,
"about-translations-displayname-beta",
{ language: displayName }
);
} else {
option.text = displayName;
}
option.text = displayName;
this.languageFrom.add(option);
}

Expand Down Expand Up @@ -488,14 +464,12 @@ class TranslationsUI {
*
* @param {string} displayName
*/
setDetectOptionTextContent(displayName, isBeta = false) {
setDetectOptionTextContent(displayName) {
// Set the text to the fluent value that takes an arg to display the language name.
if (displayName) {
document.l10n.setAttributes(
this.#detectOption,
isBeta
? "about-translations-detect-lang-beta"
: "about-translations-detect-lang",
"about-translations-detect-lang",
{ language: displayName }
);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,13 +77,6 @@ the latest [remote-settings-devtools] Firefox extension.

Firefox Translations uses semantic versioning for all of its records via the **`version`** property.

```{note}
**Beta Versions**
Versions that are below **`1.0`** are considered to be a beta version. For language translation models,
this quality will be communicated to users via the UI.
```

#### Non-breaking Changes

Firefox Translations code will always retrieve the maximum compatible version of each record from Remote Settings.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
add_task(async function test_about_translations_debounce() {
await openAboutTranslations({
languagePairs: [
{ fromLang: "en", toLang: "fr", isBeta: false },
{ fromLang: "fr", toLang: "en", isBeta: false },
{ fromLang: "en", toLang: "fr" },
{ fromLang: "fr", toLang: "en" },
],
runInPage: async ({ selectors }) => {
const { document, window } = content;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ add_task(async function test_about_translations_language_directions() {
await openAboutTranslations({
languagePairs: [
// English (en) is LTR and Arabic (ar) is RTL.
{ fromLang: "en", toLang: "ar", isBeta: true },
{ fromLang: "ar", toLang: "en", isBeta: true },
{ fromLang: "en", toLang: "ar" },
{ fromLang: "ar", toLang: "en" },
],
runInPage: async ({ selectors }) => {
const { document, window } = content;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@

add_task(async function test_about_translations_dropdowns() {
let languagePairs = [
{ fromLang: "en", toLang: "es", isBeta: false },
{ fromLang: "es", toLang: "en", isBeta: false },
{ fromLang: "en", toLang: "es" },
{ fromLang: "es", toLang: "en" },
// This is not a bi-directional translation.
{ fromLang: "is", toLang: "en", isBeta: true },
{ fromLang: "is", toLang: "en" },
];
await openAboutTranslations({
languagePairs,
Expand Down Expand Up @@ -42,30 +42,6 @@ add_task(async function test_about_translations_dropdowns() {
selectedValue,
}) {
const options = [...select.options];
const betaL10nId = "about-translations-displayname-beta";
for (const option of options) {
for (const languagePair of languagePairs) {
if (
languagePair.fromLang === option.value ||
languagePair.toLang === option.value
) {
if (option.getAttribute("data-l10n-id") === betaL10nId) {
is(
languagePair.isBeta,
true,
`Since data-l10n-id was ${betaL10nId} for ${option.value}, then it must be part of a beta language pair, but it was not.`
);
}
if (!languagePair.isBeta) {
is(
option.getAttribute("data-l10n-id") === betaL10nId,
false,
`Since the languagePair is non-beta, the language option ${option.value} should not have a data-l10-id of ${betaL10nId}, but it does.`
);
}
}
}
}
info(message);
Assert.deepEqual(
options.filter(option => !option.hidden).map(option => option.value),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@
add_task(async function test_about_translations_translations() {
await openAboutTranslations({
languagePairs: [
{ fromLang: "en", toLang: "fr", isBeta: false },
{ fromLang: "fr", toLang: "en", isBeta: false },
{ fromLang: "en", toLang: "fr" },
{ fromLang: "fr", toLang: "en" },
// This is not a bi-directional translation.
{ fromLang: "is", toLang: "en", isBeta: true },
{ fromLang: "is", toLang: "en" },
],
runInPage: async ({ selectors }) => {
const { document, window } = content;
Expand Down Expand Up @@ -108,8 +108,8 @@ add_task(async function test_about_translations_translations() {
add_task(async function test_about_translations_html() {
await openAboutTranslations({
languagePairs: [
{ fromLang: "en", toLang: "fr", isBeta: false },
{ fromLang: "fr", toLang: "en", isBeta: false },
{ fromLang: "en", toLang: "fr" },
{ fromLang: "fr", toLang: "en" },
],
prefs: [["browser.translations.useHTML", true]],
runInPage: async ({ selectors }) => {
Expand Down Expand Up @@ -176,8 +176,8 @@ add_task(async function test_about_translations_language_identification() {
detectedLangTag: "en",
detectedLanguageConfidence: "0.98",
languagePairs: [
{ fromLang: "en", toLang: "fr", isBeta: false },
{ fromLang: "fr", toLang: "en", isBeta: false },
{ fromLang: "en", toLang: "fr" },
{ fromLang: "fr", toLang: "en" },
],
prefs: [["browser.translations.languageIdentification.useFastText", true]],
runInPage: async ({ selectors }) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,15 @@ add_task(async function test_pivot_language_behavior() {

const { cleanup } = await setupActorTest({
languagePairs: [
{ fromLang: "en", toLang: "es", isBeta: false },
{ fromLang: "es", toLang: "en", isBeta: false },
{ fromLang: "en", toLang: "yue", isBeta: true },
{ fromLang: "yue", toLang: "en", isBeta: true },
{ fromLang: "en", toLang: "es" },
{ fromLang: "es", toLang: "en" },
{ fromLang: "en", toLang: "yue" },
{ fromLang: "yue", toLang: "en" },
// This is not a bi-directional translation.
{ fromLang: "is", toLang: "en", isBeta: false },
{ fromLang: "is", toLang: "en" },
// These are non-pivot languages.
{ fromLang: "zh", toLang: "ja", isBeta: true },
{ fromLang: "ja", toLang: "zh", isBeta: true },
{ fromLang: "zh", toLang: "ja" },
{ fromLang: "ja", toLang: "zh" },
],
});

Expand All @@ -43,11 +43,11 @@ add_task(async function test_pivot_language_behavior() {
Assert.deepEqual(
languagePairs,
[
{ fromLang: "en", toLang: "es", isBeta: false },
{ fromLang: "en", toLang: "yue", isBeta: true },
{ fromLang: "es", toLang: "en", isBeta: false },
{ fromLang: "is", toLang: "en", isBeta: false },
{ fromLang: "yue", toLang: "en", isBeta: true },
{ fromLang: "en", toLang: "es" },
{ fromLang: "en", toLang: "yue" },
{ fromLang: "es", toLang: "en" },
{ fromLang: "is", toLang: "en" },
{ fromLang: "yue", toLang: "en" },
],
"Non-pivot languages were removed."
);
Expand Down
Loading

0 comments on commit 68a39b4

Please sign in to comment.