From d0bf505312c7fd00024d2f55e094c519a0bb86b0 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Fri, 26 May 2023 10:31:54 +0200 Subject: [PATCH] Re-factor the `isPageVisible`-handling in the find-controller (PR 10217 follow-up) The way that this was implemented in PR 10217 has always bothered me slightly, since the `isPageVisible`-method that I introduced there always felt quite out-of-place in the `IPDFLinkService`-implementations. Hence this is instead replaced by a callback-function in `PDFFindController`, to handle the page-visibility checks. Note that since the `PDFViewer`-constructor always sets this callback-function, e.g. the viewer-component examples still work as-is. --- web/interfaces.js | 5 ----- web/pdf_find_controller.js | 15 +++++++++------ web/pdf_link_service.js | 14 -------------- web/pdf_viewer.js | 25 +++++-------------------- 4 files changed, 14 insertions(+), 45 deletions(-) diff --git a/web/interfaces.js b/web/interfaces.js index 396ea465e..24ad4b647 100644 --- a/web/interfaces.js +++ b/web/interfaces.js @@ -113,11 +113,6 @@ class IPDFLinkService { */ cachePageRef(pageNum, pageRef) {} - /** - * @param {number} pageNumber - */ - isPageVisible(pageNumber) {} - /** * @param {number} pageNumber */ diff --git a/web/pdf_find_controller.js b/web/pdf_find_controller.js index d070e9c91..201e71612 100644 --- a/web/pdf_find_controller.js +++ b/web/pdf_find_controller.js @@ -396,6 +396,12 @@ class PDFFindController { this._eventBus = eventBus; this.#updateMatchesCountOnProgress = updateMatchesCountOnProgress; + /** + * Callback used to check if a `pageNumber` is currently visible. + * @type {function} + */ + this.onIsPageVisible = null; + this.#reset(); eventBus._on("find", this.#onFind.bind(this)); eventBus._on("findbarclose", this.#onFindBarClose.bind(this)); @@ -636,15 +642,12 @@ class PDFFindController { // there's a risk that consecutive 'findagain' operations could "skip" // over matches at the top/bottom of pages thus making them completely // inaccessible when there's multiple pages visible in the viewer. - if ( + return ( pageNumber >= 1 && pageNumber <= linkService.pagesCount && pageNumber !== linkService.page && - !linkService.isPageVisible(pageNumber) - ) { - return true; - } - return false; + !(this.onIsPageVisible?.(pageNumber) ?? true) + ); case "highlightallchange": return false; } diff --git a/web/pdf_link_service.js b/web/pdf_link_service.js index 5c688454b..8c2740a12 100644 --- a/web/pdf_link_service.js +++ b/web/pdf_link_service.js @@ -569,13 +569,6 @@ class PDFLinkService { return this.#pagesRefCache.get(refStr) || null; } - /** - * @param {number} pageNumber - */ - isPageVisible(pageNumber) { - return this.pdfViewer.isPageVisible(pageNumber); - } - /** * @param {number} pageNumber */ @@ -745,13 +738,6 @@ class SimpleLinkService { */ cachePageRef(pageNum, pageRef) {} - /** - * @param {number} pageNumber - */ - isPageVisible(pageNumber) { - return true; - } - /** * @param {number} pageNumber */ diff --git a/web/pdf_viewer.js b/web/pdf_viewer.js index 143f6bd6d..499d5b5da 100644 --- a/web/pdf_viewer.js +++ b/web/pdf_viewer.js @@ -259,6 +259,11 @@ class PDFViewer { this.linkService = options.linkService || new SimpleLinkService(); this.downloadManager = options.downloadManager || null; this.findController = options.findController || null; + + if (this.findController) { + this.findController.onIsPageVisible = pageNumber => + this._getVisiblePages().ids.has(pageNumber); + } this._scriptingManager = options.scriptingManager || null; this.#textLayerMode = options.textLayerMode ?? TextLayerMode.ENABLE; this.#annotationMode = @@ -1640,26 +1645,6 @@ class PDFViewer { }); } - /** - * @param {number} pageNumber - */ - isPageVisible(pageNumber) { - if (!this.pdfDocument) { - return false; - } - if ( - !( - Number.isInteger(pageNumber) && - pageNumber > 0 && - pageNumber <= this.pagesCount - ) - ) { - console.error(`isPageVisible: "${pageNumber}" is not a valid page.`); - return false; - } - return this._getVisiblePages().ids.has(pageNumber); - } - /** * @param {number} pageNumber */