From 4c62a49483c165fb18763be278855b5f202b9961 Mon Sep 17 00:00:00 2001 From: Calixte Denizet Date: Wed, 6 May 2026 14:35:02 +0200 Subject: [PATCH] Place new annotations on the correct page when extracting pages (bug 2027682) --- src/core/editor/pdf_editor.js | 17 ++++- src/core/worker.js | 1 + src/display/api.js | 27 ++++++- test/unit/api_spec.js | 140 ++++++++++++++++++++++++++++++++++ 4 files changed, 181 insertions(+), 4 deletions(-) diff --git a/src/core/editor/pdf_editor.js b/src/core/editor/pdf_editor.js index c4358b784..b6861034b 100644 --- a/src/core/editor/pdf_editor.js +++ b/src/core/editor/pdf_editor.js @@ -125,6 +125,8 @@ class PDFEditor { #newAnnotationsParams = null; + #primaryDocument = null; + currentDocument = null; oldPages = []; @@ -774,13 +776,22 @@ class PDFEditor { * @param {Array} pageInfos * @param {Object} annotationStorage - The annotation storage containing the * annotations to be merged into the new document. + * @param {PDFDocument} primaryDocument - The document the annotation storage + * belongs to. * @param {MessageHandler} handler - The message handler to use for processing * the annotations. * @param {WorkerTask} task - The worker task to use for reporting progress * and cancellation. * @return {Promise} */ - async extractPages(pageInfos, annotationStorage, handler, task) { + async extractPages( + pageInfos, + annotationStorage, + primaryDocument, + handler, + task + ) { + this.#primaryDocument = primaryDocument; if (pageInfos.some(info => info.insertAfter !== undefined)) { pageInfos = this.#resolveInsertAfterIndices(pageInfos); } @@ -2170,7 +2181,9 @@ class PDFEditor { } const newAnnotations = - this.#newAnnotationsParams?.newAnnotationsByPage?.get(pageIndex); + documentData.document === this.#primaryDocument + ? this.#newAnnotationsParams?.newAnnotationsByPage?.get(page.pageIndex) + : null; if (newAnnotations) { const { handler, task, imagesPromises } = this.#newAnnotationsParams; const changes = new RefSetCache(); diff --git a/src/core/worker.js b/src/core/worker.js index 93b87e399..9dd67e783 100644 --- a/src/core/worker.js +++ b/src/core/worker.js @@ -647,6 +647,7 @@ class WorkerMessageHandler { const buffer = await pdfEditor.extractPages( pageInfos, annotationStorage, + pdfManager.pdfDocument, handler, task ); diff --git a/src/display/api.js b/src/display/api.js index b81ee9b97..1fa44ddeb 100644 --- a/src/display/api.js +++ b/src/display/api.js @@ -2916,9 +2916,32 @@ class WorkerTransport { }; let transfer; if (this.annotationStorage.size > 0) { - const { map, transfer: t } = this.annotationStorage.serializable; + const serialized = this.annotationStorage.serializable; + let { map } = serialized; + transfer = serialized.transfer; + // Annotation pageIndex tracks the editor's current viewer position; the + // worker keys lookups by source index. Remap UI -> source via pagesMapper + // so reorganized pages still receive their annotations after extraction. + const mapping = this.pagesMapper.getMapping(); + if (mapping) { + const remapped = new Map(); + for (const [k, v] of map) { + if ( + v?.pageIndex !== undefined && + v.pageIndex >= 0 && + v.pageIndex < mapping.length + ) { + const sourceIdx = mapping[v.pageIndex] - 1; + if (sourceIdx !== v.pageIndex) { + remapped.set(k, { ...v, pageIndex: sourceIdx }); + continue; + } + } + remapped.set(k, v); + } + map = remapped; + } params.annotationStorage = map; - transfer = t; } return this.messageHandler .sendWithPromise("ExtractPages", params, transfer) diff --git a/test/unit/api_spec.js b/test/unit/api_spec.js index 52029f98b..a268524be 100644 --- a/test/unit/api_spec.js +++ b/test/unit/api_spec.js @@ -6354,6 +6354,146 @@ small scripts as well as for`); await loadingTask.destroy(); }); + it("places new annotations on the correct page after reordering (bug 2027682)", async function () { + let loadingTask = getDocument( + buildGetDocumentParams("doc_1_3_pages.pdf") + ); + let pdfDoc = await loadingTask.promise; + + pdfDoc.annotationStorage.setValue("pdfjs_internal_editor_0", { + annotationType: AnnotationEditorType.FREETEXT, + rect: [12, 34, 56, 78], + rotation: 0, + fontSize: 10, + color: [0, 0, 0], + value: "on source page 2", + pageIndex: 1, + }); + + const data = await pdfDoc.extractPages([ + { document: null, includePages: [0, 1, 2], pageIndices: [2, 0, 1] }, + ]); + await loadingTask.destroy(); + + loadingTask = getDocument(data); + pdfDoc = await loadingTask.promise; + expect(pdfDoc.numPages).toEqual(3); + + for (let i = 1; i <= 3; i++) { + const pdfPage = await pdfDoc.getPage(i); + const annotations = await pdfPage.getAnnotations(); + const hasNew = annotations.some( + a => a.contentsObj?.str === "on source page 2" + ); + if (i === 1) { + expect(hasNew).withContext(`Page ${i}`).toBeTrue(); + } else { + expect(hasNew).withContext(`Page ${i}`).toBeFalse(); + } + } + + await loadingTask.destroy(); + }); + + it("does not bleed new annotations across documents when merging (bug 2027682)", async function () { + let loadingTask = getDocument( + buildGetDocumentParams("doc_1_3_pages.pdf") + ); + let pdfDoc = await loadingTask.promise; + const pdfData2 = await DefaultFileReaderFactory.fetch({ + path: TEST_PDFS_PATH + "doc_2_3_pages.pdf", + }); + + pdfDoc.annotationStorage.setValue("pdfjs_internal_editor_0", { + annotationType: AnnotationEditorType.FREETEXT, + rect: [12, 34, 56, 78], + rotation: 0, + fontSize: 10, + color: [0, 0, 0], + value: "primary page 1", + pageIndex: 0, + }); + + const data = await pdfDoc.extractPages([ + { document: pdfData2 }, + { document: null }, + ]); + await loadingTask.destroy(); + + loadingTask = getDocument(data); + pdfDoc = await loadingTask.promise; + expect(pdfDoc.numPages).toEqual(6); + + for (let i = 1; i <= 6; i++) { + const pdfPage = await pdfDoc.getPage(i); + const annotations = await pdfPage.getAnnotations(); + const hasNew = annotations.some( + a => a.contentsObj?.str === "primary page 1" + ); + if (i === 4) { + expect(hasNew).withContext(`Page ${i}`).toBeTrue(); + } else { + expect(hasNew).withContext(`Page ${i}`).toBeFalse(); + } + } + + await loadingTask.destroy(); + }); + + it("places annotations correctly after the user reorganizes pages in the viewer (bug 2027682)", async function () { + const loadingTask = getDocument( + buildGetDocumentParams("page_with_number.pdf") + ); + const pdfDoc = await loadingTask.promise; + + // Simulate the user moving source page 1 (1-based) to UI position 5, + // mirroring what the viewer's pagesMapper records on a drag-drop. The + // annotation editor is then rebound to that new UI position, so its + // pageIndex (=4 in 0-based) reflects where the editor *currently* is, + // not where its source page is. + pdfDoc.pagesMapper.movePages(new Set([1]), [1], 5); + + pdfDoc.annotationStorage.setValue("pdfjs_internal_editor_0", { + annotationType: AnnotationEditorType.FREETEXT, + rect: [12, 34, 56, 78], + rotation: 0, + fontSize: 10, + color: [0, 0, 0], + value: "moved annotation", + pageIndex: 4, + }); + + // Export-all reflecting the post-reorganize layout: source 0 ends at + // output 4, the others shift down by one. + const data = await pdfDoc.extractPages([ + { + document: null, + includePages: [0, 1, 2, 3, 4], + pageIndices: [4, 0, 1, 2, 3], + }, + ]); + await loadingTask.destroy(); + + const newLoadingTask = getDocument(data); + const newPdfDoc = await newLoadingTask.promise; + expect(newPdfDoc.numPages).toEqual(5); + + for (let i = 1; i <= 5; i++) { + const pdfPage = await newPdfDoc.getPage(i); + const annotations = await pdfPage.getAnnotations(); + const hasNew = annotations.some( + a => a.contentsObj?.str === "moved annotation" + ); + if (i === 5) { + expect(hasNew).withContext(`Page ${i}`).toBeTrue(); + } else { + expect(hasNew).withContext(`Page ${i}`).toBeFalse(); + } + } + + await newLoadingTask.destroy(); + }); + it("fills missing pageIndices with the first free slots", async function () { let loadingTask = getDocument( buildGetDocumentParams("tracemonkey.pdf")