For repeated 'findagain' operations, attempt to reset the search position if the user has e.g. scrolled in the document (issue 4141)

Currently we'll only attempt to start from the current page when a new search is done, however for 'findagain' operations we'll always continue from the last match position.
This could easily lead to confusing behaviour if the user has scrolled to a completely different part of the document. In an attempt to improve this somewhat, for repeated 'findagain' operations, we'll instead reset the position to the current page when it's *absolutely* certain that the user has scrolled.

Note that this required adding a new `BaseViewer` method, and exposing that through `PDFLinkService`, in order to check if a given page is visible.
In an attempt to avoid issues, in custom implementations of `PDFFindController`, the code checks for the existence of the `PDFLinkService.isPageVisible` method *before* using it.
This commit is contained in:
Jonas Jenwald 2018-10-30 11:08:26 +01:00
parent d7941b4ce7
commit d805d799ff
4 changed files with 51 additions and 0 deletions

View File

@ -878,6 +878,23 @@ class BaseViewer {
throw new Error('Not implemented: _getVisiblePages'); throw new Error('Not implemented: _getVisiblePages');
} }
/**
* @param {number} pageNumber
*/
isPageVisible(pageNumber) {
if (!this.pdfDocument) {
return false;
}
if (this.pageNumber < 1 || pageNumber > this.pagesCount) {
console.error(
`${this._name}.isPageVisible: "${pageNumber}" is out of bounds.`);
return false;
}
return this._getVisiblePages().views.some(function(view) {
return (view.id === pageNumber);
});
}
cleanup() { cleanup() {
for (let i = 0, ii = this._pages.length; i < ii; i++) { for (let i = 0, ii = this._pages.length; i < ii; i++) {
if (this._pages[i] && if (this._pages[i] &&

View File

@ -77,6 +77,11 @@ class IPDFLinkService {
* @param {Object} pageRef - reference to the page. * @param {Object} pageRef - reference to the page.
*/ */
cachePageRef(pageNum, pageRef) {} cachePageRef(pageNum, pageRef) {}
/**
* @param {number} pageNumber
*/
isPageVisible(pageNumber) {}
} }
/** /**

View File

@ -201,6 +201,21 @@ class PDFFindController {
_shouldDirtyMatch(cmd) { _shouldDirtyMatch(cmd) {
switch (cmd) { switch (cmd) {
case 'findagain': case 'findagain':
const pageNumber = this._selected.pageIdx + 1;
const linkService = this._linkService;
// Only treat a 'findagain' event as a new search operation when it's
// *absolutely* certain that the currently selected match is no longer
// visible, e.g. as a result of the user scrolling in the document.
//
// NOTE: If only a simple `this._linkService.page` check was used here,
// 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 (pageNumber >= 1 && pageNumber <= linkService.pagesCount &&
linkService.page !== pageNumber && linkService.isPageVisible &&
!linkService.isPageVisible(pageNumber)) {
break;
}
return false; return false;
} }
return true; return true;

View File

@ -352,6 +352,13 @@ class PDFLinkService {
let refStr = pageRef.num + ' ' + pageRef.gen + ' R'; let refStr = pageRef.num + ' ' + pageRef.gen + ' R';
return (this._pagesRefCache && this._pagesRefCache[refStr]) || null; return (this._pagesRefCache && this._pagesRefCache[refStr]) || null;
} }
/**
* @param {number} pageNumber
*/
isPageVisible(pageNumber) {
return this.pdfViewer.isPageVisible(pageNumber);
}
} }
function isValidExplicitDestination(dest) { function isValidExplicitDestination(dest) {
@ -483,6 +490,13 @@ class SimpleLinkService {
* @param {Object} pageRef - reference to the page. * @param {Object} pageRef - reference to the page.
*/ */
cachePageRef(pageNum, pageRef) {} cachePageRef(pageNum, pageRef) {}
/**
* @param {number} pageNumber
*/
isPageVisible(pageNumber) {
return true;
}
} }
export { export {