From 984debaa9f2cb5462967c8c85f435dc5ec0ae9f7 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sat, 10 Aug 2024 12:06:45 +0200 Subject: [PATCH 1/3] Use a few local variables in `PDFSidebar.#addEventListeners` This, ever so slightly, shortens the code for a couple of repeatedly accessed class fields. --- web/pdf_sidebar.js | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/web/pdf_sidebar.js b/web/pdf_sidebar.js index 5f3ab94310e05..fa62ca53ef144 100644 --- a/web/pdf_sidebar.js +++ b/web/pdf_sidebar.js @@ -325,11 +325,13 @@ class PDFSidebar { } #addEventListeners() { + const { eventBus, outerContainer } = this; + this.sidebarContainer.addEventListener("transitionend", evt => { if (evt.target === this.sidebarContainer) { - this.outerContainer.classList.remove("sidebarMoving"); + outerContainer.classList.remove("sidebarMoving"); // Ensure that rendering is triggered after opening/closing the sidebar. - this.eventBus.dispatch("resize", { source: this }); + eventBus.dispatch("resize", { source: this }); } }); @@ -346,7 +348,7 @@ class PDFSidebar { this.switchView(SidebarView.OUTLINE); }); this.outlineButton.addEventListener("dblclick", () => { - this.eventBus.dispatch("toggleoutlinetree", { source: this }); + eventBus.dispatch("toggleoutlinetree", { source: this }); }); this.attachmentsButton.addEventListener("click", () => { @@ -357,12 +359,12 @@ class PDFSidebar { this.switchView(SidebarView.LAYERS); }); this.layersButton.addEventListener("dblclick", () => { - this.eventBus.dispatch("resetlayers", { source: this }); + eventBus.dispatch("resetlayers", { source: this }); }); // Buttons for view-specific options. this._currentOutlineItemButton.addEventListener("click", () => { - this.eventBus.dispatch("currentoutlineitem", { source: this }); + eventBus.dispatch("currentoutlineitem", { source: this }); }); // Disable/enable views. @@ -378,7 +380,7 @@ class PDFSidebar { } }; - this.eventBus._on("outlineloaded", evt => { + eventBus._on("outlineloaded", evt => { onTreeLoaded(evt.outlineCount, this.outlineButton, SidebarView.OUTLINE); evt.currentOutlineItemPromise.then(enabled => { @@ -389,7 +391,7 @@ class PDFSidebar { }); }); - this.eventBus._on("attachmentsloaded", evt => { + eventBus._on("attachmentsloaded", evt => { onTreeLoaded( evt.attachmentsCount, this.attachmentsButton, @@ -397,12 +399,12 @@ class PDFSidebar { ); }); - this.eventBus._on("layersloaded", evt => { + eventBus._on("layersloaded", evt => { onTreeLoaded(evt.layersCount, this.layersButton, SidebarView.LAYERS); }); // Update the thumbnailViewer, if visible, when exiting presentation mode. - this.eventBus._on("presentationmodechanged", evt => { + eventBus._on("presentationmodechanged", evt => { if ( evt.state === PresentationModeState.NORMAL && this.visibleView === SidebarView.THUMBS @@ -418,13 +420,13 @@ class PDFSidebar { } // Disable the `transition-duration` rules when sidebar resizing begins, // in order to improve responsiveness and to avoid visual glitches. - this.outerContainer.classList.add(SIDEBAR_RESIZING_CLASS); + outerContainer.classList.add(SIDEBAR_RESIZING_CLASS); window.addEventListener("mousemove", this.#mouseMoveBound); window.addEventListener("mouseup", this.#mouseUpBound); }); - this.eventBus._on("resize", evt => { + eventBus._on("resize", evt => { // When the *entire* viewer is resized, such that it becomes narrower, // ensure that the sidebar doesn't end up being too wide. if (evt.source !== window) { @@ -443,15 +445,15 @@ class PDFSidebar { this.#updateWidth(this.#width); return; } - this.outerContainer.classList.add(SIDEBAR_RESIZING_CLASS); + outerContainer.classList.add(SIDEBAR_RESIZING_CLASS); const updated = this.#updateWidth(this.#width); Promise.resolve().then(() => { - this.outerContainer.classList.remove(SIDEBAR_RESIZING_CLASS); + outerContainer.classList.remove(SIDEBAR_RESIZING_CLASS); // Trigger rendering if the sidebar width changed, to avoid // depending on the order in which 'resize' events are handled. if (updated) { - this.eventBus.dispatch("resize", { source: this }); + eventBus.dispatch("resize", { source: this }); } }); }); From 7619171265f0639b3a09b3b5b5902da009a40dc6 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sat, 10 Aug 2024 12:11:31 +0200 Subject: [PATCH 2/3] Stop sidebar resizing on "blur" events Because of an old oversight (by me) we don't stop sidebar resizing when the browser window loses focus, which seems generally wrong and can also lead to duplicate mouse-related event listeners being registered. --- web/pdf_sidebar.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/web/pdf_sidebar.js b/web/pdf_sidebar.js index fa62ca53ef144..d2b48aa77289d 100644 --- a/web/pdf_sidebar.js +++ b/web/pdf_sidebar.js @@ -424,6 +424,7 @@ class PDFSidebar { window.addEventListener("mousemove", this.#mouseMoveBound); window.addEventListener("mouseup", this.#mouseUpBound); + window.addEventListener("blur", this.#mouseUpBound); }); eventBus._on("resize", evt => { @@ -506,6 +507,7 @@ class PDFSidebar { window.removeEventListener("mousemove", this.#mouseMoveBound); window.removeEventListener("mouseup", this.#mouseUpBound); + window.removeEventListener("blur", this.#mouseUpBound); } } From be685a293c148d8181e89fb433f5ee917ab8bad2 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sat, 10 Aug 2024 12:20:50 +0200 Subject: [PATCH 3/3] Remove the sidebar resizing event listeners with an `AbortController` --- web/pdf_sidebar.js | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/web/pdf_sidebar.js b/web/pdf_sidebar.js index d2b48aa77289d..b85a0372ad2b0 100644 --- a/web/pdf_sidebar.js +++ b/web/pdf_sidebar.js @@ -69,9 +69,7 @@ const UI_NOTIFICATION_CLASS = "pdfSidebarNotification"; class PDFSidebar { #isRTL = false; - #mouseMoveBound = this.#mouseMove.bind(this); - - #mouseUpBound = this.#mouseUp.bind(this); + #mouseAC = null; #outerContainerWidth = null; @@ -422,9 +420,12 @@ class PDFSidebar { // in order to improve responsiveness and to avoid visual glitches. outerContainer.classList.add(SIDEBAR_RESIZING_CLASS); - window.addEventListener("mousemove", this.#mouseMoveBound); - window.addEventListener("mouseup", this.#mouseUpBound); - window.addEventListener("blur", this.#mouseUpBound); + this.#mouseAC = new AbortController(); + const opts = { signal: this.#mouseAC.signal }; + + window.addEventListener("mousemove", this.#mouseMove.bind(this), opts); + window.addEventListener("mouseup", this.#mouseUp.bind(this), opts); + window.addEventListener("blur", this.#mouseUp.bind(this), opts); }); eventBus._on("resize", evt => { @@ -505,9 +506,8 @@ class PDFSidebar { // ... and ensure that rendering will always be triggered. this.eventBus.dispatch("resize", { source: this }); - window.removeEventListener("mousemove", this.#mouseMoveBound); - window.removeEventListener("mouseup", this.#mouseUpBound); - window.removeEventListener("blur", this.#mouseUpBound); + this.#mouseAC?.abort(); + this.#mouseAC = null; } }