Fix bug 1616918 - Correctly compare with Fluent strings in history (#…

…1574)

This solves a few comparison problems when looking for a Fluent translation in the history. It makes sure to turn all FTL string in Fluent messages, to use the `.equals` method and make sure comparison works correctly. This avoid bugs when a Fluent string misses the trailing newline character, for example. It also fly-by solves issues related to the unsaved changes checks.

frontend/src/core/editor/selectors.js

@@ -62,19 +62,26 @@ export function _existingTranslation(
         }
         // If translation is a string, from the generic editor.
         else {
-            // Except it might actually be a simple Fluent message from the SimpleEditor.
+            // Except it might actually be a Fluent message from the Simple or Source
+            // editors.
             if (entity.format === 'ftl') {
-                // This case happens when the format is Fluent and the string is simple.
-                // We thus store a simplified version of the Fluent message in memory,
-                // but the history has actual Fluent-formatted strings (with ID).
-                // Thus, we need to reconstruct the Fluent message in order to be able
-                // to compare it with items in history.
-                const reconstructed = fluent.serializer.serializeEntry(
-                    fluent.getReconstructedMessage(entity.original, translation)
-                );
+                // For Fluent files, the translation can be stored as a simple string
+                // when the Source editor or the Simple editor are on. Because of that,
+                // we want to turn the string into a Fluent message, as that's simpler
+                // to handle and less prone to errors. We do the same for each history
+                // entry.
+                let fluentTranslation = fluent.parser.parseEntry(translation);
+                if (fluentTranslation.type === 'Junk') {
+                    // If the message was junk, it means we are likely in the Simple
+                    // editor, and we thus want to reconstruct the Fluent message.
+                    // Note that if the user is actually in the Source editor, and
+                    // entered an invalid value (which creates this junk entry),
+                    // it doesn't matter as there shouldn't be anything matching anyway.
+                    fluentTranslation = fluent.getReconstructedMessage(entity.original, translation);
+                }
                 existingTranslation = history.translations.find(
-                    t => t.string === reconstructed
-                )
+                    t => fluentTranslation.equals(fluent.parser.parseEntry(t.string))
+                );
             }
             else {
                 existingTranslation = history.translations.find(

frontend/src/modules/fluenteditor/components/Editor.js

@@ -146,7 +146,11 @@ export class EditorBase extends React.Component<EditorProps, State> {
             }
         }
 
-        props.setInitialTranslation(translationContent);
+        // Update the initial translation content only when the entity changed, not
+        // when new content is loaded from an external source.
+        if (typeof translation === 'undefined') {
+            props.setInitialTranslation(translationContent);
+        }
         props.updateTranslation(translationContent, true);
     }
 

frontend/src/modules/genericeditor/components/Editor.js

@@ -12,6 +12,7 @@ import type { EditorProps } from 'core/editor';
 
 export class EditorBase extends React.Component<EditorProps> {
     componentDidMount() {
+        this.props.setInitialTranslation(this.props.activeTranslationString);
         this.props.updateTranslation(this.props.activeTranslationString, true);
     }
 
@@ -22,6 +23,7 @@ export class EditorBase extends React.Component<EditorProps> {
             pluralForm !== prevProps.pluralForm ||
             entity !== prevProps.entity
         ) {
+            this.props.setInitialTranslation(activeTranslationString);
             this.props.updateTranslation(activeTranslationString, true);
         }
     }

frontend/src/modules/genericeditor/components/Editor.test.js

@@ -22,26 +22,33 @@ const ENTITIES = [
 
 
 function createEditorBase() {
+    const setInitialTranslationMock = sinon.stub();
     const updateTranslationMock = sinon.stub();
     const wrapper = shallow(<EditorBase
         editor={ { translation: '' } }
         pluralForm={ -1 }
         entity={ ENTITIES[0] }
         activeTranslationString={ ENTITIES[0].translation[0].string }
+        setInitialTranslation={ setInitialTranslationMock }
         updateTranslation={ updateTranslationMock }
     />);
 
-    return [wrapper, updateTranslationMock];
+    return [wrapper, setInitialTranslationMock, updateTranslationMock];
 }
 
 
 describe('<Editor>', () => {
     it('updates translation on mount', () => {
-        const [, updateTranslationMock] = createEditorBase();
+        const [, , updateTranslationMock] = createEditorBase();
         expect(updateTranslationMock.calledOnce).toBeTruthy();
     });
 
-    it('updates translation on component update', () => {
+    it('sets initial translation on mount', () => {
+        const [, setInitialTranslationMock, ] = createEditorBase();
+        expect(setInitialTranslationMock.calledOnce).toBeTruthy();
+    });
+
+    it('updates translation when entity or plural change', () => {
         const [wrapper, updateTranslationMock] = createEditorBase();
         expect(updateTranslationMock.calledOnce).toBeTruthy();
 
@@ -51,4 +58,31 @@ describe('<Editor>', () => {
         wrapper.setProps({ pluralForm: 1 });
         expect(updateTranslationMock.calledThrice).toBeTruthy();
     });
+
+    it('sets initial translation when entity or plural change', () => {
+        const [wrapper, setInitialTranslationMock, ] = createEditorBase();
+        expect(setInitialTranslationMock.calledOnce).toBeTruthy();
+
+        wrapper.setProps({ entity: ENTITIES[1] });
+        expect(setInitialTranslationMock.calledTwice).toBeTruthy();
+
+        wrapper.setProps({ pluralForm: 1 });
+        expect(setInitialTranslationMock.calledThrice).toBeTruthy();
+    });
+
+    it('does not update translation when translation changes', () => {
+        const [wrapper, updateTranslationMock] = createEditorBase();
+        expect(updateTranslationMock.calledOnce).toBeTruthy();
+
+        wrapper.setProps({ editor: { translation: 'hello' } });
+        expect(updateTranslationMock.calledOnce).toBeTruthy();
+    });
+
+    it('does not set initial translation when translation changes', () => {
+        const [wrapper, setInitialTranslationMock, ] = createEditorBase();
+        expect(setInitialTranslationMock.calledOnce).toBeTruthy();
+
+        wrapper.setProps({ editor: { translation: 'hello' } });
+        expect(setInitialTranslationMock.calledOnce).toBeTruthy();
+    });
 });

frontend/src/modules/genericeditor/components/GenericTranslationForm.js

@@ -22,11 +22,6 @@ export default class GenericTranslationForm extends React.Component<EditorProps>
     }
 
     componentDidUpdate(prevProps: EditorProps) {
-        // Unused by Generic Editor - reset to default value.
-        if (this.props.editor.initialTranslation) {
-            return this.props.setInitialTranslation('');
-        }
-
         // Close failed checks popup when content of the editor changes,
         // but only if the errors and warnings did not change
         // meaning they were already shown in the previous render