Browse Source

refactor: bind the e-mail handlers once and keep the document in state

As suggested in review, the nested sendEmailAndHideModal closure is
replaced by private functions bound once in init, like the other
navigation buttons. showEmailButton and hideEmailButton only set or
clear the document the e-mail is about, and the confirmation dialog
remembers the document it was opened for, so no listeners need to be
added or removed after init and the AbortControllers are gone.

- sendEmail sends directly without expiration, otherwise opens the
confirmation dialog, and ignores clicks on the hidden button
- confirmEmailTimezone sends once per opening of the dialog, in the
timezone of the clicked button, and ignores a dialog of an earlier
document
- formatEmailExpiration rounds and formats the expiration date
- triggerEmailSend builds the subject itself
- the Bootstrap modal is created once in init, instead of on each
opening, and the focus handler is bound once
- the e-mail bindings move into bindEmailEvents, keeping bindEvents
  within the complexity limit, and the orphaned TopNav.init docblock
  moves back to init
El RIDO 5 ngày trước cách đây
mục cha
commit
27fc476753
2 tập tin đã thay đổi với 109 bổ sung và 87 xóa
  1. 108 86
      js/privatebin.js
  2. 1 1
      lib/Configuration.php

+ 108 - 86
js/privatebin.js

@@ -3827,8 +3827,12 @@ window.PrivateBin = (function () {
             retryButton,
             pasteExpiration = null,
             retryButtonCallback,
-            emailLinkListeners = null,
-            emailConfirmListeners = null;
+            emailConfirmModal,
+            emailConfirmBootstrapModal = null,
+            emailConfirmTimezoneCurrent,
+            emailConfirmTimezoneUtc,
+            emailDocument = null,
+            emailConfirmDocument = null;
 
         /**
          * set the expiration on bootstrap templates in dropdown
@@ -4199,10 +4203,10 @@ window.PrivateBin = (function () {
          *
          * @name   TopNav.triggerEmailSend
          * @private
-         * @param {string} subject
          * @param {string} emailBody
          */
-        function triggerEmailSend(subject, emailBody) {
+        function triggerEmailSend(emailBody) {
+            const subject = I18n._('Encrypted note on %s', document.title);
             window.open(
                 `mailto:?subject=${encodeURIComponent(subject)}&body=${encodeURIComponent(emailBody)}`,
                 '_self',
@@ -4211,96 +4215,84 @@ window.PrivateBin = (function () {
         }
 
         /**
-         * removes the event listeners of the e-mail confirmation dialog
+         * formats the expiration date for the e-mail body
          *
-         * @name   TopNav.removeEmailConfirmButtonListeners
+         * The date gets rounded down by at least 30 seconds, to make up for
+         * the delay of the request. We don't use Date.prototype.toUTCString(),
+         * because we would like to avoid GMT.
+         *
+         * @name   TopNav.formatEmailExpiration
          * @private
          * @function
+         * @param  {number} expirationDate - timestamp in milliseconds
+         * @param  {bool} isUtc - use UTC instead of the current timezone
+         * @return {string}
          */
-        function removeEmailConfirmButtonListeners() {
-            if (emailConfirmListeners) {
-                emailConfirmListeners.abort();
-                emailConfirmListeners = null;
+        function formatEmailExpiration(expirationDate, isUtc) {
+            const roundedDate = new Date(expirationDate);
+            roundedDate.setUTCSeconds(roundedDate.getUTCSeconds() - 30);
+            roundedDate.setUTCSeconds(0);
+
+            const localeConfiguration = { dateStyle: 'long', timeStyle: 'long' };
+            if (isUtc) {
+                localeConfiguration.timeZone = 'UTC';
             }
+            return roundedDate.toLocaleString([], localeConfiguration);
         }
 
         /**
-         * removes the event listeners of the e-mail button and its dialog
+         * Send Email with current document (URL), asking to confirm the
+         * timezone of its expiration first, if it has one.
          *
-         * @name   TopNav.removeAllEmailListeners
+         * @name   TopNav.sendEmail
          * @private
          * @function
          */
-        function removeAllEmailListeners() {
-            removeEmailConfirmButtonListeners();
-            if (emailLinkListeners) {
-                emailLinkListeners.abort();
-                emailLinkListeners = null;
+        function sendEmail() {
+            // ignore clicks on the hidden button
+            if (emailDocument === null) {
+                return;
+            }
+
+            if (emailDocument.expirationDate === null) {
+                triggerEmailSend(templateEmailBody(null, emailDocument.isBurnafterreading));
+                return;
+            }
+
+            emailConfirmDocument = emailDocument;
+            if (emailConfirmBootstrapModal) {
+                emailConfirmBootstrapModal.show();
             }
         }
 
         /**
-         * Send Email with current document (URL).
+         * Send the Email of the confirmation dialog, with the expiration in
+         * the chosen timezone.
          *
-         * @name   TopNav.sendEmail
+         * @name   TopNav.confirmEmailTimezone
          * @private
          * @function
-         * @param  {Date|null} expirationDate date of expiration
-         * @param  {bool} isBurnafterreading whether it is burn after reading
+         * @param  {Event} event
          */
-        function sendEmail(expirationDate, isBurnafterreading) {
-            const expirationDateRoundedToSecond = new Date(expirationDate);
+        function confirmEmailTimezone(event) {
+            // ignore a dialog opened for an earlier document
+            if (emailConfirmDocument === null) {
+                return;
+            }
 
-            // round down at least 30 seconds to make up for the delay of request
-            expirationDateRoundedToSecond.setUTCSeconds(
-                expirationDateRoundedToSecond.getUTCSeconds() - 30
+            const emailBody = templateEmailBody(
+                formatEmailExpiration(
+                    emailConfirmDocument.expirationDate,
+                    event.currentTarget === emailConfirmTimezoneUtc
+                ),
+                emailConfirmDocument.isBurnafterreading
             );
-            expirationDateRoundedToSecond.setUTCSeconds(0);
-
-            // reused as-is by both the confirmation-modal and direct-send paths below
-            const emailSubject = I18n._('Encrypted note on %s', document.title);
-
-            const emailconfirmmodal = document.getElementById('emailconfirmmodal');
-            if (expirationDate !== null) {
-                const emailconfirmTimezoneCurrent = emailconfirmmodal.querySelector('#emailconfirm-timezone-current');
-                const emailconfirmTimezoneUtc = emailconfirmmodal.querySelector('#emailconfirm-timezone-utc');
-                let localeConfiguration = { dateStyle: 'long', timeStyle: 'long' };
-                const bootstrap5EmailConfirmModal = typeof bootstrap !== 'undefined' && bootstrap.Tooltip && bootstrap.Tooltip.VERSION ?
-                    new bootstrap.Modal(emailconfirmmodal) : null;
-
-                function sendEmailAndHideModal() {
-                    const emailBody = templateEmailBody(
-                        // we don't use Date.prototype.toUTCString() because we would like to avoid GMT
-                        expirationDateRoundedToSecond.toLocaleString(
-                            [], localeConfiguration
-                        ), isBurnafterreading
-                    );
-                    // only send once per opening of the dialog
-                    removeEmailConfirmButtonListeners();
-                    if (bootstrap5EmailConfirmModal) {
-                        bootstrap5EmailConfirmModal.hide();
-                    }
-                    triggerEmailSend(emailSubject, emailBody);
-                }
-
-                // replace the listeners of a previous opening of the dialog
-                removeEmailConfirmButtonListeners();
-                emailConfirmListeners = new window.AbortController();
-                const signal = emailConfirmListeners.signal;
-                emailconfirmmodal.addEventListener('shown.bs.modal', () => {
-                    emailconfirmTimezoneUtc.focus();
-                }, {signal});
-                emailconfirmTimezoneCurrent.addEventListener('click', sendEmailAndHideModal, {signal});
-                emailconfirmTimezoneUtc.addEventListener('click', () => {
-                    localeConfiguration.timeZone = 'UTC';
-                    sendEmailAndHideModal();
-                }, {signal});
-                if (bootstrap5EmailConfirmModal) {
-                    bootstrap5EmailConfirmModal.show();
-                }
-            } else {
-                triggerEmailSend(emailSubject, templateEmailBody(null, isBurnafterreading));
+            // only send once per opening of the dialog
+            emailConfirmDocument = null;
+            if (emailConfirmBootstrapModal) {
+                emailConfirmBootstrapModal.hide();
             }
+            triggerEmailSend(emailBody);
         }
 
         /**
@@ -4468,22 +4460,21 @@ window.PrivateBin = (function () {
                 return;
             }
 
+            // forget a previous document, even if the expiration can't be calculated
+            emailDocument = null;
+            emailConfirmDocument = null;
             try {
-                // we cache expiration date in closure to avoid inaccurate expiration datetime
+                // we cache the expiration date to avoid an inaccurate expiration datetime
                 const expirationDate = Helper.calculateExpirationDate(
                     new Date(),
                     typeof optionalRemainingTimeInSeconds === 'number' ? optionalRemainingTimeInSeconds : TopNav.getExpiration()
                 );
-                const isBurnafterreading = typeof optionalBurnAfterReading === 'boolean' ?
-                    optionalBurnAfterReading : TopNav.getBurnAfterReading();
-
+                emailDocument = {
+                    expirationDate: expirationDate,
+                    isBurnafterreading: typeof optionalBurnAfterReading === 'boolean' ?
+                        optionalBurnAfterReading : TopNav.getBurnAfterReading()
+                };
                 emailLink.classList.remove('hidden');
-                // replace the listeners of a previous document
-                removeAllEmailListeners();
-                emailLinkListeners = new window.AbortController();
-                emailLink.addEventListener('click', () => {
-                    sendEmail(expirationDate, isBurnafterreading);
-                }, {signal: emailLinkListeners.signal});
             } catch (error) {
                 console.error(error);
                 Alert.showError('Cannot calculate expiration date.');
@@ -4502,7 +4493,8 @@ window.PrivateBin = (function () {
             }
 
             emailLink.classList.add('hidden');
-            removeAllEmailListeners();
+            emailDocument = null;
+            emailConfirmDocument = null;
         };
 
         /**
@@ -4798,13 +4790,26 @@ window.PrivateBin = (function () {
         }
 
         /**
-         * init navigation manager
-         *
-         * preloads DOM elements
+         * bind the event listeners of the e-mail button and its confirmation
+         * dialog
          *
-         * @name   TopNav.init
+         * @name   TopNav.bindEmailEvents
+         * @private
          * @function
          */
+        function bindEmailEvents() {
+            if (emailLink) {
+                emailLink.addEventListener('click', sendEmail);
+            }
+            if (emailConfirmModal) {
+                emailConfirmModal.addEventListener('shown.bs.modal', () => {
+                    emailConfirmTimezoneUtc.focus();
+                });
+                emailConfirmTimezoneCurrent.addEventListener('click', confirmEmailTimezone);
+                emailConfirmTimezoneUtc.addEventListener('click', confirmEmailTimezone);
+            }
+        }
+
         /**
          * bind all navigation event listeners
          *
@@ -4859,6 +4864,7 @@ window.PrivateBin = (function () {
             if (qrCodeLink) {
                 qrCodeLink.addEventListener('click', displayQrCode);
             }
+            bindEmailEvents();
 
             if (Helper.isBootstrap5()) {
                 const pasteExpirationSelect = document.getElementById('pasteExpiration');
@@ -4902,6 +4908,14 @@ window.PrivateBin = (function () {
             }
         }
 
+        /**
+         * init navigation manager
+         *
+         * preloads DOM elements
+         *
+         * @name   TopNav.init
+         * @function
+         */
         me.init = function () {
             attach = document.getElementById('attach');
             burnAfterReading = document.getElementById('burnafterreading');
@@ -4923,6 +4937,14 @@ window.PrivateBin = (function () {
             sendButton = document.getElementById('sendbutton');
             qrCodeLink = document.getElementById('qrcodelink');
             emailLink = document.getElementById('emaillink');
+            emailConfirmModal = document.getElementById('emailconfirmmodal');
+            emailConfirmTimezoneCurrent = document.getElementById('emailconfirm-timezone-current');
+            emailConfirmTimezoneUtc = document.getElementById('emailconfirm-timezone-utc');
+            emailConfirmBootstrapModal = emailConfirmModal && typeof bootstrap !== 'undefined' &&
+                bootstrap.Tooltip && typeof bootstrap.Modal === 'function' ?
+                new bootstrap.Modal(emailConfirmModal) : null;
+            emailDocument = null;
+            emailConfirmDocument = null;
 
             bindEvents();
 

+ 1 - 1
lib/Configuration.php

@@ -119,7 +119,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-F+zXzDQ6KkIZR9WCib25HYIrXSFDgCUBMoye9hIgM/bzmdoRLlLS+LlCQIMzQPktZTMzPYasVDrKYaTsr6vNOw==',
+            'js/privatebin.js'       => 'sha512-OjLGUI7ThGedSf2Fnuex16XhniPlEhEr6s08UJAJsLNIx3trf9hDtulW1LUXs5rqJPZG9B+fwBjq9Z9sqmMIrA==',
             'js/purify-3.4.17.js'    => 'sha512-udF2zPdEyVCAxIGeBbeSGFK1Fj5c4eHGi6NqOSsr8M75lGs39POzgUNxO3SFxOg4cHfAoZJgR2EaXdmDi6v6AQ==',
             'js/showdown-2.1.0.js'   => 'sha512-WYXZgkTR0u/Y9SVIA4nTTOih0kXMEd8RRV6MLFdL6YU8ymhR528NLlYQt1nlJQbYz4EW+ZsS0fx1awhiQJme1Q==',
             'js/zlib-1.3.2.js'       => 'sha512-RAhJgxg9siMIA8ky4c10Rc2zUgnK80olHB8Tt1IOYWY4Eh1WmrviQkDn+sgBlb38ZHq3tzufGC41kP360gmosQ==',