From cc49b65a11fbe3cfc9099377ebe3410713a953c0 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Thu, 31 Dec 2020 13:31:28 +0100 Subject: [PATCH 1/3] Use the `once: true` option, rather than manually removing the "pdf.js.response" event listener in `FirefoxCom.request` When this code was originally added, the `once` option didn't exist yet. --- web/firefoxcom.js | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/web/firefoxcom.js b/web/firefoxcom.js index da61ebb6e..fbe9091e6 100644 --- a/web/firefoxcom.js +++ b/web/firefoxcom.js @@ -63,15 +63,18 @@ const FirefoxCom = (function FirefoxComClosure() { request(action, data, callback) { const request = document.createTextNode(""); if (callback) { - document.addEventListener("pdf.js.response", function listener(event) { - const node = event.target; - const response = event.detail.response; + document.addEventListener( + "pdf.js.response", + event => { + const node = event.target; + const response = event.detail.response; - document.documentElement.removeChild(node); + document.documentElement.removeChild(node); - document.removeEventListener("pdf.js.response", listener); - return callback(response); - }); + return callback(response); + }, + { once: true } + ); } document.documentElement.appendChild(request); From 99b1a62c9787bbc9232963d2cf8c1cee97140db6 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Thu, 31 Dec 2020 13:40:03 +0100 Subject: [PATCH 2/3] Use `remove`, rather than `removeChild`, when removing the temporary `Text` nodes used in `FirefoxCom` This is the "modern" way of removing a node from the DOM, which has the benefit of being a lot shorter and more concise. Also, this patch removes the `return` statement from the "pdf.js.response" event listener, since it's always `undefined`, given that none of the `callback`-functions used here ever return anything (and don't need to either). Generally speaking, returning a value from an event listener isn't normally necessary either. --- web/firefoxcom.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/web/firefoxcom.js b/web/firefoxcom.js index fbe9091e6..f65e9bc7e 100644 --- a/web/firefoxcom.js +++ b/web/firefoxcom.js @@ -48,7 +48,8 @@ const FirefoxCom = (function FirefoxComClosure() { }); request.dispatchEvent(sender); const response = sender.detail.response; - document.documentElement.removeChild(request); + request.remove(); + return response; }, @@ -68,10 +69,9 @@ const FirefoxCom = (function FirefoxComClosure() { event => { const node = event.target; const response = event.detail.response; + node.remove(); - document.documentElement.removeChild(node); - - return callback(response); + callback(response); }, { once: true } ); From 8b3b5424471a44e0784b69b638c9093afd4e031b Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Thu, 31 Dec 2020 14:34:16 +0100 Subject: [PATCH 3/3] Ensure that the "pdf.js.response" event listener, in `FirefoxCom.request`, actually applies to the current `Text` node Given that the event listener is registered on the document, there could in *theory* be more than one of these listeners present at any one time. In practice this doesn't currently happen, since all of the `actions` invoked in [`PdfStreamConverter.jsm`](https://searchfox.org/mozilla-central/rev/bfbacfb6a4efd98247e83d3305e912ca4f7e106a/toolkit/components/pdfjs/content/PdfStreamConverter.jsm#933-952) are *synchronous* methods. However, there's no guarantee that this will always be the case, and it's easy enough to prevent any future issues here by simply registering the "pdf.js.response" event listener on the `Text` node instead. This works since, as can be seen in [`PdfStreamConverter.jsm`](https://searchfox.org/mozilla-central/rev/bfbacfb6a4efd98247e83d3305e912ca4f7e106a/toolkit/components/pdfjs/content/PdfStreamConverter.jsm#919,943), the event is dispatched on the element itself rather than the document. --- web/firefoxcom.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/web/firefoxcom.js b/web/firefoxcom.js index f65e9bc7e..ecb3095d7 100644 --- a/web/firefoxcom.js +++ b/web/firefoxcom.js @@ -64,12 +64,11 @@ const FirefoxCom = (function FirefoxComClosure() { request(action, data, callback) { const request = document.createTextNode(""); if (callback) { - document.addEventListener( + request.addEventListener( "pdf.js.response", event => { - const node = event.target; const response = event.detail.response; - node.remove(); + event.target.remove(); callback(response); },