Merge pull request #21707 from calixteman/fix/pdf-editor-copy-new-annotations

Give copied annotations distinct references
This commit is contained in:
Tim van der Meij 2026-08-04 21:30:19 +02:00 committed by GitHub
commit 5f181fd8b0
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 352 additions and 56 deletions

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;
@ -2413,12 +2423,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;
@ -2487,9 +2503,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(

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;
}
}

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) {

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)
);
}
/**

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);
})

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();
});

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."

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",
() => {