From 9e02ceed09a649a27e341e316fb5fe45009f2513 Mon Sep 17 00:00:00 2001 From: Calixte Denizet Date: Thu, 30 Jul 2026 16:59:28 +0200 Subject: [PATCH] Fix the selection rendering when a page has been destroyed and rendered again (bug 2054348) The draw layer keeps a reference on the text layer div in order to render the selection, but it was destroyed only along with the annotation editor layer. Hence, when the editor is disabled, like in Firefox for Android, scrolling far enough to destroy a page view left the draw layer with a reference on a removed text layer: the new one was never registered, consequently no selection was rendered anymore on that page. --- src/display/draw_layer.js | 3 +- test/integration/text_layer_spec.mjs | 90 ++++++++++++++++++++++++++++ web/app.js | 1 + web/pdf_page_view.js | 11 +++- 4 files changed, 103 insertions(+), 2 deletions(-) diff --git a/src/display/draw_layer.js b/src/display/draw_layer.js index 3278900ae..99398e82e 100644 --- a/src/display/draw_layer.js +++ b/src/display/draw_layer.js @@ -641,7 +641,8 @@ class DrawLayer { textLayerData.selectionDiv = div; } - if (!div.parentNode && drawLayer.#parent) { + if (drawLayer.#parent && div.parentNode !== drawLayer.#parent) { + // The div can still be in a canvas wrapper which has been removed. drawLayer.#parent.append(div); this.#selections.add(div); } diff --git a/test/integration/text_layer_spec.mjs b/test/integration/text_layer_spec.mjs index 47fbc4e06..0a5951493 100644 --- a/test/integration/text_layer_spec.mjs +++ b/test/integration/text_layer_spec.mjs @@ -20,9 +20,11 @@ import { closePages, closeSinglePage, + firstPageOnTop, getSpanRectFromText, kbSelectAll, loadAndWait, + scrollIntoView, waitForEvent, } from "./test_utils.mjs"; import { MathClamp } from "../../src/shared/math_clamp.js"; @@ -1148,6 +1150,94 @@ describe("Text layer", () => { ); }); }); + + describe("when the page has been destroyed and rendered again", () => { + const selectionSelector = ".canvasWrapper .selection svg path[d]"; + const timeout = 5000; + + let pages; + + beforeEach(async () => { + pages = await loadAndWait( + "tracemonkey.pdf", + `.page[data-page-number = "1"] .endOfContent`, + undefined, + undefined, + { annotationEditorMode: -1 } + ); + }); + + afterEach(async () => { + await closePages(pages); + }); + + async function selectSomeText(page) { + const [positionStart, positionEnd] = await Promise.all([ + getSpanRectFromText( + page, + 1, + "(frequently executed) bytecode sequences, records" + ).then(middlePosition), + getSpanRectFromText( + page, + 1, + "them, and compiles them to fast native code. We call such a se-" + ).then(belowEndPosition), + ]); + + await page.mouse.move(positionStart.x, positionStart.y); + await page.mouse.down(); + await moveInSteps(page, positionStart, positionEnd, 20); + await page.mouse.up(); + } + + it("must draw the selection", async () => { + await Promise.all( + pages.map(async ([browserName, page]) => { + await selectSomeText(page); + await page.waitForSelector(selectionSelector, { timeout }); + + await page.evaluate(() => { + document.getSelection().removeAllRanges(); + }); + await page.waitForSelector(selectionSelector, { + hidden: true, + timeout, + }); + + // Scroll down, page by page, until the first page view is + // evicted from the buffer: its canvas wrapper is then removed. + const pagesCount = await page.evaluate( + () => window.PDFViewerApplication.pagesCount + ); + let isDestroyed = false; + for (let i = 2; i <= pagesCount && !isDestroyed; i++) { + const selector = `.page[data-page-number = "${i}"]`; + await scrollIntoView(page, selector); + await page.waitForSelector( + `${selector} .canvasWrapper canvas`, + { timeout: 0 } + ); + isDestroyed = !(await page.$( + `.page[data-page-number = "1"] .canvasWrapper` + )); + } + + expect(isDestroyed) + .withContext(`In ${browserName}, first page destroyed`) + .toBeTrue(); + + await firstPageOnTop(page); + await page.waitForSelector( + `.page[data-page-number = "1"] .canvasWrapper canvas`, + { timeout: 0 } + ); + await selectSomeText(page); + await page.waitForSelector(selectionSelector, { timeout }); + }) + ); + }); + }); }); describe("using selection carets", () => { diff --git a/web/app.js b/web/app.js index cd3e6b617..9c033d85f 100644 --- a/web/app.js +++ b/web/app.js @@ -370,6 +370,7 @@ const PDFViewerApplication = { // Set some specific preferences for tests. if (typeof PDFJSDev !== "undefined" && PDFJSDev.test("TESTING")) { Object.assign(opts, { + annotationEditorMode: x => parseInt(x, 10), capCanvasAreaFactor: x => parseInt(x, 10), docBaseUrl: x => x, enableAltText: x => x === "true", diff --git a/web/pdf_page_view.js b/web/pdf_page_view.js index 3ee59520c..6691ae349 100644 --- a/web/pdf_page_view.js +++ b/web/pdf_page_view.js @@ -936,9 +936,14 @@ class PDFPageView extends BasePDFPageView { if (this.structTreeLayer && !this.textLayer) { this.structTreeLayer = null; } + // The annotation editor layer and the draw layer keep references on the + // text layer (the latter uses its div in order to render the selection), + // hence they must be recreated too. if ( this.annotationEditorLayer && - (!keepAnnotationEditorLayer || !this.annotationEditorLayer.div) + (!keepAnnotationEditorLayer || + !this.annotationEditorLayer.div || + !this.textLayer) ) { if (this.drawLayer) { this.drawLayer.cancel(); @@ -947,6 +952,10 @@ class PDFPageView extends BasePDFPageView { this.annotationEditorLayer.cancel(); this.annotationEditorLayer = null; } + if (this.drawLayer && !this.textLayer) { + this.drawLayer.cancel(); + this.drawLayer = null; + } if (this.xfaLayer && (!keepXfaLayer || !this.xfaLayer.div)) { this.xfaLayer.cancel(); this.xfaLayer = null;