[api-minor] Change PDFFindController to use the "find"-event directly (issue 12731)

Looking at the code, I do have to agree with the point made in issue 12731 about it being unexpected/unhelpful that the `PDFFindController.executeCommand`-method isn't directly usable with the "find"-event.
The reason for it being this way is, as so often, for historical reasons: The `executeCommand`-method was added (just) prior to the introduction of the `EventBus` in the viewer.

Obviously we cannot simply change the existing `PDFFindController.executeCommand`-method, since that'd be a breaking change in code which has existed for over five years.
Initially I figured that we could simply add a new method in `PDFFindController` that'd accept the state from the "find"-event, however after thinking about this and looking through the use-cases in the default viewer I settled on a slightly different approach: Let the `PDFFindController` just listen for the "find"-event (on the `EventBus`-instance) directly instead, which also removes one level of (unneeded) indirection during searching in the default viewer.

For GENERIC builds of the PDF.js library, the old `PDFFindController.executeCommand`-method is still available with a deprecation warning.

examples/components/simpleviewer.js

@@ -77,7 +77,11 @@ eventBus.on("pagesinit", function () {
 
   // We can try searching for things.
   if (SEARCH_FOR) {
-    pdfFindController.executeCommand("find", { query: SEARCH_FOR });
+    if (!pdfFindController._onFind) {
+      pdfFindController.executeCommand("find", { query: SEARCH_FOR });
+    } else {
+      eventBus.dispatch("find", { type: "", query: SEARCH_FOR });
+    }
   }
 });
 

examples/components/singlepageviewer.js

@@ -77,7 +77,11 @@ eventBus.on("pagesinit", function () {
 
   // We can try searching for things.
   if (SEARCH_FOR) {
-    pdfFindController.executeCommand("find", { query: SEARCH_FOR });
+    if (!pdfFindController._onFind) {
+      pdfFindController.executeCommand("find", { query: SEARCH_FOR });
+    } else {
+      eventBus.dispatch("find", { type: "", query: SEARCH_FOR });
+    }
   }
 });
 

test/unit/pdf_find_controller_spec.js

@@ -69,14 +69,27 @@ async function initPdfFindController(filename) {
 function testSearch({
   eventBus,
   pdfFindController,
-  parameters,
+  state,
   matchesPerPage,
   selectedMatch,
   pageMatches = null,
   pageMatchesLength = null,
 }) {
   return new Promise(function (resolve) {
-    pdfFindController.executeCommand("find", parameters);
+    const eventState = Object.assign(
+      Object.create(null),
+      {
+        source: this,
+        type: "",
+        query: null,
+        caseSensitive: false,
+        entireWord: false,
+        phraseSearch: true,
+        findPrevious: false,
+      },
+      state
+    );
+    eventBus.dispatch("find", eventState);
 
     // The `updatefindmatchescount` event is only emitted if the page contains
     // at least one match for the query, so the last non-zero item in the
@@ -142,12 +155,8 @@ describe("pdf_find_controller", function () {
     await testSearch({
       eventBus,
       pdfFindController,
-      parameters: {
+      state: {
         query: "Dynamic",
-        caseSensitive: false,
-        entireWord: false,
-        phraseSearch: true,
-        findPrevious: false,
       },
       matchesPerPage: [11, 5, 0, 3, 0, 0, 0, 1, 1, 1, 0, 3, 4, 4],
       selectedMatch: {
@@ -166,11 +175,8 @@ describe("pdf_find_controller", function () {
     await testSearch({
       eventBus,
       pdfFindController,
-      parameters: {
+      state: {
         query: "conference",
-        caseSensitive: false,
-        entireWord: false,
-        phraseSearch: true,
         findPrevious: true,
       },
       matchesPerPage: [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 5],
@@ -187,12 +193,9 @@ describe("pdf_find_controller", function () {
     await testSearch({
       eventBus,
       pdfFindController,
-      parameters: {
+      state: {
         query: "Dynamic",
         caseSensitive: true,
-        entireWord: false,
-        phraseSearch: true,
-        findPrevious: false,
       },
       matchesPerPage: [3, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 3],
       selectedMatch: {
@@ -210,12 +213,9 @@ describe("pdf_find_controller", function () {
     await testSearch({
       eventBus,
       pdfFindController,
-      parameters: {
+      state: {
         query: "Government",
-        caseSensitive: false,
         entireWord: true,
-        phraseSearch: true,
-        findPrevious: false,
       },
       matchesPerPage: [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0],
       selectedMatch: {
@@ -233,12 +233,9 @@ describe("pdf_find_controller", function () {
     await testSearch({
       eventBus,
       pdfFindController,
-      parameters: {
+      state: {
         query: "alternate solution",
-        caseSensitive: false,
-        entireWord: false,
         phraseSearch: false,
-        findPrevious: false,
       },
       matchesPerPage: [0, 0, 0, 0, 0, 1, 0, 0, 4, 0, 0, 0, 0, 0],
       selectedMatch: {
@@ -256,12 +253,8 @@ describe("pdf_find_controller", function () {
     await testSearch({
       eventBus,
       pdfFindController,
-      parameters: {
+      state: {
         query: "fraction",
-        caseSensitive: false,
-        entireWord: false,
-        phraseSearch: true,
-        findPrevious: false,
       },
       matchesPerPage: [3],
       selectedMatch: {

web/app.js

@@ -1929,7 +1929,6 @@ const PDFViewerApplication = {
     eventBus._on("switchspreadmode", webViewerSwitchSpreadMode);
     eventBus._on("spreadmodechanged", webViewerSpreadModeChanged);
     eventBus._on("documentproperties", webViewerDocumentProperties);
-    eventBus._on("find", webViewerFind);
     eventBus._on("findfromurlhash", webViewerFindFromUrlHash);
     eventBus._on("updatefindmatchescount", webViewerUpdateFindMatchesCount);
     eventBus._on("updatefindcontrolstate", webViewerUpdateFindControlState);
@@ -2025,7 +2024,6 @@ const PDFViewerApplication = {
     eventBus._off("switchspreadmode", webViewerSwitchSpreadMode);
     eventBus._off("spreadmodechanged", webViewerSpreadModeChanged);
     eventBus._off("documentproperties", webViewerDocumentProperties);
-    eventBus._off("find", webViewerFind);
     eventBus._off("findfromurlhash", webViewerFindFromUrlHash);
     eventBus._off("updatefindmatchescount", webViewerUpdateFindMatchesCount);
     eventBus._off("updatefindcontrolstate", webViewerUpdateFindControlState);
@@ -2605,19 +2603,10 @@ function webViewerDocumentProperties() {
   PDFViewerApplication.pdfDocumentProperties.open();
 }
 
-function webViewerFind(evt) {
-  PDFViewerApplication.findController.executeCommand("find" + evt.type, {
-    query: evt.query,
-    phraseSearch: evt.phraseSearch,
-    caseSensitive: evt.caseSensitive,
-    entireWord: evt.entireWord,
-    highlightAll: evt.highlightAll,
-    findPrevious: evt.findPrevious,
-  });
-}
-
 function webViewerFindFromUrlHash(evt) {
-  PDFViewerApplication.findController.executeCommand("find", {
+  PDFViewerApplication.eventBus.dispatch("find", {
+    source: evt.source,
+    type: "",
     query: evt.query,
     phraseSearch: evt.phraseSearch,
     caseSensitive: false,
@@ -2794,6 +2783,8 @@ function webViewerKeyDown(evt) {
   if (PDFViewerApplication.overlayManager.active) {
     return;
   }
+  const { eventBus, pdfViewer } = PDFViewerApplication;
+  const isViewerInPresentationMode = pdfViewer.isInPresentationMode;
 
   let handled = false,
     ensureViewerFocused = false;
@@ -2803,9 +2794,6 @@ function webViewerKeyDown(evt) {
     (evt.shiftKey ? 4 : 0) |
     (evt.metaKey ? 8 : 0);
 
-  const pdfViewer = PDFViewerApplication.pdfViewer;
-  const isViewerInPresentationMode = pdfViewer?.isInPresentationMode;
-
   // First, handle the key bindings that are independent whether an input
   // control is selected or not.
   if (cmd === 1 || cmd === 8 || cmd === 5 || cmd === 12) {
@@ -2819,16 +2807,14 @@ function webViewerKeyDown(evt) {
         break;
       case 71: // g
         if (!PDFViewerApplication.supportsIntegratedFind) {
-          const findState = PDFViewerApplication.findController.state;
+          const { state } = PDFViewerApplication.findController;
-          if (findState) {
+          if (state) {
-            PDFViewerApplication.findController.executeCommand("findagain", {
+            const eventState = Object.assign(Object.create(null), state, {
-              query: findState.query,
+              source: window,
-              phraseSearch: findState.phraseSearch,
+              type: "again",
-              caseSensitive: findState.caseSensitive,
-              entireWord: findState.entireWord,
-              highlightAll: findState.highlightAll,
               findPrevious: cmd === 5 || cmd === 12,
             });
+            eventBus.dispatch("find", eventState);
           }
           handled = true;
         }
@@ -2883,8 +2869,6 @@ function webViewerKeyDown(evt) {
   }
 
   if (typeof PDFJSDev === "undefined" || PDFJSDev.test("GENERIC || CHROME")) {
-    const { eventBus } = PDFViewerApplication;
-
     // CTRL or META without shift
     if (cmd === 1 || cmd === 8) {
       switch (evt.keyCode) {

web/firefoxcom.js

@@ -219,6 +219,8 @@ class MozL10n {
     "findentirewordchange",
     "findbarclose",
   ];
+  const findLen = "find".length;
+
   const handleEvent = function ({ type, detail }) {
     if (!PDFViewerApplication.initialized) {
       return;
@@ -229,7 +231,7 @@ class MozL10n {
     }
     PDFViewerApplication.eventBus.dispatch("find", {
       source: window,
-      type: type.substring("find".length),
+      type: type.substring(findLen),
       query: detail.query,
       phraseSearch: true,
       caseSensitive: !!detail.caseSensitive,

web/pdf_find_bar.js

@@ -89,7 +89,7 @@ class PDFFindBar {
     this.updateUIState();
   }
 
-  dispatchEvent(type, findPrev) {
+  dispatchEvent(type, findPrev = false) {
     this.eventBus.dispatch("find", {
       source: this,
       type,

web/pdf_find_controller.js

@@ -104,7 +104,22 @@ class PDFFindController {
     this._eventBus = eventBus;
 
     this._reset();
+    eventBus._on("find", this._onFind.bind(this));
     eventBus._on("findbarclose", this._onFindBarClose.bind(this));
+
+    if (typeof PDFJSDev === "undefined" || PDFJSDev.test("GENERIC")) {
+      this.executeCommand = (cmd, state) => {
+        console.error(
+          "Deprecated method `PDFFindController.executeCommand` called, " +
+            'please dispatch a "find"-event using the EventBus instead.'
+        );
+
+        const eventState = Object.assign(Object.create(null), state, {
+          type: cmd.substring("find".length),
+        });
+        this._onFind(eventState);
+      };
+    }
   }
 
   get highlightMatches() {
@@ -144,17 +159,21 @@ class PDFFindController {
     this._firstPageCapability.resolve();
   }
 
-  executeCommand(cmd, state) {
+  /**
+   * @private
+   */
+  _onFind(state) {
     if (!state) {
       return;
     }
     const pdfDocument = this._pdfDocument;
+    const { type } = state;
 
-    if (this._state === null || this._shouldDirtyMatch(cmd, state)) {
+    if (this._state === null || this._shouldDirtyMatch(state)) {
       this._dirtyMatch = true;
     }
     this._state = state;
-    if (cmd !== "findhighlightallchange") {
+    if (type !== "highlightallchange") {
       this._updateUIState(FindState.PENDING);
     }
 
@@ -176,7 +195,7 @@ class PDFFindController {
         clearTimeout(this._findTimeout);
         this._findTimeout = null;
       }
-      if (cmd === "find") {
+      if (!type) {
         // Trigger the find action with a small delay to avoid starting the
         // search when the user is still typing (saving resources).
         this._findTimeout = setTimeout(() => {
@@ -187,7 +206,7 @@ class PDFFindController {
         // Immediately trigger searching for non-'find' operations, when the
         // current state needs to be reset and matches re-calculated.
         this._nextMatch();
-      } else if (cmd === "findagain") {
+      } else if (type === "again") {
         this._nextMatch();
 
         // When the findbar was previously closed, and `highlightAll` is set,
@@ -195,7 +214,7 @@ class PDFFindController {
         if (findbarClosed && this._state.highlightAll) {
           this._updateAllPages();
         }
-      } else if (cmd === "findhighlightallchange") {
+      } else if (type === "highlightallchange") {
         // If there was a pending search operation, synchronously trigger a new
         // search *first* to ensure that the correct matches are highlighted.
         if (pendingTimeout) {
@@ -275,14 +294,14 @@ class PDFFindController {
     return this._normalizedQuery;
   }
 
-  _shouldDirtyMatch(cmd, state) {
+  _shouldDirtyMatch(state) {
     // When the search query changes, regardless of the actual search command
     // used, always re-calculate matches to avoid errors (fixes bug 1030622).
     if (state.query !== this._state.query) {
       return true;
     }
-    switch (cmd) {
+    switch (state.type) {
-      case "findagain":
+      case "again":
         const pageNumber = this._selected.pageIdx + 1;
         const linkService = this._linkService;
         // Only treat a 'findagain' event as a new search operation when it's
@@ -302,7 +321,7 @@ class PDFFindController {
           return true;
         }
         return false;
-      case "findhighlightallchange":
+      case "highlightallchange":
         return false;
     }
     return true;
@@ -797,7 +816,7 @@ class PDFFindController {
     });
   }
 
-  _updateUIState(state, previous) {
+  _updateUIState(state, previous = false) {
     this._eventBus.dispatch("updatefindcontrolstate", {
       source: this,
       state,