Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Fall back to untranslated string rather than showing missing translation error #8609

Merged
merged 6 commits into from
May 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion src/languageHandler.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,25 @@ export function _td(s: string): string { // eslint-disable-line @typescript-esli
* */
const translateWithFallback = (text: string, options?: object): { translated?: string, isFallback?: boolean } => {
const translated = counterpart.translate(text, { ...options, fallbackLocale: counterpart.getLocale() });
if (!translated || /^missing translation:/.test(translated)) {
if (!translated || translated.startsWith("missing translation:")) {
const fallbackTranslated = counterpart.translate(text, { ...options, locale: FALLBACK_LOCALE });
if ((!fallbackTranslated || fallbackTranslated.startsWith("missing translation:"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to triple check intent, these brackets that were added actually changed the behaviour of the if - && has higher precedence than ||

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

noted, the PR has been a litany of careless errors unfortunately.

the intent is to never enter the if clause if we are in development mode (i.e. leave the behaviour as-is)

&& process.env.NODE_ENV !== "development") {
// Even the translation via FALLBACK_LOCALE failed; this can happen if
//
// 1. The string isn't in the translations dictionary, usually because you're in develop
// and haven't run yarn i18n
// 2. Loading the translation resources over the network failed, which can happen due to
// to network or if the client tried to load a translation that's been removed from the
// server.
//
// At this point, its the lesser evil to show the untranslated text, which
// will be in English, so the user can still make out *something*, rather than an opaque
// "missing translation" error.
//
// Don't do this in develop so people remember to run yarn i18n.
return { translated: text, isFallback: true };
}
return { translated: fallbackTranslated, isFallback: true };
}
return { translated };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ exports[`KeyboardShortcut doesn't render same modifier twice 1`] = `
>
<kbd>

missing translation: en|Ctrl
Ctrl

</kbd>
+
Expand Down Expand Up @@ -70,7 +70,7 @@ exports[`KeyboardShortcut doesn't render same modifier twice 2`] = `
>
<kbd>

missing translation: en|Ctrl
Ctrl

</kbd>
+
Expand All @@ -95,7 +95,7 @@ exports[`KeyboardShortcut renders alternative key name 1`] = `
>
<kbd>

missing translation: en|Page Down
Page Down

</kbd>
+
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ exports[`KeyboardUserSettingsTab renders list of keyboard shortcuts 1`] = `
<div
className="mx_SettingsTab_heading"
>
missing translation: en|Keyboard
Keyboard
</div>
<KeyboardShortcutSection
category={
Expand All @@ -30,7 +30,7 @@ exports[`KeyboardUserSettingsTab renders list of keyboard shortcuts 1`] = `
<div
className="mx_SettingsTab_subheading"
>
missing translation: en|Composer
Composer
</div>
<div>

Expand Down Expand Up @@ -59,7 +59,7 @@ exports[`KeyboardUserSettingsTab renders list of keyboard shortcuts 1`] = `
>
<kbd>

missing translation: en|Ctrl
Ctrl

</kbd>
+
Expand Down Expand Up @@ -103,7 +103,7 @@ exports[`KeyboardUserSettingsTab renders list of keyboard shortcuts 1`] = `
>
<kbd>

missing translation: en|Ctrl
Ctrl

</kbd>
+
Expand Down Expand Up @@ -145,7 +145,7 @@ exports[`KeyboardUserSettingsTab renders list of keyboard shortcuts 1`] = `
<div
className="mx_SettingsTab_subheading"
>
missing translation: en|Navigation
Navigation
</div>
<div>

Expand Down Expand Up @@ -173,7 +173,7 @@ exports[`KeyboardUserSettingsTab renders list of keyboard shortcuts 1`] = `
>
<kbd>

missing translation: en|Enter
Enter

</kbd>
</KeyboardKey>
Expand Down
18 changes: 18 additions & 0 deletions test/i18n-test/languageHandler-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ import {
import { stubClient } from '../test-utils';

describe('languageHandler', function() {
/*
See /__mocks__/browser-request.js/ for how we are stubbing out translations
to provide fixture data for these tests
*/
const basicString = 'Rooms';
const selfClosingTagSub = 'Accept <policyLink /> to continue:';
const textInTagSub = '<a>Upgrade</a> to your own domain';
Expand All @@ -35,6 +39,7 @@ describe('languageHandler', function() {

type TestCase = [string, string, Record<string, unknown>, Record<string, unknown>, TranslatedString];
const testCasesEn: TestCase[] = [
// description of the test case, translationString, variables, tags, expected result
['translates a basic string', basicString, {}, undefined, 'Rooms'],
[
'handles plurals when count is 0',
Expand Down Expand Up @@ -217,4 +222,17 @@ describe('languageHandler', function() {
});
});
});

describe('when languages dont load', () => {
it('_t', async () => {
const STRING_NOT_IN_THE_DICTIONARY = "a string that isn't in the translations dictionary";
expect(_t(STRING_NOT_IN_THE_DICTIONARY, {}, undefined)).toEqual(STRING_NOT_IN_THE_DICTIONARY);
});

it('_tDom', async () => {
const STRING_NOT_IN_THE_DICTIONARY = "a string that isn't in the translations dictionary";
expect(_tDom(STRING_NOT_IN_THE_DICTIONARY, {}, undefined)).toEqual(
<span lang="en">{ STRING_NOT_IN_THE_DICTIONARY }</span>);
});
});
});