From 942745385506ec1e02fdc9558ec8002f31a0d9bf Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro.celaya@hypothes.is>
Date: Thu, 12 Sep 2024 13:49:13 +0200
Subject: [PATCH] Fix annotations highlights not rendered in Firefox 130

---
 src/annotator/anchoring/pdf.ts           | 25 ++++++++++++++++--
 src/annotator/anchoring/test/pdf-test.js | 32 ++++++++++++++++++++++++
 src/annotator/integrations/pdf.tsx       |  3 ++-
 src/types/pdfjs.ts                       |  6 ++++-
 4 files changed, 62 insertions(+), 4 deletions(-)

diff --git a/src/annotator/anchoring/pdf.ts b/src/annotator/anchoring/pdf.ts
index ceb8cc5f881..1fdd70971dd 100644
--- a/src/annotator/anchoring/pdf.ts
+++ b/src/annotator/anchoring/pdf.ts
@@ -6,7 +6,7 @@ import type {
   TextQuoteSelector,
   Selector,
 } from '../../types/api';
-import type { PDFPageView, PDFViewer } from '../../types/pdfjs';
+import type { PDFPageView, PDFViewer, TextLayer } from '../../types/pdfjs';
 import { translateOffsets } from '../util/normalize';
 import { matchQuote } from './match-quote';
 import { createPlaceholder } from './placeholder';
@@ -259,6 +259,27 @@ function isSpace(char: string) {
 
 const isNotSpace = (char: string) => !isSpace(char);
 
+/**
+ * Determines if provided text layer is done rendering.
+ * It works on older PDF.js versions which expose a public renderingDone prop,
+ * and newer versions as well
+ */
+export function isTextLayerRenderingDone(textLayer: TextLayer): boolean {
+  if (textLayer.renderingDone !== undefined) {
+    return textLayer.renderingDone;
+  }
+
+  if (!textLayer.div) {
+    return false;
+  }
+
+  // When a Page is rendered, the div gets an element with the class
+  // endOfContent appended to it. If that element exists, we can consider the
+  // text layer is done rendering.
+  // See https://github.com/mozilla/pdf.js/blob/1ab9ab67eed886f27127bd801bc349949af5054e/web/text_layer_builder.js#L103-L107
+  return textLayer.div.querySelector('.endOfContent') !== null;
+}
+
 /**
  * Locate the DOM Range which a position selector refers to.
  *
@@ -285,7 +306,7 @@ async function anchorByPosition(
   if (
     page.renderingState === RenderingStates.FINISHED &&
     page.textLayer &&
-    page.textLayer.renderingDone
+    isTextLayerRenderingDone(page.textLayer)
   ) {
     // The page has been rendered. Locate the position in the text layer.
     //
diff --git a/src/annotator/anchoring/test/pdf-test.js b/src/annotator/anchoring/test/pdf-test.js
index 60fc787202e..7199d6645e1 100644
--- a/src/annotator/anchoring/test/pdf-test.js
+++ b/src/annotator/anchoring/test/pdf-test.js
@@ -2,6 +2,7 @@ import { delay } from '@hypothesis/frontend-testing';
 
 import { matchQuote } from '../match-quote';
 import * as pdfAnchoring from '../pdf';
+import { isTextLayerRenderingDone } from '../pdf';
 import { TextRange } from '../text-range';
 import { FakePDFViewerApplication } from './fake-pdf-viewer-application';
 
@@ -724,4 +725,35 @@ describe('annotator/anchoring/pdf', () => {
       });
     });
   });
+
+  describe('isTextLayerRenderingDone', () => {
+    [true, false].forEach(renderingDone => {
+      it('returns renderingDone if present', () => {
+        assert.equal(
+          isTextLayerRenderingDone({ renderingDone }),
+          renderingDone,
+        );
+      });
+    });
+
+    it('returns false if neither renderingDone nor the div are set', () => {
+      assert.isFalse(isTextLayerRenderingDone({}));
+    });
+
+    [
+      { element: null, expectedResult: false },
+      { element: {}, expectedResult: true },
+    ].forEach(({ element, expectedResult }) => {
+      it('returns true if the div contains an endOfContent element', () => {
+        assert.equal(
+          isTextLayerRenderingDone({
+            div: {
+              querySelector: () => element,
+            },
+          }),
+          expectedResult,
+        );
+      });
+    });
+  });
 });
diff --git a/src/annotator/integrations/pdf.tsx b/src/annotator/integrations/pdf.tsx
index 9fa2c7b248c..9fb65117e6d 100644
--- a/src/annotator/integrations/pdf.tsx
+++ b/src/annotator/integrations/pdf.tsx
@@ -19,6 +19,7 @@ import {
   canDescribe,
   describe,
   documentHasText,
+  isTextLayerRenderingDone,
 } from '../anchoring/pdf';
 import { isInPlaceholder, removePlaceholder } from '../anchoring/placeholder';
 import { TextRange } from '../anchoring/text-range';
@@ -356,7 +357,7 @@ export class PDFIntegration extends TinyEmitter implements Integration {
     const pageCount = this._pdfViewer.pagesCount;
     for (let pageIndex = 0; pageIndex < pageCount; pageIndex++) {
       const page = this._pdfViewer.getPageView(pageIndex);
-      if (!page?.textLayer?.renderingDone) {
+      if (!page?.textLayer || !isTextLayerRenderingDone(page.textLayer)) {
         continue;
       }
 
diff --git a/src/types/pdfjs.ts b/src/types/pdfjs.ts
index bef2b0e11f0..7d63e026bfb 100644
--- a/src/types/pdfjs.ts
+++ b/src/types/pdfjs.ts
@@ -192,7 +192,11 @@ export type PDFViewerApplication = {
 };
 
 export type TextLayer = {
-  renderingDone: boolean;
+  /**
+   * This prop is private in PDF.js >=4.5, so we cannot safely trust it's
+   * publicly exposed
+   */
+  renderingDone?: boolean;
   /**
    * New name for root element of text layer in PDF.js >= v3.2.146
    */