feat: Add a leading RLM to RTL translations that start with LTR text

pontoon/checks/libraries/translate_toolkit.py

@@ -3,10 +3,16 @@
 from translate.storage import base as storage_base
 
 
+# Implicit directional marks (LRM, RLM, ALM) are invisible, and RTL translations
+# may start with an RLM to set their base direction (GitHub issue #3236).
+# They are not punctuation, so ignore them at the start and end of strings.
+BIDI_MARKS = "\u200e\u200f\u061c"
+
+
 def run_checks(original, string, locale_code, disabled_checks=None):
     """Check for obvious errors like blanks and missing interpunction."""
-    original = lang_data.normalize(original)
-    string = lang_data.normalize(string)
+    original = lang_data.normalize(original).strip(BIDI_MARKS)
+    string = lang_data.normalize(string).strip(BIDI_MARKS)
     disabled_checks = disabled_checks or []
 
     unit = storage_base.TranslationUnit(original)

pontoon/checks/tests/test_translate_toolkit.py

@@ -40,3 +40,12 @@ def test_tt_correct_translation(mock_locale):
     Quality check should return empty dictionary if everything is okay (no warnings).
     """
     assert run_checks("Original string", "Translation string", mock_locale) == {}
+
+
+@pytest.mark.parametrize("mark", ["\u200e", "\u200f", "\u061c"])
+def test_tt_bidi_marks(mark):
+    """
+    Leading and trailing implicit directional marks are not punctuation.
+    """
+    assert run_checks("Original string", f"{mark}Translation string", "fa") == {}
+    assert run_checks("Original string", f"Translation string{mark}", "fa") == {}

translate/public/locale/en-US/translate.ftl

@@ -240,6 +240,15 @@ editor-MachinerySourceIndicator--match = <stress>100%</stress> MATCH
     .title = 100% Translation Memory match
 
 
+## Editor right-to-left mark indicator
+## Shown for right-to-left locales when the translation starts with left-to-right text,
+## and an invisible right-to-left mark (RLM) will be added to its start when saved.
+## “RLM” and “LRM” are abbreviations of Unicode character names and should not be translated.
+
+editor-RtlMarkIndicator--rlm = RLM
+    .title = A right-to-left mark (RLM) will be inserted at the beginning of this translation so that it will be displayed right-to-left. To avoid this, you should prefix the translation with a left-to-right mark (LRM).
+
+
 ## Editor New Contributor Tooltip
 ## Renders the guidelines for new contributors
 

translate/src/context/Editor.tsx

@@ -28,6 +28,7 @@ import {
   htmlElementEscapes,
 } from '~/utils/message/entryInformation';
 import { messageEntryFromEntityTranslation } from '~/utils/message/fromEntity';
+import { addRtlMarks, removeRtlMark } from '~/utils/message/rtlMark';
 import { getMessageEntryFormat } from '~/utils/message/getMessageEntryFormat';
 import { specialFormats } from '~/utils/message/specialFormats';
 import { pojoEquals } from '~/utils/pojo';
@@ -265,13 +266,20 @@ export function EditorProvider({ children }: { children: React.ReactElement }) {
   const pendingFieldValues = useRef<Array<[string, string]> | null>(null);
   const [result, setResult] = useState<MessageEntry | null>(null);
 
+  // A leading RLM is kept out of the editor, and added back on save.
+  // https://github.com/mozilla/pontoon/issues/3236
+  const rtl = locale.direction === 'rtl';
+  const editOpts = { rtl };
+  const editString = (str: string) => (rtl ? removeRtlMark(str) : str);
+
   const actions = useMemo<EditorActions>(() => {
     if (readonly) {
       return initEditorActions;
     }
     const buildOpts = {
       escapeHTML: htmlElementEscapes(sourceEntry),
       trim: !hasOuterWhitespace(sourceEntry),
+      rtl,
     };
 
     const resetFields = (next: EditorData): EditorData => {
@@ -302,7 +310,7 @@ export function EditorProvider({ children }: { children: React.ReactElement }) {
         setState((prev) => {
           const { fields, focusField, sourceView } = prev;
           let field = focusField.current ?? fields[0];
-          field.handle.current.setValue(str);
+          field.handle.current.setValue(editString(str));
           const next = {
             ...prev,
             machinery: { manual, translation: str, sources },
@@ -337,7 +345,7 @@ export function EditorProvider({ children }: { children: React.ReactElement }) {
           } else {
             next.fields = prev.sourceView
               ? editSource(entry)
-              : editMessageEntry(sourceEntry, entry);
+              : editMessageEntry(sourceEntry, entry, editOpts);
           }
           const state = resetFields(next);
           return {
@@ -370,14 +378,14 @@ export function EditorProvider({ children }: { children: React.ReactElement }) {
             if (entry && !requiresSourceView(entry)) {
               next.fields = prev.sourceView
                 ? editSource(entry)
-                : editMessageEntry(sourceEntry, entry);
+                : editMessageEntry(sourceEntry, entry, editOpts);
             } else {
               next.fields = editSource(str);
               next.sourceView = true;
             }
           } else {
-            next.fields = editMessageEntry(sourceEntry, prev.initial);
-            next.fields[0].handle.current.setValue(str);
+            next.fields = editMessageEntry(sourceEntry, prev.initial, editOpts);
+            next.fields[0].handle.current.setValue(editString(str));
           }
           return resetFields(next);
         }),
@@ -395,9 +403,12 @@ export function EditorProvider({ children }: { children: React.ReactElement }) {
         setState((state) => {
           // Inside setState() only to access the current `state` value
           const { base, fields, sourceView } = state;
-          const result = sourceView
+          let result = sourceView
             ? parseEntryFromFluentSource(sourceEntry, fields)
             : buildMessageEntry(base, fields, buildOpts);
+          if (sourceView && rtl && result) {
+            result = addRtlMarks(result);
+          }
           setResult(result);
           return state;
         }),
@@ -409,9 +420,13 @@ export function EditorProvider({ children }: { children: React.ReactElement }) {
             const entry = parseEntryFromFluentSource(sourceEntry, fields);
             if (entry && !requiresSourceView(entry)) {
               includeSourceAttributesAndDeclarations(entry, sourceEntry);
-              const fields = editMessageEntry(sourceEntry, entry);
+              const fields = editMessageEntry(sourceEntry, entry, editOpts);
               state.focusField.current = fields[0];
-              setResult(entry);
+              setResult(
+                rtl
+                  ? (buildMessageEntry(entry, fields, buildOpts) ?? entry)
+                  : entry,
+              );
               return { ...state, base: entry, fields, sourceView: false };
             }
           } else if (format === 'fluent') {
@@ -427,15 +442,15 @@ export function EditorProvider({ children }: { children: React.ReactElement }) {
           return state;
         }),
     };
-  }, [format, readonly, sourceEntry]);
+  }, [format, readonly, rtl, sourceEntry]);
 
   useEffect(() => {
     const base = messageEntryFromEntityTranslation(entity, locale);
     includeSourceAttributesAndDeclarations(base, sourceEntry);
     const sourceView = requiresSourceView(base);
     const fields = sourceView
       ? editSource(serializeEntry(base))
-      : editMessageEntry(sourceEntry, base);
+      : editMessageEntry(sourceEntry, base, editOpts);
     setState(() => ({
       pk: entity.pk,
       busy: false,

translate/src/modules/editor/components/EditorMenu.tsx

@@ -17,6 +17,7 @@ import { FailedChecks } from './FailedChecks';
 import { FtlSwitch } from './FtlSwitch';
 import { KeyboardShortcuts } from './KeyboardShortcuts';
 import { MachinerySourceIndicator } from './MachinerySourceIndicator';
+import { RtlMarkIndicator } from './RtlMarkIndicator';
 import { TranslationLength } from './TranslationLength';
 
 /**
@@ -81,6 +82,7 @@ function MenuContent() {
       <KeyboardShortcuts />
       <TranslationLength />
       <MachinerySourceIndicator />
+      <RtlMarkIndicator />
       <div className='actions'>
         <Localized id='editor-EditorMenu--button-copy' attrs={{ title: true }}>
           <button

translate/src/modules/editor/components/RtlMarkIndicator.css

@@ -0,0 +1,12 @@
+.rtl-mark {
+  background-color: var(--background-1);
+  border: 1px solid var(--main-border-1);
+  border-radius: 3px;
+  color: var(--editor-menu-color);
+  cursor: help;
+  float: left;
+  font-size: 11px;
+  line-height: 20px;
+  margin-left: 5px;
+  padding: 9px;
+}

translate/src/modules/editor/components/RtlMarkIndicator.test.jsx

@@ -0,0 +1,102 @@
+import React from 'react';
+import { render } from '@testing-library/react';
+import { describe, expect, it } from 'vitest';
+
+import { EditorData, EditorResult } from '~/context/Editor';
+import { Locale } from '~/context/Locale';
+import { MockLocalizationProvider } from '~/test/utils';
+import { parseEntry } from '~/utils/message';
+import { addRtlMarks } from '~/utils/message/rtlMark';
+
+import { RtlMarkIndicator } from './RtlMarkIndicator';
+
+const field = (value) => ({ handle: { current: { value } } });
+
+const mount = ({
+  direction = 'rtl',
+  values = ['{ $name } را باز کنید'],
+  sourceView = false,
+  result = null,
+}) =>
+  render(
+    <MockLocalizationProvider
+      resources={[
+        `editor-RtlMarkIndicator--rlm = RLM
+    .title = A right-to-left mark (RLM) will be inserted`,
+      ]}
+    >
+      <Locale.Provider value={{ direction }}>
+        <EditorData.Provider value={{ fields: values.map(field), sourceView }}>
+          <EditorResult.Provider value={result}>
+            <RtlMarkIndicator />
+          </EditorResult.Provider>
+        </EditorData.Provider>
+      </Locale.Provider>
+    </MockLocalizationProvider>,
+  );
+
+describe('<RtlMarkIndicator>', () => {
+  it('shows when an RTL translation starts with LTR content', () => {
+    const { container } = mount({});
+
+    const indicator = container.querySelector('.rtl-mark');
+    expect(indicator).not.toBeNull();
+    expect(indicator.textContent).toBe('RLM');
+    expect(indicator.getAttribute('title')).toBeTruthy();
+  });
+
+  it('shows when any of the fields needs a mark', () => {
+    const { container } = mount({ values: ['فایل', 'Firefox فایل'] });
+
+    expect(container.querySelector('.rtl-mark')).not.toBeNull();
+  });
+
+  it('shows nothing for content starting with RTL text', () => {
+    const { container } = mount({ values: ['فایرفاکس را باز کنید'] });
+
+    expect(container.querySelector('.rtl-mark')).toBeNull();
+  });
+
+  it('shows nothing for unchanged translations', () => {
+    const result = parseEntry('fluent', 'key = { $n } فایل');
+    const { container } = render(
+      <MockLocalizationProvider>
+        <Locale.Provider value={{ direction: 'rtl' }}>
+          <EditorData.Provider
+            value={{
+              fields: [field('{ $n } فایل')],
+              initial: result,
+              sourceView: false,
+            }}
+          >
+            <EditorResult.Provider value={result}>
+              <RtlMarkIndicator />
+            </EditorResult.Provider>
+          </EditorData.Provider>
+        </Locale.Provider>
+      </MockLocalizationProvider>,
+    );
+
+    expect(container.querySelector('.rtl-mark')).toBeNull();
+  });
+
+  it('shows nothing for LTR locales', () => {
+    const { container } = mount({ direction: 'ltr' });
+
+    expect(container.querySelector('.rtl-mark')).toBeNull();
+  });
+
+  it('shows in the source view when the result gets a mark', () => {
+    const result = addRtlMarks(parseEntry('fluent', 'key = { $n } فایل'));
+    const { container } = mount({ sourceView: true, result });
+
+    expect(container.querySelector('.rtl-mark')).not.toBeNull();
+  });
+
+  it('shows nothing in the source view without a mark', () => {
+    const result = parseEntry('fluent', 'key = فایل { $n }');
+    const { container } = mount({ sourceView: true, result });
+
+    expect(container.querySelector('.rtl-mark')).toBeNull();
+  });
+});

translate/src/modules/editor/components/RtlMarkIndicator.tsx

@@ -0,0 +1,42 @@
+import { Localized } from '@fluent/react';
+import React, { useContext } from 'react';
+
+import { EditorData, EditorResult } from '~/context/Editor';
+import { Locale } from '~/context/Locale';
+import { hasAddedRtlMark, needsRtlMark } from '~/utils/message/rtlMark';
+import { pojoEquals } from '~/utils/pojo';
+
+import './RtlMarkIndicator.css';
+
+/**
+ * Shows when a right-to-left mark will be added to the start of the translation.
+ * Unchanged translations are not saved again, so it's not shown for them.
+ * https://github.com/mozilla/pontoon/issues/3236
+ */
+export function RtlMarkIndicator() {
+  const { direction } = useContext(Locale);
+  const { fields, initial, sourceView } = useContext(EditorData);
+  const result = useContext(EditorResult);
+
+  // In the source view, the mark is visible in the result
+  const show =
+    direction === 'rtl' &&
+    !(result && pojoEquals(initial, result)) &&
+    (sourceView
+      ? !!result && hasAddedRtlMark(result)
+      : fields.some((field) => needsRtlMark(field.handle.current.value)));
+  if (!show) {
+    return null;
+  }
+
+  return (
+    <Localized id='editor-RtlMarkIndicator--rlm' attrs={{ title: true }}>
+      <div
+        className='rtl-mark'
+        title='A right-to-left mark (RLM) will be inserted at the beginning of this translation so that it will be displayed right-to-left. To avoid this, you should prefix the translation with a left-to-right mark (LRM).'
+      >
+        RLM
+      </div>
+    </Localized>
+  );
+}

translate/src/modules/editor/components/TranslationLength.test.jsx

@@ -2,6 +2,8 @@ import React, { useContext } from 'react';
 
 import { EditorData, EditorResult } from '~/context/Editor';
 import { EntityView } from '~/context/EntityView';
+import { Locale } from '~/context/Locale';
+import { RLM } from '~/utils/message/rtlMark';
 
 import { TranslationLength } from './TranslationLength';
 import { vi } from 'vitest';
@@ -17,8 +19,15 @@ describe('<TranslationLength>', () => {
     vi.restoreAllMocks();
   });
 
-  function mountTranslationLength(format, source, value, comment) {
+  function mountTranslationLength(
+    format,
+    source,
+    value,
+    comment,
+    direction = 'ltr',
+  ) {
     const context = new Map([
+      [Locale, { direction }],
       [EditorData, { sourceView: false, fields: [{}] }],
       [EditorResult, { value: [value] }],
       [EntityView, { entity: { key: ['id'], comment, format, value: source } }],
@@ -68,4 +77,17 @@ describe('<TranslationLength>', () => {
     const div = container.querySelector('.translation-vs-original');
     expect(div.childNodes[0].textContent).toEqual('19');
   });
+
+  it('does not count an RLM added to RTL translations', () => {
+    const { container } = mountTranslationLength(
+      '',
+      ['12345'],
+      `${RLM}%s فایل`,
+      '',
+      'rtl',
+    );
+
+    const div = container.querySelector('.translation-vs-original');
+    expect(div.childNodes[0].textContent).toEqual('7');
+  });
 });