Include the state in the "presentationmodechanged" event, and remov…

…e the separate `active`/`switchInProgress` properties Given that we already have a `PresentationModeState`-enumeration, we should use that with the "presentationmodechanged" event rather than including separate properties. Note that this new behaviour, of including an enumeration-value in the event, is consistent with lots of other existing viewer-events. To hopefully avoid issues in custom implementations of the default viewer, any attempt to access the removed properties will now throw.
mozilla · Dec 28, 2020 · f3e31a7 · f3e31a7
1 parent df53e78
commit f3e31a7
Show file tree

Hide file tree

Showing 5 changed files with 55 additions and 31 deletions.
diff --git a/web/app.js b/web/app.js
@@ -29,7 +29,6 @@ import {
   noContextMenuHandler,
   normalizeWheelEventDirection,
   parseQueryString,
-  PresentationModeState,
   ProgressBar,
   RendererType,
   ScrollMode,
@@ -2620,14 +2619,8 @@ function webViewerNamedAction(evt) {
   }
 }
 
-function webViewerPresentationModeChanged({ active, switchInProgress }) {
-  let state = PresentationModeState.NORMAL;
-  if (switchInProgress) {
-    state = PresentationModeState.CHANGING;
-  } else if (active) {
-    state = PresentationModeState.FULLSCREEN;
-  }
-  PDFViewerApplication.pdfViewer.presentationModeState = state;
+function webViewerPresentationModeChanged(evt) {
+  PDFViewerApplication.pdfViewer.presentationModeState = evt.state;
 }
 
 function webViewerSidebarViewChanged(evt) {

diff --git a/web/pdf_cursor_tools.js b/web/pdf_cursor_tools.js
@@ -14,6 +14,7 @@
  */
 
 import { GrabToPan } from "./grab_to_pan.js";
+import { PresentationModeState } from "./ui_utils.js";
 
 const CursorTool = {
   SELECT: 0, // The default value.
@@ -127,21 +128,23 @@ class PDFCursorTools {
     });
 
     this.eventBus._on("presentationmodechanged", evt => {
-      if (evt.switchInProgress) {
-        return;
-      }
-      let previouslyActive;
-
-      if (evt.active) {
-        previouslyActive = this.active;
+      switch (evt.state) {
+        case PresentationModeState.CHANGING:
+          break;
+        case PresentationModeState.FULLSCREEN: {
+          const previouslyActive = this.active;
 
-        this.switchTool(CursorTool.SELECT);
-        this.activeBeforePresentationMode = previouslyActive;
-      } else {
-        previouslyActive = this.activeBeforePresentationMode;
+          this.switchTool(CursorTool.SELECT);
+          this.activeBeforePresentationMode = previouslyActive;
+          break;
+        }
+        case PresentationModeState.NORMAL: {
+          const previouslyActive = this.activeBeforePresentationMode;
 
-        this.activeBeforePresentationMode = null;
-        this.switchTool(previouslyActive);
+          this.activeBeforePresentationMode = null;
+          this.switchTool(previouslyActive);
+          break;
+        }
       }
     });
   }

diff --git a/web/pdf_history.js b/web/pdf_history.js
@@ -16,6 +16,7 @@
 import {
   isValidRotation,
   parseQueryString,
+  PresentationModeState,
   waitOnEventOrTimeout,
 } from "./ui_utils.js";
 
@@ -69,7 +70,8 @@ class PDFHistory {
     // Ensure that we don't miss either a 'presentationmodechanged' or a
     // 'pagesinit' event, by registering the listeners immediately.
     this.eventBus._on("presentationmodechanged", evt => {
-      this._isViewerInPresentationMode = evt.active || evt.switchInProgress;
+      this._isViewerInPresentationMode =
+        evt.state !== PresentationModeState.NORMAL;
     });
     this.eventBus._on("pagesinit", () => {
       this._isPagesLoaded = false;

diff --git a/web/pdf_presentation_mode.js b/web/pdf_presentation_mode.js
@@ -13,7 +13,7 @@
  * limitations under the License.
  */
 
-import { normalizeWheelEventDelta } from "./ui_utils.js";
+import { normalizeWheelEventDelta, PresentationModeState } from "./ui_utils.js";
 
 const DELAY_BEFORE_RESETTING_SWITCH_IN_PROGRESS = 1500; // in ms
 const DELAY_BEFORE_HIDING_CONTROLS = 3000; // in ms
@@ -183,11 +183,34 @@ class PDFPresentationMode {
    * @private
    */
   _notifyStateChange() {
-    this.eventBus.dispatch("presentationmodechanged", {
-      source: this,
-      active: this.active,
-      switchInProgress: !!this.switchInProgress,
-    });
+    let state = PresentationModeState.NORMAL;
+    if (this.switchInProgress) {
+      state = PresentationModeState.CHANGING;
+    } else if (this.active) {
+      state = PresentationModeState.FULLSCREEN;
+    }
+
+    if (typeof PDFJSDev === "undefined" || !PDFJSDev.test("MOZCENTRAL")) {
+      this.eventBus.dispatch("presentationmodechanged", {
+        source: this,
+        state,
+      });
+    } else {
+      this.eventBus.dispatch("presentationmodechanged", {
+        source: this,
+        state,
+        get active() {
+          throw new Error(
+            "Removed parameter: `active`, please use `state` instead."
+          );
+        },
+        get switchInProgress() {
+          throw new Error(
+            "Removed parameter: `switchInProgress`, please use `state` instead."
+          );
+        },
+      });
+    }
   }
 
   /**

diff --git a/web/pdf_sidebar.js b/web/pdf_sidebar.js
@@ -13,7 +13,7 @@
  * limitations under the License.
  */
 
-import { NullL10n } from "./ui_utils.js";
+import { NullL10n, PresentationModeState } from "./ui_utils.js";
 import { RenderingStates } from "./pdf_rendering_queue.js";
 
 const UI_NOTIFICATION_CLASS = "pdfSidebarNotification";
@@ -491,7 +491,10 @@ class PDFSidebar {
 
     // Update the thumbnailViewer, if visible, when exiting presentation mode.
     this.eventBus._on("presentationmodechanged", evt => {
-      if (!evt.active && !evt.switchInProgress && this.isThumbnailViewVisible) {
+      if (
+        evt.state === PresentationModeState.NORMAL &&
+        this.isThumbnailViewVisible
+      ) {
         this._updateThumbnailViewer();
       }
     });