Fix scrolling to annotations in un-rendered pages

When scrolling to an anchor in a page that has not been rendered by
PDF.js, the anchor actually references a placeholder element in the
middle of the page. In order to scroll to the correct location in the
page scrolling for such anchors needs to happen in three phases:

 1. Scroll to the approximate location of the final anchor, given by the
    placeholder anchor. This will trigger PDF.js to re-render the target
    page.
 2. Wait for PDF.js to finish re-rendering the page and for the client
    to finish re-anchoring the annotation.
 3. Scroll to the real/non-placeholder anchor.

Change `PDFIntegration#scrollToAnchor` to implement the above steps.

Fixes https://github.com/hypothesis/client/issues/3269

src/annotator/integrations/pdf.js

 import debounce from 'lodash.debounce';
 import { render } from 'preact';
-import scrollIntoView from 'scroll-into-view';
 
 import {
   RenderingStates,
@@ -8,15 +7,17 @@ import {
   describe,
   documentHasText,
 } from '../anchoring/pdf';
-import { removePlaceholder } from '../anchoring/placeholder';
+import { isInPlaceholder, removePlaceholder } from '../anchoring/placeholder';
 import WarningBanner from '../components/WarningBanner';
 import { createShadowRoot } from '../util/shadow-root';
 import { ListenerCollection } from '../util/listener-collection';
+import { offsetRelativeTo, scrollElement } from '../util/scroll';
 
 import { PDFMetadata } from './pdf-metadata';
 
 /**
  * @typedef {import('../../types/annotator').Anchor} Anchor
+ * @typedef {import('../../types/annotator').AnnotationData} AnnotationData
  * @typedef {import('../../types/annotator').Annotator} Annotator
  * @typedef {import('../../types/annotator').HypothesisWindow} HypothesisWindow
  * @typedef {import('../../types/annotator').Integration} Integration
@@ -29,6 +30,21 @@ import { PDFMetadata } from './pdf-metadata';
 // is enough room. Otherwise, allow sidebar to overlap PDF
 const MIN_PDF_WIDTH = 680;
 
+/**
+ * Return true if `anchor` is in an un-rendered page.
+ *
+ * @param {Anchor} anchor
+ */
+function anchorIsPlaceholder(anchor) {
+  const highlight = anchor.highlights?.[0];
+  return highlight && isInPlaceholder(highlight);
+}
+
+/** @param {number} ms */
+function delay(ms) {
+  return new Promise(resolve => setTimeout(resolve, ms));
+}
+
 /**
  * Integration that works with PDF.js
  *
@@ -37,8 +53,11 @@ const MIN_PDF_WIDTH = 680;
 export class PDFIntegration {
   /**
    * @param {Annotator} annotator
+   * @param {object} options
+   *   @param {number} [options.reanchoringWait] - Max time to wait for
+   *     re-anchoring to complete when scrolling to an un-rendered page.
    */
-  constructor(annotator) {
+  constructor(annotator, options = {}) {
     this.annotator = annotator;
 
     const window_ = /** @type {HypothesisWindow} */ (window);
@@ -59,6 +78,12 @@ export class PDFIntegration {
       subtree: true,
     });
 
+    /**
+     * Amount of time to wait for re-anchoring to complete when scrolling to
+     * an anchor in a not-yet-rendered page.
+     */
+    this._reanchoringWait = options.reanchoringWait ?? 3000;
+
     /**
      * A banner shown at the top of the PDF viewer warning the user if the PDF
      * is not suitable for use with Hypothesis.
@@ -309,14 +334,78 @@ export class PDFIntegration {
   }
 
   /**
+   * Scroll to the location of an anchor in the PDF.
+   *
+   * If the anchor refers to a location that is an un-rendered page far from
+   * the viewport, then scrolling happens in three phases. First the document
+   * scrolls to the approximate location indicated by the placeholder anchor,
+   * then `scrollToAnchor` waits until the page's text layer is rendered and
+   * the annotation is re-anchored in the fully rendered page. Then it scrolls
+   * again to the final location.
+   *
    * @param {Anchor} anchor
    */
-  scrollToAnchor(anchor) {
-    // TODO - Account for pages changing state between un-rendered and rendered
-    // while scrolling.
-    const highlights = /** @type {Element[]} */ (anchor.highlights);
-    return new Promise(resolve => {
-      scrollIntoView(highlights[0], resolve);
-    });
+  async scrollToAnchor(anchor) {
+    const annotation = anchor.annotation;
+    const isPlaceholder = anchorIsPlaceholder(anchor);
+    const offset = this._anchorOffset(anchor);
+    if (offset === null) {
+      return;
+    }
+
+    await scrollElement(this.contentContainer(), offset);
+
+    if (isPlaceholder) {
+      const anchor = await this._waitForAnnotationToBeAnchored(
+        annotation,
+        this._reanchoringWait
+      );
+      if (!anchor) {
+        return;
+      }
+      const offset = this._anchorOffset(anchor);
+      if (offset === null) {
+        return;
+      }
+      await scrollElement(this.contentContainer(), offset);
+    }
+  }
+
+  /**
+   * Wait for an annotation to be anchored in a rendered page.
+   *
+   * @param {AnnotationData} annotation
+   * @param {number} maxWait
+   * @return {Promise<Anchor|null>}
+   */
+  async _waitForAnnotationToBeAnchored(annotation, maxWait) {
+    const start = Date.now();
+    let anchor;
+    do {
+      // nb. Re-anchoring might result in a different anchor object for the
+      // same annotation.
+      anchor = this.annotator.anchors.find(a => a.annotation === annotation);
+      if (!anchor || anchorIsPlaceholder(anchor)) {
+        anchor = null;
+        await delay(20);
+      }
+    } while (!anchor && Date.now() - start < maxWait);
+    return anchor ?? null;
+  }
+
+  /**
+   * Return the offset that the PDF content container would need to be scrolled
+   * to, in order to make an anchor visible.
+   *
+   * @param {Anchor} anchor
+   * @return {number|null} - Target offset or `null` if this anchor was not resolved
+   */
+  _anchorOffset(anchor) {
+    if (!anchor.highlights) {
+      // This anchor was not resolved to a location in the document.
+      return null;
+    }
+    const highlight = anchor.highlights[0];
+    return offsetRelativeTo(highlight, this.contentContainer());
   }
 }

src/annotator/integrations/test/pdf-test.js

@@ -2,6 +2,7 @@ import { PDFIntegration, $imports } from '../pdf';
 
 import FakePDFViewerApplication from '../../anchoring/test/fake-pdf-viewer-application';
 import { RenderingStates } from '../../anchoring/pdf';
+import { createPlaceholder } from '../../anchoring/placeholder';
 
 function delay(ms) {
   return new Promise(resolve => setTimeout(resolve, ms));
@@ -26,11 +27,11 @@ describe('PDFIntegration', () => {
   let fakePDFAnchoring;
   let fakePDFMetadata;
   let fakePDFViewerApplication;
-  let fakeScrollIntoView;
+  let fakeScrollUtils;
   let pdfIntegration;
 
-  function createPDFIntegration() {
-    return new PDFIntegration(fakeAnnotator);
+  function createPDFIntegration(options = {}) {
+    return new PDFIntegration(fakeAnnotator, options);
   }
 
   beforeEach(() => {
@@ -62,8 +63,6 @@ describe('PDFIntegration', () => {
       documentHasText: sinon.stub().resolves(true),
     };
 
-    fakeScrollIntoView = sinon.stub().yields();
-
     fakePDFMetadata = {
       getMetadata: sinon
         .stub()
@@ -71,10 +70,15 @@ describe('PDFIntegration', () => {
       getUri: sinon.stub().resolves('https://example.com/test.pdf'),
     };
 
+    fakeScrollUtils = {
+      offsetRelativeTo: sinon.stub().returns(0),
+      scrollElement: sinon.stub().resolves(),
+    };
+
     $imports.$mock({
-      'scroll-into-view': fakeScrollIntoView,
       './pdf-metadata': { PDFMetadata: sinon.stub().returns(fakePDFMetadata) },
       '../anchoring/pdf': fakePDFAnchoring,
+      '../util/scroll': fakeScrollUtils,
 
       // Disable debouncing of updates.
       'lodash.debounce': callback => callback,
@@ -369,15 +373,119 @@ describe('PDFIntegration', () => {
   describe('#scrollToAnchor', () => {
     it('scrolls to first highlight of anchor', async () => {
       const highlight = document.createElement('div');
-      document.body.appendChild(highlight);
+      const offset = 42;
+      const integration = createPDFIntegration();
+      fakeScrollUtils.offsetRelativeTo
+        .withArgs(highlight, integration.contentContainer())
+        .returns(offset);
 
       const anchor = { highlights: [highlight] };
+      await integration.scrollToAnchor(anchor);
 
+      assert.calledOnce(fakeScrollUtils.scrollElement);
+      assert.calledWith(
+        fakeScrollUtils.scrollElement,
+        integration.contentContainer(),
+        offset
+      );
+    });
+
+    it('does not scroll if anchor has no highlights', async () => {
       const integration = createPDFIntegration();
+      const anchor = {};
+
       await integration.scrollToAnchor(anchor);
 
-      assert.calledOnce(fakeScrollIntoView);
-      assert.calledWith(fakeScrollIntoView, highlight, sinon.match.func);
+      assert.notCalled(fakeScrollUtils.scrollElement);
+    });
+
+    /**
+     * Create an anchor whose highlight is inside a placeholder for a non-rendered
+     * PDF page.
+     */
+    function createPlaceholderHighlight() {
+      const container = document.createElement('div');
+      const placeholder = createPlaceholder(container);
+      const highlight = document.createElement('div');
+      placeholder.append(highlight);
+      return highlight;
+    }
+
+    it('waits for anchors in placeholders to be re-anchored and scrolls to final highlight', async () => {
+      const placeholderHighlight = createPlaceholderHighlight();
+      const integration = createPDFIntegration();
+      fakeScrollUtils.offsetRelativeTo
+        .withArgs(placeholderHighlight, integration.contentContainer())
+        .returns(50);
+      const annotation = { $tag: 'tag1' };
+      const anchor = { annotation, highlights: [placeholderHighlight] };
+
+      // Check that the PDF content was scrolled to the approximate position of
+      // the anchor, indicated by the placeholder.
+      const scrollDone = integration.scrollToAnchor(anchor);
+      assert.calledWith(
+        fakeScrollUtils.scrollElement,
+        integration.contentContainer(),
+        50
+      );
+
+      // Simulate a delay while rendering of the text layer for the page happens
+      // and re-anchoring completes.
+      await delay(5);
+
+      // Create a new anchor for the annotation created by re-anchoring.
+      const finalHighlight = document.createElement('div');
+      fakeScrollUtils.scrollElement.resetHistory();
+      fakeAnnotator.anchors.push({
+        annotation,
+        highlights: [finalHighlight],
+      });
+      fakeScrollUtils.offsetRelativeTo
+        .withArgs(finalHighlight, integration.contentContainer())
+        .returns(150);
+
+      await scrollDone;
+
+      // Check that we scrolled to the location of the final highlight.
+      assert.calledWith(
+        fakeScrollUtils.scrollElement,
+        integration.contentContainer(),
+        150
+      );
+    });
+
+    it('skips scrolling to final anchor if re-anchoring does not complete within timeout', async () => {
+      const highlight = createPlaceholderHighlight();
+      const integration = createPDFIntegration({ reanchoringWait: 10 });
+      const annotation = { $tag: 'tag1' };
+      const anchor = { annotation, highlights: [highlight] };
+
+      const scrollDone = integration.scrollToAnchor(anchor);
+      await delay(5); // Simulate delay in re-anchoring
+      fakeScrollUtils.scrollElement.resetHistory();
+
+      // Wait until the re-anchoring timeout expires.
+      await scrollDone;
+
+      assert.notCalled(fakeScrollUtils.scrollElement);
+    });
+
+    it('skips scrolling to final anchor if re-anchoring fails', async () => {
+      const placeholderHighlight = createPlaceholderHighlight();
+      const integration = createPDFIntegration();
+      const annotation = { $tag: 'tag1' };
+      const anchor = { annotation, highlights: [placeholderHighlight] };
+
+      const scrollDone = integration.scrollToAnchor(anchor);
+      await delay(5);
+      fakeScrollUtils.scrollElement.resetHistory();
+
+      // Simulate re-anchoring failing (anchor has no `highlights` field). The
+      // PDF should remain scrolled to the location of the placeholder highlight.
+      fakeAnnotator.anchors.push({ annotation });
+      await scrollDone;
+
+      assert.notCalled(fakeScrollUtils.scrollElement);
     });
   });
 });