From a578571f59fa167712ee9717e4149f09b727fd8f Mon Sep 17 00:00:00 2001
From: Jonas Jenwald <jonas.jenwald@gmail.com>
Date: Mon, 26 Dec 2022 15:00:22 +0100
Subject: [PATCH] Only display the page-loadingIcon when rendering is currently
 running

Given that we only render one page at a time, this will lead to only *one* page-loadingIcon being displayed at a time even if multiple pages are visible in the viewer. However, this will make it clearer which page is the currently parsing/rendering one.

To simplify toggling of the page-loadingIcon visibility, the existing `PDFPageView.renderingState` is changed into a getter/setter-pair with the latter also handling the page-loadingIcon state.
An additional benefit of these changes is that the `PDFViewer` no longer needs to handling toggling of page-loadingIcon visibility during rendering, since there can only ever be *one* page rendering.
Finally, this may also simplify future changes w.r.t. page-loadingIcon visibility toggling (using e.g. a show-timeout).
---
 web/pdf_page_view.js | 55 +++++++++++++++++++++-----------------------
 web/pdf_viewer.js    | 18 ---------------
 2 files changed, 26 insertions(+), 47 deletions(-)

diff --git a/web/pdf_page_view.js b/web/pdf_page_view.js
index ad410af4a..f0f2b6f7b 100644
--- a/web/pdf_page_view.js
+++ b/web/pdf_page_view.js
@@ -120,6 +120,8 @@ class PDFPageView {
 
   #previousRotation = null;
 
+  #renderingState = RenderingStates.INITIAL;
+
   #useThumbnailCanvas = {
     initialOptionalContent: true,
     regularAnnotations: true,
@@ -167,7 +169,6 @@ class PDFPageView {
 
     this.paintTask = null;
     this.paintedViewportMap = new WeakMap();
-    this.renderingState = RenderingStates.INITIAL;
     this.resume = null;
     this._renderError = null;
     if (
@@ -227,6 +228,30 @@ class PDFPageView {
     }
   }
 
+  get renderingState() {
+    return this.#renderingState;
+  }
+
+  set renderingState(state) {
+    this.#renderingState = state;
+
+    switch (state) {
+      case RenderingStates.INITIAL:
+      case RenderingStates.PAUSED:
+        this.loadingIconDiv?.classList.add("notVisible");
+        break;
+      case RenderingStates.RUNNING:
+        this.loadingIconDiv?.classList.remove("notVisible");
+        break;
+      case RenderingStates.FINISHED:
+        if (this.loadingIconDiv) {
+          this.loadingIconDiv.remove();
+          delete this.loadingIconDiv;
+        }
+        break;
+    }
+  }
+
   #setDimensions() {
     const { viewport } = this;
     if (this.pdfPage) {
@@ -496,17 +521,6 @@ class PDFPageView {
         this.loadingIconDiv?.setAttribute("aria-label", msg);
       });
       div.append(this.loadingIconDiv);
-    } else {
-      this.toggleLoadingIconSpinner();
-      div.append(this.loadingIconDiv);
-    }
-
-    if (
-      (typeof PDFJSDev === "undefined" ||
-        PDFJSDev.test("!PRODUCTION || GENERIC")) &&
-      this._isStandalone
-    ) {
-      this.toggleLoadingIconSpinner(/* viewVisible = */ true);
     }
   }
 
@@ -775,13 +789,6 @@ class PDFPageView {
     return this.viewport.convertToPdfPoint(x, y);
   }
 
-  /**
-   * @ignore
-   */
-  toggleLoadingIconSpinner(viewVisible = false) {
-    this.loadingIconDiv?.classList.toggle("notVisible", !viewVisible);
-  }
-
   draw() {
     if (this.renderingState !== RenderingStates.INITIAL) {
       console.error("Must be in new state before drawing");
@@ -791,11 +798,6 @@ class PDFPageView {
 
     if (!pdfPage) {
       this.renderingState = RenderingStates.FINISHED;
-
-      if (this.loadingIconDiv) {
-        this.loadingIconDiv.remove();
-        delete this.loadingIconDiv;
-      }
       return Promise.reject(new Error("pdfPage is not loaded"));
     }
 
@@ -888,11 +890,6 @@ class PDFPageView {
       this._renderError = error;
 
       this.renderingState = RenderingStates.FINISHED;
-
-      if (this.loadingIconDiv) {
-        this.loadingIconDiv.remove();
-        delete this.loadingIconDiv;
-      }
       this._resetZoomLayer(/* removeFromDOM = */ true);
 
       // Ensure that the thumbnails won't become partially (or fully) blank,
diff --git a/web/pdf_viewer.js b/web/pdf_viewer.js
index 995b15d83..25caf7c15 100644
--- a/web/pdf_viewer.js
+++ b/web/pdf_viewer.js
@@ -1626,23 +1626,6 @@ class PDFViewer {
     return this.scroll.down;
   }
 
-  /**
-   * Only show the `loadingIcon`-spinner on visible pages (see issue 14242).
-   */
-  #toggleLoadingIconSpinner(visibleIds) {
-    for (const id of visibleIds) {
-      const pageView = this._pages[id - 1];
-      pageView?.toggleLoadingIconSpinner(/* viewVisible = */ true);
-    }
-    for (const pageView of this.#buffer) {
-      if (visibleIds.has(pageView.id)) {
-        // Handled above, since the "buffer" may not contain all visible pages.
-        continue;
-      }
-      pageView.toggleLoadingIconSpinner(/* viewVisible = */ false);
-    }
-  }
-
   forceRendering(currentlyVisiblePages) {
     const visiblePages = currentlyVisiblePages || this._getVisiblePages();
     const scrollAhead = this.#getScrollAhead(visiblePages);
@@ -1656,7 +1639,6 @@ class PDFViewer {
       scrollAhead,
       preRenderExtra
     );
-    this.#toggleLoadingIconSpinner(visiblePages.ids);
 
     if (pageView) {
       this.#ensurePdfPageLoaded(pageView).then(() => {