Marmelator/pdf.js.mirror
1
0
Fork 0
mirror of https://github.com/mozilla/pdf.js.git synced 2026-08-04 21:37:23 +02:00
Code Issues Packages Projects Releases Wiki Activity

Give copied annotations distinct references

Browse Source 
When a viewer page was copied, each copy got its own annotationStorage
entry, but newAnnotationsByPage was keyed only by source page. As a
result, every output copy received all entries and reused their memoized
references.

Tag each entry with its rank among output copies of the same source page.
The display and worker compute this rank independently; inserted documents
preserve the order of copies. Keep unextracted entries at rank -1 so shared
stamp bitmaps remain available without being written. Without ranks, new
annotations are applied only to the first copy.
This commit is contained in:
Calixte Denizet 2026-07-13 12:04:22 +02:00
parent e57a46436f
commit bf7c9258c9
8 changed files with 352 additions and 56 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
src/core/editor/pdf_editor.js
View File

@ -57,6 +57,8 @@ class PageData {
this.annotations = null;
// Named destinations which points to this page.
this.pointingNamedDestinations = null;
// Rank of this page among the output pages sharing the same source page.
this.copyLevel = 0;
documentData.pagesMap.put(page.ref, this);
}
@ -1097,10 +1099,18 @@ class PDFEditor {
}
}
await Promise.all(promises);
const copyCounts = new Map();
for (let i = 0, ii = this.oldPages.length; i < ii; i++) {
if (this.oldPages[i] === undefined) {
const pageData = this.oldPages[i];
if (pageData === undefined) {
throw new Error("extractPages: sparse pageIndices.");
}
if (pageData) {
const { page } = pageData;
const copyLevel = copyCounts.get(page) ?? 0;
copyCounts.set(page, copyLevel + 1);
pageData.copyLevel = copyLevel;
}
}
promises.length = 0;
@ -2398,12 +2408,18 @@ class PDFEditor {
/**
* Create a copy of a page.
* @param {number} pageIndex
* @param {number} pageIndex - Index of the page slot in the new document
* (the index of the source page is `page.pageIndex`).
* @returns {Promise<Ref>} the page reference in the new PDF document.
*/
async #makePageCopy(pageIndex) {
const { page, documentData, annotations, pointingNamedDestinations } =
this.oldPages[pageIndex];
const {
page,
documentData,
annotations,
pointingNamedDestinations,
copyLevel,
} = this.oldPages[pageIndex];
this.currentDocument = documentData;
const { dedupNamedDestinations, oldRefMapping } = documentData;
const { xref, rotate, mediaBox, resources, ref: oldPageRef } = page;
@ -2472,9 +2488,11 @@ class PDFEditor {
const newAnnotations =
documentData.document === this.#primaryDocument
? this.#newAnnotationsParams?.newAnnotationsByPage?.get(page.pageIndex)
? this.#newAnnotationsParams?.newAnnotationsByPage
?.get(page.pageIndex)
?.filter(({ copyLevel: level }) => (level ?? 0) === copyLevel)
: null;
if (newAnnotations) {
if (newAnnotations?.length) {
const { handler, task, imagesPromises } = this.#newAnnotationsParams;
const changes = new RefSetCache();
const newData = await AnnotationFactory.saveNewAnnotations(

20
src/display/api.js
View File

@ -1009,11 +1009,15 @@ class PDFDocumentProxy {
/**
* @param {Array<PageInfo>} pageInfos - The pages to extract.
* @param {Int32Array} [copyLevels] - For each viewer page, its rank among the
* extracted pages sharing the same source page, or -1 if it isn't extracted.
* This routes editor annotations when the viewer contains multiple copies
* of a source page.
* @returns {Promise<Uint8Array>} A promise that is resolved with a
* {Uint8Array} containing the full data of the saved document.
*/
extractPages(pageInfos) {
return this._transport.extractPages(pageInfos);
extractPages(pageInfos, copyLevels = null) {
return this._transport.extractPages(pageInfos, copyLevels);
}
/**
@ -2936,7 +2940,7 @@ class WorkerTransport {
});
}
extractPages(pageInfos) {
extractPages(pageInfos, copyLevels = null) {
const params = {
pageInfos,
};
@ -2963,6 +2967,8 @@ class WorkerTransport {
// 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.
// Multiple viewer pages can share a source page. The copy level routes
// each editor annotation to the corresponding extracted copy.
const mapping = this.pagesMapper.getMapping();
if (mapping) {
const remapped = new Map();
@ -2972,9 +2978,13 @@ class WorkerTransport {
v.pageIndex >= 0 &&
v.pageIndex < mapping.length
) {
// copyLevels uses -1 for non-extracted pages. Keep their entries
// because an extracted stamp may share their bitmapId; the worker
// uses the negative level to skip the annotation itself.
const copyLevel = copyLevels?.[v.pageIndex] ?? 0;
const sourceIdx = mapping[v.pageIndex] - 1;
if (sourceIdx !== v.pageIndex) {
remapped.set(k, { ...v, pageIndex: sourceIdx });
if (sourceIdx !== v.pageIndex || copyLevel !== 0) {
remapped.set(k, { ...v, pageIndex: sourceIdx, copyLevel });
continue;
}
}

26
src/display/editor/tools.js
View File

@ -748,6 +748,8 @@ class AnnotationEditorUIManager {
#savedAllLayers = null;
#savedEditorsByPage = null;
#altTextManager = null;
#annotationStorage = null;
@ -1913,7 +1915,7 @@ class AnnotationEditorUIManager {
}
updatePageIndex(oldPageIndex, newPageIndex) {
for (const editor of this.getEditors(oldPageIndex)) {
for (const editor of this.#savedEditorsByPage.get(oldPageIndex) || []) {
editor.pageIndex = newPageIndex;
}
const layer = this.#savedAllLayers.get(oldPageIndex);
@ -1931,10 +1933,32 @@ class AnnotationEditorUIManager {
startUpdatePages() {
this.#savedAllLayers = new Map(this.#allLayers);
this.#allLayers.clear();
const savedEditorsByPage = (this.#savedEditorsByPage = new Map());
const saveEditor = editor => {
savedEditorsByPage
.getOrInsertComputed(editor.pageIndex, makeArr)
.push(editor);
};
for (const editor of this.#allEditors.values()) {
saveEditor(editor);
}
// Clones are initially kept serialized until their editor layer is
// rendered, hence they're not present in #allEditors yet.
for (const [id, editor] of this.#annotationStorage) {
if (
id.startsWith(AnnotationEditorPrefix) &&
!this.#allEditors.has(id) &&
Number.isInteger(editor?.pageIndex)
) {
saveEditor(editor);
}
}
}
endUpdatePages() {
this.#savedAllLayers = null;
this.#savedEditorsByPage = null;
}
clonePage(pageIndex, newPageIndex) {

61
src/display/pages_mapper.js
View File

@ -111,7 +111,6 @@ class PagesMapper {
movePages(selectedPages, pagesToMove, index) {
this.#ensureInit();
const pageNumberToId = this.#pageNumberToId;
const prevIdToPageNumber = this.#buildIdToPageNumber();
const movedCount = pagesToMove.length;
const mappedPagesToMove = new Uint32Array(movedCount);
let removedBeforeTarget = 0;
@ -126,6 +125,7 @@ class PagesMapper {
const pagesNumber = this.#pagesNumber;
const remainingLen = pagesNumber - movedCount;
const prevPageNumbers = new Int32Array(pagesNumber);
const adjustedTarget = MathClamp(
index - removedBeforeTarget,
0,
@ -135,7 +135,8 @@ class PagesMapper {
// Compact: keep only non-moved pages.
for (let i = 0, r = 0; i < pagesNumber; i++) {
if (!selectedPages.has(i + 1)) {
pageNumberToId[r++] = pageNumberToId[i];
pageNumberToId[r] = pageNumberToId[i];
prevPageNumbers[r++] = i + 1;
}
}
@ -146,8 +147,13 @@ class PagesMapper {
remainingLen
);
pageNumberToId.set(mappedPagesToMove, adjustedTarget);
this.#updatePrevPageNumbers(prevIdToPageNumber);
prevPageNumbers.copyWithin(
adjustedTarget + movedCount,
adjustedTarget,
remainingLen
);
prevPageNumbers.set(pagesToMove, adjustedTarget);
this.#prevPageNumbers = prevPageNumbers;
if (pageNumberToId.every((id, i) => id === i + 1)) {
this.#pageNumberToId = null;
@ -310,12 +316,48 @@ class PagesMapper {
return this.#pageNumberToId !== null;
}
/**
* Builds the copy level of every page. Extracted pages are ranked, in page
* order, among the extracted pages sharing their page ID. Pages which aren't
* extracted get -1.
* @param {Array<number>} [extractedPageNumbers] - Sorted 1-based page
* numbers to extract, or null to extract everything.
* @returns {Int32Array|null} null when the page mapping is the identity.
*/
#buildCopyLevels(extractedPageNumbers = null) {
if (!this.#pageNumberToId) {
return null;
}
const copyLevels = new Int32Array(this.#pagesNumber).fill(-1);
const counts = new Map();
if (extractedPageNumbers) {
for (const pageNumber of extractedPageNumbers) {
const id = this.getPageId(pageNumber);
const level = counts.get(id) ?? 0;
counts.set(id, level + 1);
copyLevels[pageNumber - 1] = level;
}
} else {
for (let i = 0, ii = this.#pagesNumber; i < ii; i++) {
const id = this.#pageNumberToId[i];
const level = counts.get(id) ?? 0;
counts.set(id, level + 1);
copyLevels[i] = level;
}
}
return copyLevels;
}
/**
* Gets the current page mapping suitable for saving.
* @param {Map<number, Array<number>>} [idToPageNumber]
* @returns {Array<Object>}
* @param {Int32Array} [copyLevels]
* @returns {{pageInfos: Array<Object>, copyLevels: Int32Array|null}}
*/
getPageMappingForSaving(idToPageNumber = null) {
getPageMappingForSaving(
idToPageNumber = null,
copyLevels = this.#buildCopyLevels()
) {
idToPageNumber ??= this.#buildIdToPageNumber();
// idToPageNumber maps used 1-based IDs to 1-based page numbers.
// For example if the final pdf contains page 3 twice and they are moved at
@ -364,7 +406,7 @@ class PagesMapper {
}
}
return extractParams;
return { pageInfos: extractParams, copyLevels };
}
extractPages(extractedPageNumbers) {
@ -377,7 +419,10 @@ class PagesMapper {
const usedPageNumbers = usedIds.getOrInsertComputed(id, makeArr);
usedPageNumbers.push(i + 1);
}
return this.getPageMappingForSaving(usedIds);
return this.getPageMappingForSaving(
usedIds,
this.#buildCopyLevels(extractedPageNumbers)
);
}
/**

60
test/integration/reorganize_pages_spec.mjs
View File

@ -815,19 +815,13 @@ describe("Reorganize Pages View", () => {
await page.waitForSelector("#viewsManagerStatusActionButton", {
visible: true,
});
const rect1 = await getRect(page, getThumbnailSelector(1));
const rect2 = await getRect(page, getThumbnailSelector(2));
await dragAndDrop(
page,
getThumbnailSelector(1),
[[0, rect2.y - rect1.y + rect2.height / 2]],
10
);
// Drag-and-drop behavior is covered separately. Use an exact move
// here so this test only checks the save payload.
await movePages(page, [1], 2);
const handleSave = await createPromise(page, resolve => {
window.PDFViewerApplication.onSavePages = async ({ data }) => {
resolve(Array.from(data[0].pageIndices));
resolve(Array.from(data.pageInfos[0].pageIndices));
};
});
@ -2032,9 +2026,12 @@ describe("Reorganize Pages View", () => {
const pagesData = await awaitPromise(handleExport);
expect(pagesData)
.withContext(`In ${browserName}`)
.toEqual([
{ document: null, pageIndices: [0, 1], includePages: [0, 2] },
]);
.toEqual({
pageInfos: [
{ document: null, pageIndices: [0, 1], includePages: [0, 2] },
],
copyLevels: null,
});
await waitForTextToBe(page, labelSelector, "Select pages");
// All checkboxes should be unchecked.
@ -2865,7 +2862,7 @@ describe("Reorganize Pages View", () => {
await closePages(pages);
});
it("should check that the pasted page has an ink annotation in the DOM", async () => {
it("should keep an ink annotation on a pasted and moved page", async () => {
await Promise.all(
pages.map(async ([browserName, page]) => {
// Enable ink editor mode and draw a line on page 1.
@ -2899,20 +2896,43 @@ describe("Reorganize Pages View", () => {
// Both the original and the cloned annotation must now be in storage.
await waitForStorageEntries(page, 2);
// Close the reorganize view and navigate to page 3 (the pasted copy)
// to trigger rendering of its annotation editor layer.
const editorIds = await page.evaluate(() => {
const entries = Array.from(
window.PDFViewerApplication.pdfDocument.annotationStorage
);
return {
original: entries.find(([, editor]) => editor.pageIndex === 0)[0],
clone: entries.find(([, editor]) => editor.pageIndex === 2)[0],
};
});
// Move the pasted copy before the original and verify that both
// stored page indices follow the new page order.
await movePages(page, [3], 0);
const editorPageIndices = await page.evaluate(ids => {
const storage =
window.PDFViewerApplication.pdfDocument.annotationStorage;
return {
original: storage.getRawValue(ids.original).pageIndex,
clone: storage.getRawValue(ids.clone).pageIndex,
};
}, editorIds);
expect(editorPageIndices)
.withContext(`In ${browserName}`)
.toEqual({ original: 1, clone: 0 });
// Show the moved copy and verify that its cloned editor survived.
await page.click("#viewsManagerToggleButton");
await page.waitForSelector("#viewsManager", { hidden: true });
await page.evaluate(() => {
window.PDFViewerApplication.pdfViewer.currentPageNumber = 3;
window.PDFViewerApplication.pdfViewer.currentPageNumber = 1;
});
// The cloned ink annotation must appear in the DOM of page 3.
await page.waitForSelector(`.page[data-page-number="3"] .inkEditor`, {
await page.waitForSelector(`.page[data-page-number="1"] .inkEditor`, {
visible: true,
});
const inkEditors = await page.$$(
`.page[data-page-number="3"] .inkEditor`
`.page[data-page-number="1"] .inkEditor`
);
expect(inkEditors.length).withContext(`In ${browserName}`).toBe(1);
})

193
test/unit/api_spec.js
View File

@ -7302,9 +7302,14 @@ small scripts as well as for`);
let loadingTask = getDocument(buildGetDocumentParams("empty.pdf"));
let pdfDoc = await loadingTask.promise;
// Simulate what clonePage() puts in annotationStorage when a page is
// copied: the original annotation stays on pageIndex 0 and the clone
// is placed on pageIndex 1 (the new position of the pasted copy).
// Mirror the viewer: copy page 1 and paste the copy right after it.
pdfDoc.pagesMapper.copyPages(new Uint32Array([1]));
pdfDoc.pagesMapper.pastePages(1);
expect(pdfDoc.pagesMapper.pagesNumber).toEqual(2);
// Model the annotationStorage entries after clonePage(): the original
// stays at viewer page index 0 and the copy gets an entry at index 1.
// Both viewer pages map to source page 0.
const inkAnnotation = {
annotationType: AnnotationEditorType.INK,
rect: [50, 50, 200, 200],
@ -7342,15 +7347,15 @@ small scripts as well as for`);
});
pdfDoc.annotationStorage.setValue("pdfjs_internal_editor_1", {
...inkAnnotation,
color: [255, 0, 0],
pageIndex: 1,
isClone: true,
});
// Extract page 0 twice: once at output position 0 (original) and once
// at output position 1 (clone), mirroring copy+paste in the UI.
const data = await pdfDoc.extractPages([
{ document: null, includePages: [0], pageIndices: [0] },
{ document: null, includePages: [0], pageIndices: [1] },
]);
const { pageInfos, copyLevels } = pdfDoc.pagesMapper.extractPages(
new Set([1, 2])
);
const data = await pdfDoc.extractPages(pageInfos, copyLevels);
await loadingTask.destroy();
loadingTask = getDocument({ data });
@ -7358,7 +7363,8 @@ small scripts as well as for`);
expect(pdfDoc.numPages).toEqual(2);
// Both pages should carry the ink annotation.
// Each page must have the expected color and a distinct annotation ID.
const annotationIds = [];
for (let i = 1; i <= 2; i++) {
const pdfPage = await pdfDoc.getPage(i);
const annotations = await pdfPage.getAnnotations();
@ -7366,7 +7372,174 @@ small scripts as well as for`);
expect(annotations[0].annotationType)
.withContext(`Page ${i}`)
.toEqual(AnnotationType.INK);
expect(Array.from(annotations[0].color))
.withContext(`Page ${i}`)
.toEqual(i === 1 ? [0, 0, 255] : [255, 0, 0]);
annotationIds.push(annotations[0].id);
}
expect(new Set(annotationIds).size).toEqual(2);
await loadingTask.destroy();
});
it("keeps distinct annotations when a clone moves before its original", async function () {
let loadingTask = getDocument(buildGetDocumentParams("empty.pdf"));
let pdfDoc = await loadingTask.promise;
pdfDoc.pagesMapper.copyPages(new Uint32Array([1]));
pdfDoc.pagesMapper.pastePages(1);
const freeText = {
annotationType: AnnotationEditorType.FREETEXT,
rect: [12, 34, 56, 78],
rotation: 0,
fontSize: 10,
color: [0, 0, 0],
};
const original = {
...freeText,
value: "on the original",
pageIndex: 0,
};
const clone = {
...freeText,
value: "on the clone",
pageIndex: 1,
isClone: true,
};
pdfDoc.annotationStorage.setValue("pdfjs_internal_editor_0", original);
pdfDoc.annotationStorage.setValue("pdfjs_internal_editor_1", clone);
// Move the clone before the original and update both editor page
// indices to match.
pdfDoc.pagesMapper.movePages(new Set([2]), [2], 0);
expect(pdfDoc.pagesMapper.getPrevPageNumber(1)).toEqual(2);
expect(pdfDoc.pagesMapper.getPrevPageNumber(2)).toEqual(1);
clone.pageIndex = 0;
original.pageIndex = 1;
const { pageInfos, copyLevels } =
pdfDoc.pagesMapper.getPageMappingForSaving();
const data = await pdfDoc.extractPages(pageInfos, copyLevels);
await loadingTask.destroy();
loadingTask = getDocument({ data });
pdfDoc = await loadingTask.promise;
expect(pdfDoc.numPages).toEqual(2);
for (let i = 1; i <= 2; i++) {
const pdfPage = await pdfDoc.getPage(i);
const annotations = await pdfPage.getAnnotations();
expect(annotations.map(a => a.contentsObj?.str)).toEqual([
i === 1 ? "on the clone" : "on the original",
]);
}
await loadingTask.destroy();
});
it("only keeps the annotations of the extracted copy of a page", async function () {
let loadingTask = getDocument(
buildGetDocumentParams("three_pages_with_number.pdf")
);
let pdfDoc = await loadingTask.promise;
// Copy page 1 and paste it at the end: source page 0 is now at the
// viewer positions 1 and 4.
pdfDoc.pagesMapper.copyPages(new Uint32Array([1]));
pdfDoc.pagesMapper.pastePages(3);
expect(pdfDoc.pagesMapper.pagesNumber).toEqual(4);
const freeText = {
annotationType: AnnotationEditorType.FREETEXT,
rect: [12, 34, 56, 78],
rotation: 0,
fontSize: 10,
color: [0, 0, 0],
};
pdfDoc.annotationStorage.setValue("pdfjs_internal_editor_0", {
...freeText,
value: "on the original",
pageIndex: 0,
});
pdfDoc.annotationStorage.setValue("pdfjs_internal_editor_1", {
...freeText,
value: "on the clone",
pageIndex: 3,
isClone: true,
});
// Extract the clone only: the annotation of the original must not be
// written on it.
const { pageInfos, copyLevels } = pdfDoc.pagesMapper.extractPages(
new Set([4])
);
const data = await pdfDoc.extractPages(pageInfos, copyLevels);
await loadingTask.destroy();
loadingTask = getDocument({ data });
pdfDoc = await loadingTask.promise;
expect(pdfDoc.numPages).toEqual(1);
const pdfPage = await pdfDoc.getPage(1);
const annotations = await pdfPage.getAnnotations();
expect(annotations.map(a => a.contentsObj?.str)).toEqual([
"on the clone",
]);
await loadingTask.destroy();
});
it("keeps a shared stamp image when extracting only a copied page", async function () {
if (isNodeJS) {
pending("Cannot create a bitmap from Node.js.");
}
const bitmap = await getImageBitmap("firefox_logo.png");
let loadingTask = getDocument(buildGetDocumentParams("empty.pdf"));
let pdfDoc = await loadingTask.promise;
pdfDoc.pagesMapper.copyPages(new Uint32Array([1]));
pdfDoc.pagesMapper.pastePages(1);
const stamp = {
annotationType: AnnotationEditorType.STAMP,
rect: [12, 34, 56, 78],
rotation: 0,
bitmapId: "im1",
};
// Model stamp serialization: both entries share a bitmapId, but only
// one carries the bitmap data.
pdfDoc.annotationStorage.setValue("pdfjs_internal_editor_0", {
...stamp,
bitmap,
pageIndex: 0,
});
pdfDoc.annotationStorage.setValue("pdfjs_internal_editor_1", {
...stamp,
pageIndex: 1,
isClone: true,
isCopy: true,
});
const { pageInfos, copyLevels } = pdfDoc.pagesMapper.extractPages(
new Set([2])
);
const data = await pdfDoc.extractPages(pageInfos, copyLevels);
expect(data).not.toBeNull();
await loadingTask.destroy();
loadingTask = getDocument({ data });
pdfDoc = await loadingTask.promise;
expect(pdfDoc.numPages).toEqual(1);
const pdfPage = await pdfDoc.getPage(1);
const annotations = await pdfPage.getAnnotations();
expect(annotations.length).toEqual(1);
expect(annotations[0].annotationType).toEqual(AnnotationType.STAMP);
const opList = await pdfPage.getOperatorList();
expect(opList.fnArray).toContain(OPS.paintImageXObject);
await loadingTask.destroy();
});

14
web/app.js
View File

@ -2563,7 +2563,7 @@ const PDFViewerApplication = {
this.pdfViewer.onPagesEdited(data);
},
async onSavePages({ data: extractParams }) {
async onSavePages({ data: { pageInfos, copyLevels } }) {
if (typeof PDFJSDev !== "undefined" && PDFJSDev.test("TESTING")) {
return;
}
@ -2573,7 +2573,10 @@ const PDFViewerApplication = {
if (!this.pdfDocument) {
return;
}
const modifiedPdfBytes = await this.pdfDocument.extractPages(extractParams);
const modifiedPdfBytes = await this.pdfDocument.extractPages(
pageInfos,
copyLevels
);
if (!modifiedPdfBytes) {
console.error(
"Something wrong happened when saving the edited PDF.\nPlease file a bug."
@ -2587,11 +2590,14 @@ const PDFViewerApplication = {
);
},
async onSaveAndLoad({ data: extractParams }) {
async onSaveAndLoad({ data: { pageInfos, copyLevels } }) {
if (!this.pdfDocument) {
return;
}
const modifiedPdfBytes = await this.pdfDocument.extractPages(extractParams);
const modifiedPdfBytes = await this.pdfDocument.extractPages(
pageInfos,
copyLevels
);
if (!modifiedPdfBytes) {
console.error(
"Something wrong happened when saving the edited PDF.\nPlease file a bug."

4
web/pdf_thumbnail_viewer.js
View File

@ -386,8 +386,8 @@ class PDFThumbnailViewer {
const pagesCount = this.#pagesMapper.pagesNumber;
const data = this.hasStructuralChanges()
? this.getStructuralChanges()
: [{ document: null }];
data.push(...entries);
: { pageInfos: [{ document: null }], copyLevels: null };
data.pageInfos.push(...entries);
this.eventBus.on(
"pagesloaded",
() => {