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

Attribute fallback i18n strings with lang attribute #7323

Merged
merged 7 commits into from
Jan 5, 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
12 changes: 11 additions & 1 deletion __mocks__/browser-request.js
Original file line number Diff line number Diff line change
@@ -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")) {
Expand All @@ -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}, "");
}
Expand Down
20 changes: 10 additions & 10 deletions src/components/structures/HomePage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -72,8 +72,8 @@ const UserWelcomeTop = () => {
return <div>
<MiniAvatarUploader
hasAvatar={!!ownProfile.avatarUrl}
hasAvatarLabel={_t("Great, that'll help people know it's you")}
noAvatarLabel={_t("Add a photo so people know it's you.")}
hasAvatarLabel={_tDom("Great, that'll help people know it's you")}
noAvatarLabel={_tDom("Add a photo so people know it's you.")}
setAvatarUrl={url => cli.setAvatarUrl(url)}
>
<BaseAvatar
Expand All @@ -86,8 +86,8 @@ const UserWelcomeTop = () => {
/>
</MiniAvatarUploader>

<h1>{ _t("Welcome %(name)s", { name: ownProfile.displayName }) }</h1>
<h4>{ _t("Now, let's help you get started") }</h4>
<h1>{ _tDom("Welcome %(name)s", { name: ownProfile.displayName }) }</h1>
<h4>{ _tDom("Now, let's help you get started") }</h4>
</div>;
};

Expand All @@ -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>
<h4>{ _t("Own your conversations.") }</h4>
<h1>{ _tDom("Welcome to %(appName)s", { appName: config.brand }) }</h1>
<h4>{ _tDom("Own your conversations.") }</h4>
</React.Fragment>;
}

Expand All @@ -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>
Expand Down
5 changes: 3 additions & 2 deletions src/components/views/elements/MiniAvatarUploader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
hasAvatarLabel?: string;
noAvatarLabel?: TranslatedString;
hasAvatarLabel?: TranslatedString;
setAvatarUrl(url: string): Promise<unknown>;
}

Expand Down
9 changes: 0 additions & 9 deletions src/i18n/strings/en_EN.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
99 changes: 68 additions & 31 deletions src/languageHandler.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -72,20 +73,40 @@ 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 } => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the isFallback parameter?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need to mark when translations are in a language != the lang attributed on the page level. The isFallback param in the result is used to identify which translations fell back to english so we can wrap them with <span lang='en'> in _tDom

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
// in JS land passing undefined/null will simply stringify instead, and when converting
// 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);
Expand All @@ -97,13 +118,7 @@ function safeCounterpartTranslate(text: string, options?: object) {
}
});
}
let translated = counterpart.translate(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;
return translateWithFallback(text, options);
}

type SubstitutionValue = number | string | React.ReactNode | ((sub: string) => React.ReactNode);
Expand All @@ -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".
Expand All @@ -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
// Don't do that for release versions
if (ANNOTATE_STRINGS) {
if (typeof substituted === 'string') {
return `@@${text}##${substituted}@@`;
} else {
return <span className='translated-string' data-orig-string={text}>{ substituted }</span>;
}
} else {
return substituted;
}
return annotateStrings(substituted, text);
}

/*
* Wraps normal _t function and adds atttribution for translations that used a fallback locale
* Wraps translations that fell back from active locale to fallback locale with a `<span lang=<fallback locale>>`
* @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' }
* @param {object} tags Tag substitutions e.g. { 'a': (sub) => <a>{sub}</a> }
*
* @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);
}

/**
Expand Down
79 changes: 0 additions & 79 deletions test/i18n-test/languageHandler-test.js

This file was deleted.

Loading