From 655ef12b14d0f50811548d2fe401ef79cd18dac3 Mon Sep 17 00:00:00 2001
From: Jonas Jenwald <jonas.jenwald@gmail.com>
Date: Tue, 1 Oct 2024 13:10:41 +0200
Subject: [PATCH] Re-factor how `PDFLayerViewer` decides if the UI needs to
 updated on "optionalcontentconfigchanged" events

The current implementation won't work correctly in some cases, e.g. if RBGroups are present, which means that it's possible for the UI to get out-of-sync with the actual optionalContent-state.

To avoid querying the DOM unnecessarily we cache the last known UI-state and compare with the actual optionalContent-state, which thus replaces the previously used hash-comparison.
---
 web/pdf_layer_viewer.js | 39 ++++++++++++++++++++++++++++-----------
 1 file changed, 28 insertions(+), 11 deletions(-)

diff --git a/web/pdf_layer_viewer.js b/web/pdf_layer_viewer.js
index 381030f38bf39..c5759a6b9f565 100644
--- a/web/pdf_layer_viewer.js
+++ b/web/pdf_layer_viewer.js
@@ -50,7 +50,9 @@ class PDFLayerViewer extends BaseTreeViewer {
   reset() {
     super.reset();
     this._optionalContentConfig = null;
-    this._optionalContentHash = null;
+
+    this._optionalContentVisibility?.clear();
+    this._optionalContentVisibility = null;
   }
 
   /**
@@ -68,8 +70,13 @@ class PDFLayerViewer extends BaseTreeViewer {
    */
   _bindLink(element, { groupId, input }) {
     const setVisibility = () => {
-      this._optionalContentConfig.setVisibility(groupId, input.checked);
-      this._optionalContentHash = this._optionalContentConfig.getHash();
+      const visible = input.checked;
+      this._optionalContentConfig.setVisibility(groupId, visible);
+
+      const cached = this._optionalContentVisibility.get(groupId);
+      if (cached) {
+        cached.visible = visible;
+      }
 
       this.eventBus.dispatch("optionalcontentconfig", {
         source: this,
@@ -137,7 +144,7 @@ class PDFLayerViewer extends BaseTreeViewer {
       this._dispatchEvent(/* layersCount = */ 0);
       return;
     }
-    this._optionalContentHash = optionalContentConfig.getHash();
+    this._optionalContentVisibility = new Map();
 
     const fragment = document.createDocumentFragment(),
       queue = [{ parent: fragment, groups }];
@@ -170,6 +177,11 @@ class PDFLayerViewer extends BaseTreeViewer {
           input.type = "checkbox";
           input.checked = group.visible;
 
+          this._optionalContentVisibility.set(groupId, {
+            input,
+            visible: input.checked,
+          });
+
           const label = document.createElement("label");
           label.textContent = this._normalizeTextContent(group.name);
 
@@ -197,15 +209,20 @@ class PDFLayerViewer extends BaseTreeViewer {
       return; // The document was closed while the optional content resolved.
     }
     if (promise) {
-      if (optionalContentConfig.getHash() === this._optionalContentHash) {
-        return; // The optional content didn't change, hence no need to reset the UI.
+      // Ensure that the UI displays the correct state (e.g. with RBGroups).
+      for (const [groupId, cached] of this._optionalContentVisibility) {
+        const group = optionalContentConfig.getGroup(groupId);
+
+        if (group && cached.visible !== group.visible) {
+          cached.input.checked = cached.visible = !cached.visible;
+        }
       }
-    } else {
-      this.eventBus.dispatch("optionalcontentconfig", {
-        source: this,
-        promise: Promise.resolve(optionalContentConfig),
-      });
+      return;
     }
+    this.eventBus.dispatch("optionalcontentconfig", {
+      source: this,
+      promise: Promise.resolve(optionalContentConfig),
+    });
 
     // Reset the sidebarView to the new state.
     this.render({