From 27b21f2558a73125b73cd49fc3b85e705c78f6cd Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Tue, 23 Oct 2018 15:57:10 +0200 Subject: [PATCH 1/2] Add a `_updateAllPages` helper method to `PDFFindController` in order to reduce the amount of event dispatching Given that dispatching the 'updatetextlayermatches' event with `pageIndex = -1` set is now used to target the textLayers of *all* pages, there's no need to send individual events to every single page during `_nextMatch`. Since there can be an arbitrary number of pages in a document, this small/simple optimization seems too easy to ignore. --- web/pdf_find_controller.js | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/web/pdf_find_controller.js b/web/pdf_find_controller.js index 5c19b4435..d21507067 100644 --- a/web/pdf_find_controller.js +++ b/web/pdf_find_controller.js @@ -389,6 +389,13 @@ class PDFFindController { }); } + _updateAllPages() { + this._eventBus.dispatch('updatetextlayermatches', { + source: this, + pageIndex: -1, + }); + } + _nextMatch() { const previous = this._state.findPrevious; const currentPageIndex = this._linkService.page - 1; @@ -407,18 +414,18 @@ class PDFFindController { this._pageMatchesLength = null; this._matchesCountTotal = 0; - for (let i = 0; i < numPages; i++) { - // Wipe out any previously highlighted matches. - this._updatePage(i); + this._updateAllPages(); // Wipe out any previously highlighted matches. + for (let i = 0; i < numPages; i++) { // Start finding the matches as soon as the text is extracted. - if (!(i in this._pendingFindMatches)) { - this._pendingFindMatches[i] = true; - this._extractTextPromises[i].then((pageIdx) => { - delete this._pendingFindMatches[pageIdx]; - this._calculateMatch(pageIdx); - }); + if (this._pendingFindMatches[i] === true) { + continue; } + this._pendingFindMatches[i] = true; + this._extractTextPromises[i].then((pageIdx) => { + delete this._pendingFindMatches[pageIdx]; + this._calculateMatch(pageIdx); + }); } } @@ -557,11 +564,7 @@ class PDFFindController { this._updateUIState(FindState.FOUND); } this._highlightMatches = false; - - this._eventBus.dispatch('updatetextlayermatches', { - source: this, - pageIndex: -1, - }); + this._updateAllPages(); // Wipe out any previously highlighted matches. }); } From 64d75c32bf9a568757a63f192215467c913c366c Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Fri, 26 Oct 2018 11:56:25 +0200 Subject: [PATCH 2/2] Ensure that matches are not scrolled into after the findbar has been closed (PR 10100 follow-up) Despite all highlighted matches being removed in response to the 'findbarclose' event, there's a risk that a match could still be scrolled into view *after* the findbar has been closed[1]. Hence we need to ensure that long running searches, particularily those happening in large and/or slow loading documents[2], are ignored as well. --- [1] The match is hidden, as expected, but the document could still scroll unexpectedly. [2] Large documents loaded with `disableAutoFetch = true` and `disableStream = true` set are particularily susceptible to this issue. --- web/pdf_find_controller.js | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/web/pdf_find_controller.js b/web/pdf_find_controller.js index d21507067..2a90d69d6 100644 --- a/web/pdf_find_controller.js +++ b/web/pdf_find_controller.js @@ -112,10 +112,10 @@ class PDFFindController { this._updateUIState(FindState.PENDING); this._firstPageCapability.promise.then(() => { + // If the document was closed before searching began, or if the search + // operation was relevant for a previously opened document, do nothing. if (!this._pdfDocument || (pdfDocument && this._pdfDocument !== pdfDocument)) { - // If the document was closed before searching began, or if the search - // operation was relevant for a previously opened document, do nothing. return; } this._extractText(); @@ -150,6 +150,7 @@ class PDFFindController { this._offset = { // Where the find algorithm currently is in the document. pageIdx: null, matchIdx: null, + wrapped: false, }; this._extractTextPromises = []; this._pageContents = []; // Stores the text for each page. @@ -409,6 +410,7 @@ class PDFFindController { this._selected.pageIdx = this._selected.matchIdx = -1; this._offset.pageIdx = currentPageIndex; this._offset.matchIdx = null; + this._offset.wrapped = false; this._resumePageIdx = null; this._pageMatches.length = 0; this._pageMatchesLength = null; @@ -552,17 +554,27 @@ class PDFFindController { // matches (from the UI) is async too such that the 'updatetextlayermatches' // events will always be dispatched in the expected order. this._firstPageCapability.promise.then(() => { + // Only update the UI if the document is open, and is the current one. if (!this._pdfDocument || (pdfDocument && this._pdfDocument !== pdfDocument)) { - // Only update the UI if the document is open, and is the current one. return; } + // Ensure that a pending, not yet started, search operation is aborted. if (this._findTimeout) { clearTimeout(this._findTimeout); this._findTimeout = null; - // Avoid the UI being in a pending state if the findbar is re-opened. - this._updateUIState(FindState.FOUND); } + // Abort any long running searches, to avoid a match being scrolled into + // view *after* the findbar has been closed. In this case `this._offset` + // will most likely differ from `this._selected`, hence we also ensure + // that any new search operation will always start with a clean slate. + if (this._resumePageIdx) { + this._resumePageIdx = null; + this._dirtyMatch = true; + } + // Avoid the UI being in a pending state when the findbar is re-opened. + this._updateUIState(FindState.FOUND); + this._highlightMatches = false; this._updateAllPages(); // Wipe out any previously highlighted matches. });