Parcourir la source

fix: avatar titles never getting set

Because the languageLoadedEvent is not a public property, the
registered callback never fired. To avoid handling this in two
places, this change moves attaching the title to the icon creation
logic, registers a callback for each one immediatly, to avoid a
gap where the translation may load between first translation
attempt and registering the callback, so the callback would not
get fired.

Instead of exposing the name of the event as a public property
that could be changed, this introduces a new interface to register
callbacks for outside consumers. The event name remains an
implementation detail. The new methods documentation explains its
existance - normally the translate method will handle such re-
translations, but that only works for text nodes or when replacing
the inner HTML of an element. For properties we need a more
flexible solution, if we don't want to expose internal details of
the I18n class. The avatar title is currently the only item that
needs this.

The alternative would be to make the icon a templated snippet.
Then the translation of the title would occur once and server side
and the addComment method would just copy that into place.
El RIDO il y a 6 jours
Parent
commit
323c72ef92
3 fichiers modifiés avec 65 ajouts et 27 suppressions
  1. 32 10
      js/privatebin.js
  2. 32 16
      js/test/I18n.js
  3. 1 1
      lib/Configuration.php

+ 32 - 10
js/privatebin.js

@@ -945,6 +945,27 @@ window.PrivateBin = (function () {
                 });
         };
 
+        /**
+         * Register a callback to be invoked when translations have been loaded.
+         *
+         * This is useful for code that needs to re-apply translated strings to
+         * DOM attributes (e.g. title attributes) that cannot be handled by
+         * I18n.translate's built-in element re-translation mechanism (which only
+         * sets textContent/innerHTML).
+         *
+         * Note: If translations are already loaded when this is called, the
+         * callback will not be invoked. Callers should apply the translation
+         * immediately as well, to cover the case where the language is already
+         * available.
+         *
+         * @name   I18n.onLanguageLoaded
+         * @function
+         * @param     {function} callback - function to call when language is loaded
+         */
+        me.onLanguageLoaded = function (callback) {
+            document.addEventListener(languageLoadedEvent, callback);
+        };
+
         /**
          * resets state, used for unit testing
          *
@@ -3628,7 +3649,18 @@ window.PrivateBin = (function () {
             // if an avatar is available, display it
             const icon = comment.getIcon();
             if (icon) {
+                const iconTitle = 'Avatar generated from IP address';
                 const image = document.createElement('img');
+                // Set the title immediately with the current translation. If
+                // the language has not been loaded yet, this will fall back to
+                // English, so we also register a callback to re-apply the
+                // translation once available. To avoid a race condition, we
+                // register the callback first, then translate - if the language
+                // loads between the two lines, both will set the translated text.
+                I18n.onLanguageLoaded(function () {
+                    image.setAttribute('title', I18n._(iconTitle));
+                });
+                image.setAttribute('title', I18n._(iconTitle));
                 image.setAttribute('src', icon);
                 image.setAttribute('class', 'vizhash');
                 const nickSpan = commentEntry.querySelector('span.nickname');
@@ -5525,16 +5557,6 @@ window.PrivateBin = (function () {
                         plaintexts[i][1]
                     );
                 }
-
-                document.addEventListener(I18n.languageLoadedEvent, function () {
-                    const commentContainer = document.getElementById('commentcontainer');
-                    if (!commentContainer) {
-                        return;
-                    }
-
-                    commentContainer.querySelectorAll('img.vizhash')
-                        .forEach(img => img.setAttribute('title', I18n._('Avatar generated from IP address')));
-                });
             });
         }
 

+ 32 - 16
js/test/I18n.js

@@ -14,24 +14,24 @@ describe('I18n', function () {
                 fc.string(),
                 function (messageId) {
                     messageId   = messageId.replace(/%(s|d)/g, '%%');
-                    var plurals = [messageId, messageId + 's'],
-                        fake    = [messageId],
-                        result  = PrivateBin.I18n.translate(messageId);
+                    const plurals = [messageId, messageId + 's'],
+                          fake    = [messageId],
+                          result  = PrivateBin.I18n.translate(messageId);
                     PrivateBin.I18n.reset();
 
-                    var alias = PrivateBin.I18n._(messageId);
+                    const alias = PrivateBin.I18n._(messageId);
                     PrivateBin.I18n.reset();
 
-                    var pluralResult = PrivateBin.I18n.translate(plurals);
+                    const pluralResult = PrivateBin.I18n.translate(plurals);
                     PrivateBin.I18n.reset();
 
-                    var pluralAlias = PrivateBin.I18n._(plurals);
+                    const pluralAlias = PrivateBin.I18n._(plurals);
                     PrivateBin.I18n.reset();
 
-                    var fakeResult = PrivateBin.I18n.translate(fake);
+                    const fakeResult = PrivateBin.I18n.translate(fake);
                     PrivateBin.I18n.reset();
 
-                    var fakeAlias = PrivateBin.I18n._(fake);
+                    const fakeAlias = PrivateBin.I18n._(fake);
                     PrivateBin.I18n.reset();
 
                     if (messageId.indexOf('<a') === -1) {
@@ -177,7 +177,7 @@ describe('I18n', function () {
                 fc.integer(),
                 function(language, n) {
                     PrivateBin.I18n.reset(language);
-                    var result = PrivateBin.I18n.getPluralForm(n);
+                    const result = PrivateBin.I18n.getPluralForm(n);
                     // arabic seems to have the highest plural count with 6 forms
                     return result >= 0 && result <= 5;
                 }
@@ -198,7 +198,7 @@ describe('I18n', function () {
                 common.fcSupportedLanguages(),
                 function(language) {
                     // cleanup
-                    var clean = globalThis.cleanup('', {cookie: ['lang=en']});
+                    let clean = globalThis.cleanup('', {cookie: ['lang=en']});
                     PrivateBin.I18n.reset('en');
                     PrivateBin.I18n.loadTranslations();
                     clean();
@@ -207,9 +207,9 @@ describe('I18n', function () {
                     clean = globalThis.cleanup('', {cookie: ['lang=' + language]});
                     // eslint-disable-next-line global-require
                     PrivateBin.I18n.reset(language, require('../../i18n/' + language + '.json'));
-                    var loadedLang = PrivateBin.I18n.getLanguage(),
-                        result = PrivateBin.I18n.translate('Never'),
-                        alias  = PrivateBin.I18n._('Never');
+                    const loadedLang = PrivateBin.I18n.getLanguage(),
+                          result     = PrivateBin.I18n.translate('Never'),
+                          alias      = PrivateBin.I18n._('Never');
                     clean();
                     return language === loadedLang && result === alias;
                 }
@@ -259,7 +259,7 @@ describe('I18n', function () {
         });
 
         it('should default to en', () => {
-            var clean = globalThis.cleanup('', {url: 'https://privatebin.net/'});
+            const clean = globalThis.cleanup('', {url: 'https://privatebin.net/'});
 
             // when navigator.userLanguage is undefined and no default language
             // is specified, it would throw an error
@@ -274,11 +274,27 @@ describe('I18n', function () {
 
             PrivateBin.I18n.reset('en');
             PrivateBin.I18n.loadTranslations();
-            var result = PrivateBin.I18n.translate('Never'),
-                alias  = PrivateBin.I18n._('Never');
+            const result = PrivateBin.I18n.translate('Never'),
+                  alias  = PrivateBin.I18n._('Never');
 
             clean();
             return 'Never' === result && 'Never' === alias;
         });
     });
+
+    describe('onLanguageLoaded', function () {
+        before(function () {
+            PrivateBin.I18n.reset();
+        });
+
+        it('invokes the callback when the languageLoaded event is dispatched', function () {
+            let called = false;
+            PrivateBin.I18n.onLanguageLoaded(function () {
+                called = true;
+            });
+            assert.strictEqual(called, false);
+            document.dispatchEvent(new CustomEvent('languageLoaded'));
+            assert.strictEqual(called, true);
+        });
+    });
 });

+ 1 - 1
lib/Configuration.php

@@ -124,7 +124,7 @@ class Configuration
             'js/kjua-0.10.0.js'      => 'sha512-BYj4xggowR7QD150VLSTRlzH62YPfhpIM+b/1EUEr7RQpdWAGKulxWnOvjFx1FUlba4m6ihpNYuQab51H6XlYg==',
             'js/legacy.js'           => 'sha512-pRofxsrf5UItjiP22Dcjh3FAcBjF/n7h8U9/W5xqJk17U0N2U1oajhXypq/omo9jhwS1iVGOhWrRepoPeFns+w==',
             'js/prettify.js'         => 'sha512-puO0Ogy++IoA2Pb9IjSxV1n4+kQkKXYAEUtVzfZpQepyDPyXk8hokiYDS7ybMogYlyyEIwMLpZqVhCkARQWLMg==',
-            'js/privatebin.js'       => 'sha512-EDBvid7ZFsTiqmEbYUR2Bwo7ypn7GKf+JwW6VFvdE6qLQbzdKrAla+AKhONnt/Tve3zEPc9bXI+4hUHl5itdZw==',
+            'js/privatebin.js'       => 'sha512-3zy2f0oCXDWD3oeXQlIvtHZ1HS9yV1JIPrCbXgMFafJoAPf7+zFVvTWffc5eK2OC5CQOkOJFFaoTwGqMkjximQ==',
             'js/purify-3.4.12.js'    => 'sha512-Akf6HnAJZm0sWWWI4gp2GYff0NDnHUB02XJE5S7Hdq/Z5xtMjkuFacsDA8ZtViv1gi+onBxMhEMIaGyQeGxBng==',
             'js/showdown-2.1.0.js'   => 'sha512-WYXZgkTR0u/Y9SVIA4nTTOih0kXMEd8RRV6MLFdL6YU8ymhR528NLlYQt1nlJQbYz4EW+ZsS0fx1awhiQJme1Q==',
             'js/zlib-1.3.2.js'       => 'sha512-RAhJgxg9siMIA8ky4c10Rc2zUgnK80olHB8Tt1IOYWY4Eh1WmrviQkDn+sgBlb38ZHq3tzufGC41kP360gmosQ==',