From 221eba29b956d63dbcf63449d75b677d02b2b963 Mon Sep 17 00:00:00 2001
From: Jonas Jenwald <jonas.jenwald@gmail.com>
Date: Tue, 11 Mar 2025 11:16:56 +0100
Subject: [PATCH] Don't close the `secondaryToolbar` when clicking inside it
 (PR 18385 follow-up)

When the DOM structure of the viewer was updated in PR 18385 it caused the `secondaryToolbar` to accidentally start closing when clicking inside of it, since the `secondaryToolbar` now reside *under* the `toolbar` in the DOM.

**Steps to reproduce:**
 - Open the viewer.
 - Open the `secondaryToolbar`.
 - Try to change document rotation at least *twice*.

**Expected behaviour:**
 The document rotation can be changed an arbitrary number of times.

**Actual results:**
 The `secondaryToolbar` closes after changing rotation just once.
---
 test/integration/viewer_spec.mjs | 64 ++++++++++++++++++++++++++++++++
 web/app.js                       | 11 +++---
 2 files changed, 70 insertions(+), 5 deletions(-)

diff --git a/test/integration/viewer_spec.mjs b/test/integration/viewer_spec.mjs
index 8bc58fee8..907c82db6 100644
--- a/test/integration/viewer_spec.mjs
+++ b/test/integration/viewer_spec.mjs
@@ -1096,4 +1096,68 @@ describe("PDF viewer", () => {
       });
     });
   });
+
+  describe("SecondaryToolbar", () => {
+    let pages;
+
+    function normalizeRotation(rotation) {
+      return ((rotation % 360) + 360) % 360;
+    }
+
+    function waitForRotationChanging(page, pagesRotation) {
+      return page.evaluateHandle(
+        rotation => [
+          new Promise(resolve => {
+            const { eventBus } = window.PDFViewerApplication;
+            eventBus.on("rotationchanging", function handler(e) {
+              if (rotation === undefined || e.pagesRotation === rotation) {
+                resolve();
+                eventBus.off("rotationchanging", handler);
+              }
+            });
+          }),
+        ],
+        normalizeRotation(pagesRotation)
+      );
+    }
+
+    beforeAll(async () => {
+      pages = await loadAndWait("issue18694.pdf", ".textLayer .endOfContent");
+    });
+
+    afterAll(async () => {
+      await closePages(pages);
+    });
+
+    it("must check that the SecondaryToolbar doesn't close between rotations", async () => {
+      await Promise.all(
+        pages.map(async ([browserName, page]) => {
+          await page.click("#secondaryToolbarToggleButton");
+          await page.waitForSelector("#secondaryToolbar", { hidden: false });
+
+          for (let i = 1; i <= 4; i++) {
+            const secondaryToolbarIsOpen = await page.evaluate(
+              () => window.PDFViewerApplication.secondaryToolbar.isOpen
+            );
+            expect(secondaryToolbarIsOpen)
+              .withContext(`In ${browserName}`)
+              .toBeTrue();
+
+            const rotation = i * 90;
+            const handle = await waitForRotationChanging(page, rotation);
+
+            await page.click("#pageRotateCw");
+            await awaitPromise(handle);
+
+            const pagesRotation = await page.evaluate(
+              () => window.PDFViewerApplication.pdfViewer.pagesRotation
+            );
+            expect(pagesRotation)
+              .withContext(`In ${browserName}`)
+              .toBe(normalizeRotation(rotation));
+          }
+        })
+      );
+    });
+  });
 });
diff --git a/web/app.js b/web/app.js
index ddb9006a0..29f3029bd 100644
--- a/web/app.js
+++ b/web/app.js
@@ -2683,18 +2683,19 @@ function onWheel(evt) {
   }
 }
 
-function closeSecondaryToolbar(evt) {
+function closeSecondaryToolbar({ target }) {
   if (!this.secondaryToolbar?.isOpen) {
     return;
   }
-  const appConfig = this.appConfig;
+  const { toolbar, secondaryToolbar } = this.appConfig;
   if (
-    this.pdfViewer.containsElement(evt.target) ||
-    (appConfig.toolbar?.container.contains(evt.target) &&
+    this.pdfViewer.containsElement(target) ||
+    (toolbar?.container.contains(target) &&
+      !secondaryToolbar?.toolbar.contains(target) &&
       // TODO: change the `contains` for an equality check when the bug:
       //  https://bugzilla.mozilla.org/show_bug.cgi?id=1921984
       // is fixed.
-      !appConfig.secondaryToolbar?.toggleButton.contains(evt.target))
+      !secondaryToolbar?.toggleButton.contains(target))
   ) {
     this.secondaryToolbar.close();
   }