From cd22c317522e4e3f319069a7bdc7c96b0d1270fb Mon Sep 17 00:00:00 2001
From: Jonas Jenwald <jonas.jenwald@gmail.com>
Date: Sun, 3 Oct 2021 15:32:33 +0200
Subject: [PATCH 1/2] Fix the remaining `Promise.resolve(undefined)` cases
Many years ago now there were some `Promise` implementations that had issues resolving with an *implicitly* `undefined` value. That should no longer be the case, and we've not been using the `Promise.resolve(undefined)` format for a long time, hence this patch fixes the few remaining cases.
---
web/pdf_page_view.js | 2 +-
web/pdf_thumbnail_view.js | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/web/pdf_page_view.js b/web/pdf_page_view.js
index cac0ae619..aea372e07 100644
--- a/web/pdf_page_view.js
+++ b/web/pdf_page_view.js
@@ -831,7 +831,7 @@ class PDFPageView {
renderTask.promise.then(
function () {
showCanvas();
- renderCapability.resolve(undefined);
+ renderCapability.resolve();
},
function (error) {
showCanvas();
diff --git a/web/pdf_thumbnail_view.js b/web/pdf_thumbnail_view.js
index 74fc2eaeb..4b43d26f1 100644
--- a/web/pdf_thumbnail_view.js
+++ b/web/pdf_thumbnail_view.js
@@ -278,7 +278,7 @@ class PDFThumbnailView {
draw() {
if (this.renderingState !== RenderingStates.INITIAL) {
console.error("Must be in new state before drawing");
- return Promise.resolve(undefined);
+ return Promise.resolve();
}
const { pdfPage } = this;
From fa8c0ef6164c7abfd5236e97823102a89517f8a4 Mon Sep 17 00:00:00 2001
From: Jonas Jenwald <jonas.jenwald@gmail.com>
Date: Sun, 3 Oct 2021 16:03:51 +0200
Subject: [PATCH 2/2] [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 | 6 ++-
examples/components/singlepageviewer.js | 6 ++-
test/unit/pdf_find_controller_spec.js | 49 +++++++++++--------------
web/app.js | 38 ++++++-------------
web/firefoxcom.js | 4 +-
web/pdf_find_bar.js | 2 +-
web/pdf_find_controller.js | 41 +++++++++++++++------
7 files changed, 76 insertions(+), 70 deletions(-)
diff --git a/examples/components/simpleviewer.js b/examples/components/simpleviewer.js
index 942a80a63..f064a53be 100644
--- a/examples/components/simpleviewer.js
+++ b/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 });
+ }
}
});
diff --git a/examples/components/singlepageviewer.js b/examples/components/singlepageviewer.js
index 3eac4c0c8..c22db3003 100644
--- a/examples/components/singlepageviewer.js
+++ b/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 });
+ }
}
});
diff --git a/test/unit/pdf_find_controller_spec.js b/test/unit/pdf_find_controller_spec.js
index 1b97f47e4..e0d6f6a98 100644
--- a/test/unit/pdf_find_controller_spec.js
+++ b/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: {
diff --git a/web/app.js b/web/app.js
index a804bb04a..3c81f5dbe 100644
--- a/web/app.js
+++ b/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;
- if (findState) {
- PDFViewerApplication.findController.executeCommand("findagain", {
- query: findState.query,
- phraseSearch: findState.phraseSearch,
- caseSensitive: findState.caseSensitive,
- entireWord: findState.entireWord,
- highlightAll: findState.highlightAll,
+ const { state } = PDFViewerApplication.findController;
+ if (state) {
+ const eventState = Object.assign(Object.create(null), state, {
+ source: window,
+ type: "again",
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) {
diff --git a/web/firefoxcom.js b/web/firefoxcom.js
index 129190a74..5b3b2a351 100644
--- a/web/firefoxcom.js
+++ b/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,
diff --git a/web/pdf_find_bar.js b/web/pdf_find_bar.js
index 3388711d9..3c0192566 100644
--- a/web/pdf_find_bar.js
+++ b/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,
diff --git a/web/pdf_find_controller.js b/web/pdf_find_controller.js
index 91302efc5..9e36a1631 100644
--- a/web/pdf_find_controller.js
+++ b/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) {
- case "findagain":
+ switch (state.type) {
+ 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,