Commit 94274538 authored by Alejandro Celaya's avatar Alejandro Celaya Committed by Alejandro Celaya

Fix annotations highlights not rendered in Firefox 130

parent 4b60a072
...@@ -6,7 +6,7 @@ import type { ...@@ -6,7 +6,7 @@ import type {
TextQuoteSelector, TextQuoteSelector,
Selector, Selector,
} from '../../types/api'; } from '../../types/api';
import type { PDFPageView, PDFViewer } from '../../types/pdfjs'; import type { PDFPageView, PDFViewer, TextLayer } from '../../types/pdfjs';
import { translateOffsets } from '../util/normalize'; import { translateOffsets } from '../util/normalize';
import { matchQuote } from './match-quote'; import { matchQuote } from './match-quote';
import { createPlaceholder } from './placeholder'; import { createPlaceholder } from './placeholder';
...@@ -259,6 +259,27 @@ function isSpace(char: string) { ...@@ -259,6 +259,27 @@ function isSpace(char: string) {
const isNotSpace = (char: string) => !isSpace(char); 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. * Locate the DOM Range which a position selector refers to.
* *
...@@ -285,7 +306,7 @@ async function anchorByPosition( ...@@ -285,7 +306,7 @@ async function anchorByPosition(
if ( if (
page.renderingState === RenderingStates.FINISHED && page.renderingState === RenderingStates.FINISHED &&
page.textLayer && page.textLayer &&
page.textLayer.renderingDone isTextLayerRenderingDone(page.textLayer)
) { ) {
// The page has been rendered. Locate the position in the text layer. // The page has been rendered. Locate the position in the text layer.
// //
......
...@@ -2,6 +2,7 @@ import { delay } from '@hypothesis/frontend-testing'; ...@@ -2,6 +2,7 @@ import { delay } from '@hypothesis/frontend-testing';
import { matchQuote } from '../match-quote'; import { matchQuote } from '../match-quote';
import * as pdfAnchoring from '../pdf'; import * as pdfAnchoring from '../pdf';
import { isTextLayerRenderingDone } from '../pdf';
import { TextRange } from '../text-range'; import { TextRange } from '../text-range';
import { FakePDFViewerApplication } from './fake-pdf-viewer-application'; import { FakePDFViewerApplication } from './fake-pdf-viewer-application';
...@@ -724,4 +725,35 @@ describe('annotator/anchoring/pdf', () => { ...@@ -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,
);
});
});
});
}); });
...@@ -19,6 +19,7 @@ import { ...@@ -19,6 +19,7 @@ import {
canDescribe, canDescribe,
describe, describe,
documentHasText, documentHasText,
isTextLayerRenderingDone,
} from '../anchoring/pdf'; } from '../anchoring/pdf';
import { isInPlaceholder, removePlaceholder } from '../anchoring/placeholder'; import { isInPlaceholder, removePlaceholder } from '../anchoring/placeholder';
import { TextRange } from '../anchoring/text-range'; import { TextRange } from '../anchoring/text-range';
...@@ -356,7 +357,7 @@ export class PDFIntegration extends TinyEmitter implements Integration { ...@@ -356,7 +357,7 @@ export class PDFIntegration extends TinyEmitter implements Integration {
const pageCount = this._pdfViewer.pagesCount; const pageCount = this._pdfViewer.pagesCount;
for (let pageIndex = 0; pageIndex < pageCount; pageIndex++) { for (let pageIndex = 0; pageIndex < pageCount; pageIndex++) {
const page = this._pdfViewer.getPageView(pageIndex); const page = this._pdfViewer.getPageView(pageIndex);
if (!page?.textLayer?.renderingDone) { if (!page?.textLayer || !isTextLayerRenderingDone(page.textLayer)) {
continue; continue;
} }
......
...@@ -192,7 +192,11 @@ export type PDFViewerApplication = { ...@@ -192,7 +192,11 @@ export type PDFViewerApplication = {
}; };
export type TextLayer = { 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 * New name for root element of text layer in PDF.js >= v3.2.146
*/ */
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment