Attribute fallback i18n strings with lang attribute (#7323)

* add lang attribute to fallback translations

Signed-off-by: Kerry Archibald <kerrya@element.io>

* readability improvement

Signed-off-by: Kerry Archibald <kerrya@element.io>

* split _t and _tDom

Signed-off-by: Kerry <kerry@Kerrys-MBP.fritz.box>

* use tDom in HomePage

Signed-off-by: Kerry Archibald <kerrya@element.io>

* lint

Signed-off-by: Kerry Archibald <kerrya@element.io>

* bump matrix-web-i18n

Signed-off-by: Kerry Archibald <kerrya@element.io>

__mocks__/browser-request.js

@@ -1,10 +1,14 @@
 const en = require("../src/i18n/strings/en_EN");
 const de = require("../src/i18n/strings/de_DE");
+const lv = {
+    "Save": "Saglabāt",
+};
 
 // Mock the browser-request for the languageHandler tests to return
-// Fake languages.json containing references to en_EN and de_DE
+// Fake languages.json containing references to en_EN, de_DE and lv
 // en_EN.json
 // de_DE.json
+// lv.json - mock version with few translations, used to test fallback translation
 module.exports = jest.fn((opts, cb) => {
     const url = opts.url || opts.uri;
     if (url && url.endsWith("languages.json")) {
@@ -17,11 +21,17 @@ module.exports = jest.fn((opts, cb) => {
                 "fileName": "de_DE.json",
                 "label": "German",
             },
+            "lv": {
+                "fileName": "lv.json",
+                "label": "Latvian"
+            }
         }));
     } else if (url && url.endsWith("en_EN.json")) {
         cb(undefined, {status: 200}, JSON.stringify(en));
     } else if (url && url.endsWith("de_DE.json")) {
         cb(undefined, {status: 200}, JSON.stringify(de));
+    } else if (url && url.endsWith("lv.json")) {
+        cb(undefined, {status: 200}, JSON.stringify(lv));
     } else {
         cb(true, {status: 404}, "");
     }

src/components/structures/HomePage.tsx

@@ -19,7 +19,7 @@ import { useContext, useState } from "react";
 
 import AutoHideScrollbar from './AutoHideScrollbar';
 import { getHomePageUrl } from "../../utils/pages";
-import { _t } from "../../languageHandler";
+import { _tDom } from "../../languageHandler";
 import SdkConfig from "../../SdkConfig";
 import * as sdk from "../../index";
 import dis from "../../dispatcher/dispatcher";
@@ -72,8 +72,8 @@ const UserWelcomeTop = () => {
     return <div>
         <MiniAvatarUploader
             hasAvatar={!!ownProfile.avatarUrl}
-            hasAvatarLabel={_t("Great, that'll help people know it's you")}
+            hasAvatarLabel={_tDom("Great, that'll help people know it's you")}
-            noAvatarLabel={_t("Add a photo so people know it's you.")}
+            noAvatarLabel={_tDom("Add a photo so people know it's you.")}
             setAvatarUrl={url => cli.setAvatarUrl(url)}
         >
             <BaseAvatar
@@ -86,8 +86,8 @@ const UserWelcomeTop = () => {
             />
         </MiniAvatarUploader>
 
-        <h1>{ _t("Welcome %(name)s", { name: ownProfile.displayName }) }</h1>
+        <h1>{ _tDom("Welcome %(name)s", { name: ownProfile.displayName }) }</h1>
-        <h4>{ _t("Now, let's help you get started") }</h4>
+        <h4>{ _tDom("Now, let's help you get started") }</h4>
     </div>;
 };
 
@@ -113,8 +113,8 @@ const HomePage: React.FC<IProps> = ({ justRegistered = false }) => {
 
         introSection = <React.Fragment>
             <img src={logoUrl} alt={config.brand} />
-            <h1>{ _t("Welcome to %(appName)s", { appName: config.brand }) }</h1>
+            <h1>{ _tDom("Welcome to %(appName)s", { appName: config.brand }) }</h1>
-            <h4>{ _t("Own your conversations.") }</h4>
+            <h4>{ _tDom("Own your conversations.") }</h4>
         </React.Fragment>;
     }
 
@@ -123,13 +123,13 @@ const HomePage: React.FC<IProps> = ({ justRegistered = false }) => {
             { introSection }
             <div className="mx_HomePage_default_buttons">
                 <AccessibleButton onClick={onClickSendDm} className="mx_HomePage_button_sendDm">
-                    { _t("Send a Direct Message") }
+                    { _tDom("Send a Direct Message") }
                 </AccessibleButton>
                 <AccessibleButton onClick={onClickExplore} className="mx_HomePage_button_explore">
-                    { _t("Explore Public Rooms") }
+                    { _tDom("Explore Public Rooms") }
                 </AccessibleButton>
                 <AccessibleButton onClick={onClickNewRoom} className="mx_HomePage_button_createGroup">
-                    { _t("Create a Group Chat") }
+                    { _tDom("Create a Group Chat") }
                 </AccessibleButton>
             </div>
         </div>

src/components/views/elements/MiniAvatarUploader.tsx

@@ -24,14 +24,15 @@ import MatrixClientContext from "../../../contexts/MatrixClientContext";
 import { useTimeout } from "../../../hooks/useTimeout";
 import Analytics from "../../../Analytics";
 import CountlyAnalytics from '../../../CountlyAnalytics';
+import { TranslatedString } from '../../../languageHandler';
 import RoomContext from "../../../contexts/RoomContext";
 
 export const AVATAR_SIZE = 52;
 
 interface IProps {
     hasAvatar: boolean;
-    noAvatarLabel?: string;
+    noAvatarLabel?: TranslatedString;
-    hasAvatarLabel?: string;
+    hasAvatarLabel?: TranslatedString;
     setAvatarUrl(url: string): Promise<unknown>;
 }
 

src/i18n/strings/en_EN.json

@@ -2976,15 +2976,6 @@
     "Community %(groupId)s not found": "Community %(groupId)s not found",
     "This homeserver does not support communities": "This homeserver does not support communities",
     "Failed to load %(groupId)s": "Failed to load %(groupId)s",
-    "Great, that'll help people know it's you": "Great, that'll help people know it's you",
-    "Add a photo so people know it's you.": "Add a photo so people know it's you.",
-    "Welcome %(name)s": "Welcome %(name)s",
-    "Now, let's help you get started": "Now, let's help you get started",
-    "Welcome to %(appName)s": "Welcome to %(appName)s",
-    "Own your conversations.": "Own your conversations.",
-    "Send a Direct Message": "Send a Direct Message",
-    "Explore Public Rooms": "Explore Public Rooms",
-    "Create a Group Chat": "Create a Group Chat",
     "Upgrade to %(hostSignupBrand)s": "Upgrade to %(hostSignupBrand)s",
     "Open dial pad": "Open dial pad",
     "Public community": "Public community",

src/languageHandler.tsx

@@ -38,8 +38,9 @@ const ANNOTATE_STRINGS = false;
 
 // We use english strings as keys, some of which contain full stops
 counterpart.setSeparator('|');
-// Fall back to English
+
-counterpart.setFallbackLocale('en');
+// see `translateWithFallback` for an explanation of fallback handling
+const FALLBACK_LOCALE = 'en';
 
 interface ITranslatableError extends Error {
     translatedMessage: string;
@@ -72,9 +73,32 @@ export function _td(s: string): string { // eslint-disable-line @typescript-esli
     return s;
 }
 
+/**
+ * to improve screen reader experience translations that are not in the main page language
+ * eg a translation that fell back to english from another language
+ * should be wrapped with an appropriate `lang='en'` attribute
+ * counterpart's `translate` doesn't expose a way to determine if the resulting translation
+ * is in the target locale or a fallback locale
+ * for this reason, we do not set a fallback via `counterpart.setFallbackLocale`
+ * and fallback 'manually' so we can mark fallback strings appropriately
+ * */
+const translateWithFallback = (text: string, options?: object): { translated?: string, isFallback?: boolean } => {
+    const translated = counterpart.translate(text, options);
+    if (/^missing translation:/.test(translated)) {
+        const fallbackTranslated = counterpart.translate(text, { ...options, fallbackLocale: FALLBACK_LOCALE });
+        return { translated: fallbackTranslated, isFallback: true };
+    }
+    return { translated };
+};
+
 // Wrapper for counterpart's translation function so that it handles nulls and undefineds properly
 // Takes the same arguments as counterpart.translate()
-function safeCounterpartTranslate(text: string, options?: object) {
+function safeCounterpartTranslate(text: string, variables?: object) {
+    // Don't do substitutions in counterpart. We handle it ourselves so we can replace with React components
+    // However, still pass the variables to counterpart so that it can choose the correct plural if count is given
+    // It is enough to pass the count variable, but in the future counterpart might make use of other information too
+    const options = { ...variables, interpolate: false };
+
     // Horrible hack to avoid https://github.com/vector-im/element-web/issues/4191
     // The interpolation library that counterpart uses does not support undefined/null
     // values and instead will throw an error. This is a problem since everywhere else
@@ -82,10 +106,7 @@ function safeCounterpartTranslate(text: string, options?: object) {
     // valid ES6 template strings to i18n strings it's extremely easy to pass undefined/null
     // if there are no existing null guards. To avoid this making the app completely inoperable,
     // we'll check all the values for undefined/null and stringify them here.
-    let count;
-
     if (options && typeof options === 'object') {
-        count = options['count'];
         Object.keys(options).forEach((k) => {
             if (options[k] === undefined) {
                 logger.warn("safeCounterpartTranslate called with undefined interpolation name: " + k);
@@ -97,13 +118,7 @@ function safeCounterpartTranslate(text: string, options?: object) {
             }
         });
     }
-    let translated = counterpart.translate(text, options);
+    return translateWithFallback(text, options);
-    if (translated === undefined && count !== undefined) {
-        // counterpart does not do fallback if no pluralisation exists
-        // in the preferred language, so do it here
-        translated = counterpart.translate(text, Object.assign({}, options, { locale: 'en' }));
-    }
-    return translated;
 }
 
 type SubstitutionValue = number | string | React.ReactNode | ((sub: string) => React.ReactNode);
@@ -117,6 +132,20 @@ export type Tags = Record<string, SubstitutionValue>;
 
 export type TranslatedString = string | React.ReactNode;
 
+// For development/testing purposes it is useful to also output the original string
+// Don't do that for release versions
+const annotateStrings = (result: TranslatedString, translationKey: string): TranslatedString => {
+    if (!ANNOTATE_STRINGS) {
+        return result;
+    }
+
+    if (typeof result === 'string') {
+        return `@@${translationKey}##${result}@@`;
+    } else {
+        return <span className='translated-string' data-orig-string={translationKey}>{ result }</span>;
+    }
+};
+
 /*
  * Translates text and optionally also replaces XML-ish elements in the text with e.g. React components
  * @param {string} text The untranslated text, e.g "click <a>here</a> now to %(foo)s".
@@ -134,31 +163,39 @@ export type TranslatedString = string | React.ReactNode;
  * @return a React <span> component if any non-strings were used in substitutions, otherwise a string
  */
 // eslint-next-line @typescript-eslint/naming-convention
-// eslint-nexline @typescript-eslint/naming-convention
 export function _t(text: string, variables?: IVariables): string;
 export function _t(text: string, variables: IVariables, tags: Tags): React.ReactNode;
 export function _t(text: string, variables?: IVariables, tags?: Tags): TranslatedString {
-    // Don't do substitutions in counterpart. We handle it ourselves so we can replace with React components
-    // However, still pass the variables to counterpart so that it can choose the correct plural if count is given
-    // It is enough to pass the count variable, but in the future counterpart might make use of other information too
-    const args = Object.assign({ interpolate: false }, variables);
-
     // The translation returns text so there's no XSS vector here (no unsafe HTML, no code execution)
-    const translated = safeCounterpartTranslate(text, args);
+    const { translated } = safeCounterpartTranslate(text, variables);
 
     const substituted = substitute(translated, variables, tags);
 
-    // For development/testing purposes it is useful to also output the original string
+    return annotateStrings(substituted, text);
-    // Don't do that for release versions
+}
-    if (ANNOTATE_STRINGS) {
+
-        if (typeof substituted === 'string') {
+/*
-            return `@@${text}##${substituted}@@`;
+ * Wraps normal _t function and adds atttribution for translations that used a fallback locale
-        } else {
+ * Wraps translations that fell back from active locale to fallback locale with a `<span lang=<fallback locale>>`
-            return <span className='translated-string' data-orig-string={text}>{ substituted }</span>;
+ * @param {string} text The untranslated text, e.g "click <a>here</a> now to %(foo)s".
-        }
+ * @param {object} variables Variable substitutions, e.g { foo: 'bar' }
-    } else {
+ * @param {object} tags Tag substitutions e.g. { 'a': (sub) => <a>{sub}</a> }
-        return substituted;
+ *
-    }
+ * @return a React <span> component if any non-strings were used in substitutions
+ * or translation used a fallback locale, otherwise a string
+ */
+// eslint-next-line @typescript-eslint/naming-convention
+export function _tDom(text: string, variables?: IVariables): TranslatedString;
+export function _tDom(text: string, variables: IVariables, tags: Tags): React.ReactNode;
+export function _tDom(text: string, variables?: IVariables, tags?: Tags): TranslatedString {
+    // The translation returns text so there's no XSS vector here (no unsafe HTML, no code execution)
+    const { translated, isFallback } = safeCounterpartTranslate(text, variables);
+    const substituted = substitute(translated, variables, tags);
+
+    // wrap en fallback translation with lang attribute for screen readers
+    const result = isFallback ? <span lang='en'>{ substituted }</span> : substituted;
+
+    return annotateStrings(result, text);
 }
 
 /**

test/i18n-test/languageHandler-test.js

@@ -1,79 +0,0 @@
-import * as languageHandler from '../../src/languageHandler';
-
-const React = require('react');
-const expect = require('expect');
-
-const testUtils = require('../test-utils');
-
-describe('languageHandler', function() {
-    beforeEach(function(done) {
-        testUtils.stubClient();
-
-        languageHandler.setLanguage('en').then(done);
-        languageHandler.setMissingEntryGenerator(key => key.split("|", 2)[1]);
-    });
-
-    it('translates a string to german', function(done) {
-        languageHandler.setLanguage('de').then(function() {
-            const translated = languageHandler._t('Rooms');
-            expect(translated).toBe('Räume');
-        }).then(done);
-    });
-
-    it('handles plurals', function() {
-        const text = 'and %(count)s others...';
-        expect(languageHandler._t(text, { count: 1 })).toBe('and one other...');
-        expect(languageHandler._t(text, { count: 2 })).toBe('and 2 others...');
-    });
-
-    it('handles simple variable subsitutions', function() {
-        const text = 'You are now ignoring %(userId)s';
-        expect(languageHandler._t(text, { userId: 'foo' })).toBe('You are now ignoring foo');
-    });
-
-    it('handles simple tag substitution', function() {
-        const text = 'Press <StartChatButton> to start a chat with someone';
-        expect(languageHandler._t(text, {}, { 'StartChatButton': () => 'foo' }))
-            .toBe('Press foo to start a chat with someone');
-    });
-
-    it('handles text in tags', function() {
-        const text = '<a>Click here</a> to join the discussion!';
-        expect(languageHandler._t(text, {}, { 'a': (sub) => `x${sub}x` }))
-            .toBe('xClick herex to join the discussion!');
-    });
-
-    it('variable substitution with React component', function() {
-        const text = 'You are now ignoring %(userId)s';
-        expect(languageHandler._t(text, { userId: () => <i>foo</i> }))
-            .toEqual((<span>You are now ignoring <i>foo</i></span>));
-    });
-
-    it('variable substitution with plain React component', function() {
-        const text = 'You are now ignoring %(userId)s';
-        expect(languageHandler._t(text, { userId: <i>foo</i> }))
-            .toEqual((<span>You are now ignoring <i>foo</i></span>));
-    });
-
-    it('tag substitution with React component', function() {
-        const text = 'Press <StartChatButton> to start a chat with someone';
-        expect(languageHandler._t(text, {}, { 'StartChatButton': () => <i>foo</i> }))
-            .toEqual(<span>Press <i>foo</i> to start a chat with someone</span>);
-    });
-
-    it('replacements in the wrong order', function() {
-        const text = '%(var1)s %(var2)s';
-        expect(languageHandler._t(text, { var2: 'val2', var1: 'val1' })).toBe('val1 val2');
-    });
-
-    it('multiple replacements of the same variable', function() {
-        const text = '%(var1)s %(var1)s';
-        expect(languageHandler.substitute(text, { var1: 'val1' })).toBe('val1 val1');
-    });
-
-    it('multiple replacements of the same tag', function() {
-        const text = '<a>Click here</a> to join the discussion! <a>or here</a>';
-        expect(languageHandler.substitute(text, {}, { 'a': (sub) => `x${sub}x` }))
-            .toBe('xClick herex to join the discussion! xor herex');
-    });
-});

test/i18n-test/languageHandler-test.tsx

@@ -0,0 +1,144 @@
+import React from 'react';
+
+import {
+    _t,
+    _tDom,
+    TranslatedString,
+    setLanguage,
+    setMissingEntryGenerator,
+    substitute,
+} from '../../src/languageHandler';
+import { stubClient } from '../test-utils';
+
+describe('languageHandler', function() {
+    const basicString = 'Rooms';
+    const selfClosingTagSub = 'Accept <policyLink /> to continue:';
+    const textInTagSub = '<a>Upgrade</a> to your own domain';
+    const plurals = 'and %(count)s others...';
+    const variableSub = 'You are now ignoring %(userId)s';
+
+    type TestCase = [string, string, Record<string, unknown>, Record<string, unknown>, TranslatedString];
+    const testCasesEn: TestCase[] = [
+        ['translates a basic string', basicString, {}, undefined, 'Rooms'],
+        [
+            'handles plurals when count is 1',
+            plurals,
+            { count: 1 },
+            undefined,
+            'and one other...',
+        ],
+        [
+            'handles plurals when count is not 1',
+            plurals,
+            { count: 2 },
+            undefined,
+            'and 2 others...',
+        ],
+        [
+            'handles simple variable substitution',
+            variableSub,
+            { userId: 'foo' },
+            undefined,
+            'You are now ignoring foo',
+        ],
+        [
+            'handles simple tag substitution',
+            selfClosingTagSub,
+            {},
+            { 'policyLink': () => 'foo' },
+            'Accept foo to continue:',
+        ],
+        ['handles text in tags', textInTagSub, {}, { 'a': (sub) => `x${sub}x` }, 'xUpgradex to your own domain'],
+        [
+            'handles variable substitution with React function component',
+            variableSub,
+            { userId: () => <i>foo</i> },
+            undefined,
+            // eslint-disable-next-line react/jsx-key
+            <span>You are now ignoring <i>foo</i></span>,
+        ],
+        [
+            'handles variable substitution with react node',
+            variableSub,
+            { userId: <i>foo</i> },
+            undefined,
+            // eslint-disable-next-line react/jsx-key
+            <span>You are now ignoring <i>foo</i></span>,
+        ],
+        [
+            'handles tag substitution with React function component',
+            selfClosingTagSub,
+            {},
+            { 'policyLink': () => <i>foo</i> },
+            // eslint-disable-next-line react/jsx-key
+            <span>Accept <i>foo</i> to continue:</span>,
+        ],
+    ];
+
+    describe('when translations exist in language', () => {
+        beforeEach(function(done) {
+            stubClient();
+
+            setLanguage('en').then(done);
+            setMissingEntryGenerator(key => key.split("|", 2)[1]);
+        });
+
+        it('translates a string to german', function(done) {
+            setLanguage('de').then(function() {
+                const translated = _t(basicString);
+                expect(translated).toBe('Räume');
+            }).then(done);
+        });
+
+        it.each(testCasesEn)("%s", async (_d, translationString, variables, tags, result) => {
+            expect(_t(translationString, variables, tags)).toEqual(result);
+        });
+
+        it('replacements in the wrong order', function() {
+            const text = '%(var1)s %(var2)s';
+            expect(_t(text, { var2: 'val2', var1: 'val1' })).toBe('val1 val2');
+        });
+
+        it('multiple replacements of the same variable', function() {
+            const text = '%(var1)s %(var1)s';
+            expect(substitute(text, { var1: 'val1' })).toBe('val1 val1');
+        });
+
+        it('multiple replacements of the same tag', function() {
+            const text = '<a>Click here</a> to join the discussion! <a>or here</a>';
+            expect(substitute(text, {}, { 'a': (sub) => `x${sub}x` }))
+                .toBe('xClick herex to join the discussion! xor herex');
+        });
+    });
+
+    describe('when a translation string does not exist in active language', () => {
+        beforeEach(async () => {
+            stubClient();
+            await setLanguage('lv');
+            // counterpart doesnt expose any way to restore default config
+            // missingEntryGenerator is mocked in the root setup file
+            // reset to default here
+            const counterpartDefaultMissingEntryGen =
+                function(key) { return 'missing translation: ' + key; };
+            setMissingEntryGenerator(counterpartDefaultMissingEntryGen);
+        });
+
+        describe('_t', () => {
+            it.each(testCasesEn)(
+                "%s and translates with fallback locale",
+                async (_d, translationString, variables, tags, result) => {
+                    expect(_t(translationString, variables, tags)).toEqual(result);
+                },
+            );
+        });
+
+        describe('_tDom()', () => {
+            it.each(testCasesEn)(
+                "%s and translates with fallback locale, attributes fallback locale",
+                async (_d, translationString, variables, tags, result) => {
+                    expect(_tDom(translationString, variables, tags)).toEqual(<span lang="en">{ result }</span>);
+                },
+            );
+        });
+    });
+});