From d805d799ffad7d27f9938143b67a75020a7d40b3 Mon Sep 17 00:00:00 2001
From: Jonas Jenwald <jonas.jenwald@gmail.com>
Date: Tue, 30 Oct 2018 11:08:26 +0100
Subject: [PATCH] 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.
---
 web/base_viewer.js         | 17 +++++++++++++++++
 web/interfaces.js          |  5 +++++
 web/pdf_find_controller.js | 15 +++++++++++++++
 web/pdf_link_service.js    | 14 ++++++++++++++
 4 files changed, 51 insertions(+)

diff --git a/web/base_viewer.js b/web/base_viewer.js
index 9588d75dc..19b3c344a 100644
--- a/web/base_viewer.js
+++ b/web/base_viewer.js
@@ -878,6 +878,23 @@ class BaseViewer {
     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() {
     for (let i = 0, ii = this._pages.length; i < ii; i++) {
       if (this._pages[i] &&
diff --git a/web/interfaces.js b/web/interfaces.js
index 170622681..55631555e 100644
--- a/web/interfaces.js
+++ b/web/interfaces.js
@@ -77,6 +77,11 @@ class IPDFLinkService {
    * @param {Object} pageRef - reference to the page.
    */
   cachePageRef(pageNum, pageRef) {}
+
+  /**
+   * @param {number} pageNumber
+   */
+  isPageVisible(pageNumber) {}
 }
 
 /**
diff --git a/web/pdf_find_controller.js b/web/pdf_find_controller.js
index 7279a6ae5..3c1785124 100644
--- a/web/pdf_find_controller.js
+++ b/web/pdf_find_controller.js
@@ -201,6 +201,21 @@ class PDFFindController {
   _shouldDirtyMatch(cmd) {
     switch (cmd) {
       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 true;
diff --git a/web/pdf_link_service.js b/web/pdf_link_service.js
index 9fe9427b5..44bd88a34 100644
--- a/web/pdf_link_service.js
+++ b/web/pdf_link_service.js
@@ -352,6 +352,13 @@ class PDFLinkService {
     let refStr = pageRef.num + ' ' + pageRef.gen + ' R';
     return (this._pagesRefCache && this._pagesRefCache[refStr]) || null;
   }
+
+  /**
+   * @param {number} pageNumber
+   */
+  isPageVisible(pageNumber) {
+    return this.pdfViewer.isPageVisible(pageNumber);
+  }
 }
 
 function isValidExplicitDestination(dest) {
@@ -483,6 +490,13 @@ class SimpleLinkService {
    * @param {Object} pageRef - reference to the page.
    */
   cachePageRef(pageNum, pageRef) {}
+
+  /**
+   * @param {number} pageNumber
+   */
+  isPageVisible(pageNumber) {
+    return true;
+  }
 }
 
 export {