Merge pull request #20907 from calixteman/fix_bad_bugs

Fix various bug around copy/paste/delete/undo (bug 2022586, bug 2022824, bug 2022884, bug 2023171, bug 2023176)
This commit is contained in:
calixteman 2026-03-18 21:38:15 +01:00 committed by GitHub
commit e65d643af5
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 795 additions and 274 deletions

View File

@ -1363,6 +1363,19 @@ class PDFPageProxy {
this.imageCoordinates = null;
}
clone(id) {
const clone = new PDFPageProxy(
id,
this._pageInfo,
this._transport,
this.#pagesMapper,
this._pdfBug
);
clone.clonedFromIndex = this.clonedFromIndex ?? this._pageIndex;
this._transport.updatePage(clone);
return clone;
}
/**
* @type {number} Page number of the page. First page is 1.
*/
@ -1375,6 +1388,7 @@ class PDFPageProxy {
*/
set pageNumber(value) {
this._pageIndex = value - 1;
this._transport.updatePage(this);
}
/**
@ -2431,10 +2445,6 @@ class WorkerTransport {
#passwordCapability = null;
#copiedPageInfo = null;
#savedPageInfo = null;
constructor(
messageHandler,
loadingTask,
@ -2468,7 +2478,6 @@ class WorkerTransport {
this.setupMessageHandler();
this.pagesMapper = pagesMapper;
this.pagesMapper.addListener(this.#updateCaches.bind(this));
if (typeof PDFJSDev === "undefined" || PDFJSDev.test("TESTING")) {
// For testing purposes.
@ -2494,76 +2503,10 @@ class WorkerTransport {
}
}
#updateCaches({ type, pageNumbers }) {
if (type === "copy") {
this.#copiedPageInfo = new Map();
for (const pageNum of pageNumbers) {
this.#copiedPageInfo.set(pageNum, {
proxy: this.#pageCache.get(pageNum - 1) || null,
promise: this.#pagePromises.get(pageNum - 1) || null,
});
}
return;
}
if (type === "cancelCopy") {
this.#copiedPageInfo = null;
return;
}
if (type === "delete") {
this.#savedPageInfo = {
pageCache: new Map(this.#pageCache),
pagePromises: new Map(this.#pagePromises),
};
for (const pageNum of pageNumbers) {
this.#pageCache.delete(pageNum - 1);
this.#pagePromises.delete(pageNum - 1);
}
}
if (type === "cancelDelete") {
if (this.#savedPageInfo) {
this.#pageCache = this.#savedPageInfo.pageCache;
this.#pagePromises = this.#savedPageInfo.pagePromises;
this.#savedPageInfo = null;
}
return;
}
if (type === "cleanSavedData") {
this.#savedPageInfo = null;
return;
}
const newPageCache = new Map();
const newPromiseCache = new Map();
const { pagesMapper } = this;
for (let i = 0, ii = pagesMapper.pagesNumber; i < ii; i++) {
const prevPageNumber = pagesMapper.getPrevPageNumber(i + 1);
if (prevPageNumber < 0) {
const { proxy, promise } =
this.#copiedPageInfo?.get(-prevPageNumber) || {};
if (proxy) {
newPageCache.set(i, proxy);
}
if (promise) {
newPromiseCache.set(i, promise);
}
continue;
}
const prevPageIndex = prevPageNumber - 1;
const page = this.#pageCache.get(prevPageIndex);
if (page) {
newPageCache.set(i, page);
}
const promise = this.#pagePromises.get(prevPageIndex);
if (promise) {
newPromiseCache.set(i, promise);
}
}
this.#pageCache = newPageCache;
this.#pagePromises = newPromiseCache;
updatePage(page) {
const { _pageIndex } = page;
this.#pageCache.set(_pageIndex, page);
this.#pagePromises.set(_pageIndex, Promise.resolve(page));
}
#cacheSimpleMethod(name, data = null) {

View File

@ -19,146 +19,75 @@ import { makeArr, MathClamp } from "../shared/util.js";
* Maps between page IDs and page numbers, allowing bidirectional conversion
* between the two representations. This is useful when the page numbering
* in the PDF document doesn't match the default sequential ordering.
*
* When #pageNumberToId is null the mapping is the identity (page N has ID N).
*/
class PagesMapper {
/**
* Maps page IDs to their corresponding page numbers.
* @type {Map<number, Array<number>>|null}
*/
#idToPageNumber = null;
/**
* Maps page numbers to their corresponding page IDs.
* Maps page positions (0-indexed) to their page IDs (1-indexed).
* Null when the mapping is the identity.
* @type {Uint32Array|null}
*/
#pageNumberToId = null;
/**
* Previous mapping of page IDs to page numbers.
* Previous page number for each position, used to track what happened to
* each page after a mutation. Negative values indicate copied pages.
* @type {Int32Array|null}
*/
#prevPageNumbers = null;
/**
* The total number of pages.
* @type {number}
*/
/** @type {number} */
#pagesNumber = 0;
/**
* Listeners for page changes.
* @type {Array<function>}
* Clipboard state for copy/paste operations.
* @type {{pageNumbers: Uint32Array, pageIds: Uint32Array}|null}
*/
#listeners = [];
/**
* Maps page numbers to their corresponding page IDs (used in copy
* operations).
* @type {Uint32Array|null}
*/
#copiedPageIds = null;
/**
* Maps page IDs to their corresponding page numbers, used in copy operations.
* @type {Uint32Array|null}
*/
#copiedPageNumbers = null;
#clipboard = null;
/** Saved state for undoing a delete. */
#savedData = null;
/**
* Gets the total number of pages.
* @returns {number} The number of pages.
*/
get pagesNumber() {
return this.#pagesNumber;
}
/**
* Sets the total number of pages and initializes default mappings
* where page IDs equal page numbers (1-indexed).
* @param {number} n - The total number of pages.
*/
set pagesNumber(n) {
if (this.#pagesNumber === n) {
return;
}
this.#pagesNumber = n;
this.#reset();
}
/**
* Resets the page mappings to their default state, where page IDs equal page
* numbers (1-indexed). This is called when the number of pages changes, or
* when the current mapping matches the default mapping after a move
* operation.
*/
#reset() {
this.#pageNumberToId = null;
this.#idToPageNumber = null;
this.#prevPageNumbers = null;
}
/**
* Adds a listener function that will be called whenever the page mappings
* are updated.
* @param {function} listener
* Ensures the identity mapping is initialized.
* Must be called before any mutation or before reading #pageNumberToId.
*/
addListener(listener) {
this.#listeners.push(listener);
}
/**
* Removes a previously added listener function.
* @param {function} listener
*/
removeListener(listener) {
const index = this.#listeners.indexOf(listener);
if (index >= 0) {
this.#listeners.splice(index, 1);
}
}
/**
* Calls all registered listener functions to notify them of changes to the
* page mappings.
* @param {Object} data - An object containing information about the update.
*/
#updateListeners(data) {
for (const listener of this.#listeners) {
listener(data);
}
}
/**
* Initializes the page mappings if they haven't been initialized yet.
* @param {boolean} mustInit
*/
#init(mustInit) {
#ensureInit() {
if (this.#pageNumberToId) {
return;
}
const n = this.#pagesNumber;
const pageNumberToId = (this.#pageNumberToId = new Uint32Array(n));
this.#prevPageNumbers = new Int32Array(pageNumberToId);
const idToPageNumber = (this.#idToPageNumber = new Map());
if (mustInit) {
for (let i = 1; i <= n; i++) {
pageNumberToId[i - 1] = i;
idToPageNumber.set(i, [i]);
}
for (let i = 0; i < n; i++) {
pageNumberToId[i] = i + 1;
}
this.#prevPageNumbers = new Int32Array(pageNumberToId);
}
/**
* Updates the mapping from page IDs to page numbers based on the current
* mapping from page numbers to page IDs. This should be called after any
* changes to the page-number-to-ID mapping to keep the two mappings in sync.
* Builds and returns the inverse map (id to page numbers) from
* #pageNumberToId.
* Since a page ID can appear multiple times (after a copy), the value is an
* array of all page numbers that share that ID.
* @returns {Map<number, Array<number>>}
*/
#updateIdToPageNumber() {
const idToPageNumber = this.#idToPageNumber;
#buildIdToPageNumber() {
const idToPageNumber = new Map();
const pageNumberToId = this.#pageNumberToId;
idToPageNumber.clear();
for (let i = 0, ii = this.#pagesNumber; i < ii; i++) {
const id = pageNumberToId[i];
const pageNumbers = idToPageNumber.get(id);
@ -168,20 +97,20 @@ class PagesMapper {
idToPageNumber.set(id, [i + 1]);
}
}
return idToPageNumber;
}
/**
* Move a set of pages to a new position while keeping IDnumber mappings in
* sync.
* Move a set of pages to a new position.
*
* @param {Set<number>} selectedPages - Page numbers being moved (1-indexed).
* @param {number[]} pagesToMove - Ordered list of page numbers to move.
* @param {number} index - Zero-based insertion index in the page-number list.
*/
movePages(selectedPages, pagesToMove, index) {
this.#init(true);
this.#ensureInit();
const pageNumberToId = this.#pageNumberToId;
const idToPageNumber = this.#idToPageNumber;
const prevIdToPageNumber = this.#buildIdToPageNumber();
const movedCount = pagesToMove.length;
const mappedPagesToMove = new Uint32Array(movedCount);
let removedBeforeTarget = 0;
@ -190,63 +119,60 @@ class PagesMapper {
const pageIndex = pagesToMove[i] - 1;
mappedPagesToMove[i] = pageNumberToId[pageIndex];
if (pageIndex < index) {
removedBeforeTarget += 1;
removedBeforeTarget++;
}
}
const pagesNumber = this.#pagesNumber;
// target index after removing elements that were before it
let adjustedTarget = index - removedBeforeTarget;
const remainingLen = pagesNumber - movedCount;
adjustedTarget = MathClamp(adjustedTarget, 0, remainingLen);
const adjustedTarget = MathClamp(
index - removedBeforeTarget,
0,
remainingLen
);
// Create the new mapping.
// First copy over the pages that are not being moved.
// Then insert the moved pages at the target position.
// Compact: keep only non-moved pages.
for (let i = 0, r = 0; i < pagesNumber; i++) {
if (!selectedPages.has(i + 1)) {
pageNumberToId[r++] = pageNumberToId[i];
}
}
// Shift the pages after the target position.
// Make room at the target and insert.
pageNumberToId.copyWithin(
adjustedTarget + movedCount,
adjustedTarget,
remainingLen
);
// Finally insert the moved pages.
pageNumberToId.set(mappedPagesToMove, adjustedTarget);
this.#setPrevPageNumbers(idToPageNumber, null);
this.#updateIdToPageNumber();
this.#updateListeners({ type: "move" });
this.#updatePrevPageNumbers(prevIdToPageNumber);
if (pageNumberToId.every((id, i) => id === i + 1)) {
this.#reset();
this.#pageNumberToId = null;
}
}
/**
* Deletes a set of pages while keeping IDnumber mappings in sync.
* @param {Array<number>} pagesToDelete - Page numbers to delete (1-indexed).
* These must be unique and sorted in ascending order.
* Deletes a set of pages.
* @param {Array<number>} pagesToDelete - Page numbers to delete (1-indexed),
* unique and sorted ascending.
*/
deletePages(pagesToDelete) {
this.#init(true);
this.#ensureInit();
const pageNumberToId = this.#pageNumberToId;
const prevIdToPageNumber = this.#idToPageNumber;
const prevIdToPageNumber = this.#buildIdToPageNumber();
this.#savedData = {
pageNumberToId: pageNumberToId.slice(),
idToPageNumber: new Map(prevIdToPageNumber),
pageNumber: this.#pagesNumber,
pagesNumber: this.#pagesNumber,
prevPageNumbers: this.#prevPageNumbers.slice(),
};
this.pagesNumber -= pagesToDelete.length;
this.#init(false);
const newPageNumberToId = this.#pageNumberToId;
const newN = this.#pagesNumber - pagesToDelete.length;
this.#pagesNumber = newN;
const newPageNumberToId = (this.#pageNumberToId = new Uint32Array(newN));
this.#prevPageNumbers = new Int32Array(newN);
let sourceIndex = 0;
let destIndex = 0;
@ -265,107 +191,119 @@ class PagesMapper {
newPageNumberToId.set(pageNumberToId.subarray(sourceIndex), destIndex);
}
this.#setPrevPageNumbers(prevIdToPageNumber, null);
this.#updateIdToPageNumber();
this.#updateListeners({ type: "delete", pageNumbers: pagesToDelete });
this.#updatePrevPageNumbers(prevIdToPageNumber, new Set(pagesToDelete));
}
cancelDelete() {
if (this.#savedData) {
this.#pageNumberToId = this.#savedData.pageNumberToId;
this.#idToPageNumber = this.#savedData.idToPageNumber;
this.pagesNumber = this.#savedData.pageNumber;
this.#pagesNumber = this.#savedData.pagesNumber;
this.#prevPageNumbers = this.#savedData.prevPageNumbers;
this.#savedData = null;
this.#updateListeners({ type: "cancelDelete" });
}
}
cleanSavedData() {
this.#savedData = null;
this.#updateListeners({ type: "cleanSavedData" });
}
/**
* Copies a set of pages while keeping IDnumber mappings in sync.
* Records which pages are being copied so that pastePages can insert them.
* @param {Uint32Array} pagesToCopy - Page numbers to copy (1-indexed).
*/
copyPages(pagesToCopy) {
this.#init(true);
this.#copiedPageNumbers = pagesToCopy;
this.#copiedPageIds = pagesToCopy.map(
pageNumber => this.#pageNumberToId[pageNumber - 1]
);
this.#updateListeners({ type: "copy", pageNumbers: pagesToCopy });
this.#ensureInit();
this.#clipboard = {
pageNumbers: pagesToCopy,
pageIds: pagesToCopy.map(n => this.#pageNumberToId[n - 1]),
};
}
cancelCopy() {
this.#copiedPageIds = null;
this.#copiedPageNumbers = null;
this.#updateListeners({ type: "cancelCopy" });
this.#clipboard = null;
}
/**
* Pastes a set of pages while keeping IDnumber mappings in sync.
* Inserts the previously copied pages at the given position.
* @param {number} index - Zero-based insertion index in the page-number list.
*/
pastePages(index) {
this.#init(true);
this.#ensureInit();
const pageNumberToId = this.#pageNumberToId;
const prevIdToPageNumber = this.#idToPageNumber;
const copiedPageNumbers = this.#copiedPageNumbers;
const prevIdToPageNumber = this.#buildIdToPageNumber();
const { pageNumbers: copiedPageNumbers, pageIds: copiedPageIds } =
this.#clipboard;
const copiedPageMapping = new Map();
let base = index;
for (const pageNumber of copiedPageNumbers) {
copiedPageMapping.set(++base, pageNumber);
}
this.pagesNumber += copiedPageNumbers.length;
this.#init(false);
const newPageNumberToId = this.#pageNumberToId;
const newN = this.#pagesNumber + copiedPageNumbers.length;
this.#pagesNumber = newN;
const newPageNumberToId = (this.#pageNumberToId = new Uint32Array(newN));
this.#prevPageNumbers = new Int32Array(newN);
newPageNumberToId.set(pageNumberToId.subarray(0, index), 0);
newPageNumberToId.set(this.#copiedPageIds, index);
newPageNumberToId.set(copiedPageIds, index);
newPageNumberToId.set(
pageNumberToId.subarray(index),
index + copiedPageNumbers.length
);
this.#setPrevPageNumbers(prevIdToPageNumber, copiedPageMapping);
this.#updateIdToPageNumber();
this.#updateListeners({ type: "paste" });
this.#copiedPageIds = null;
this.#copiedPageNumbers = null;
this.#updatePrevPageNumbers(
prevIdToPageNumber,
null,
index,
copiedPageNumbers
);
this.#clipboard = null;
}
/**
* Updates the previous page numbers based on the current page-number-to-ID
* mapping and the provided previous ID-to-page-number mapping.
* This is used to keep track of the original page numbers for each page ID.
* @param {Map<number, Array<number>} prevIdToPageNumber
* @param {Map<number, number>|null} copiedPageMapping
* Recomputes #prevPageNumbers after a mutation, using the pre-mutation
* id to pageNumbers map to track where each page came from.
*
* @param {Map<number, Array<number>>} prevIdToPageNumber - Id to pageNumbers
* before the mutation.
* @param {Set<number>|null} [deletedPageNumbers] - Page numbers that were
* deleted (so their old positions are skipped).
* @param {number} [pasteIndex] - If this is a paste, the zero-based
* insertion index; paired with copiedPageNumbers.
* @param {Uint32Array} [copiedPageNumbers] - Source page numbers of the
* pasted pages; paired with pasteIndex.
*/
#setPrevPageNumbers(prevIdToPageNumber, copiedPageMapping) {
#updatePrevPageNumbers(
prevIdToPageNumber,
deletedPageNumbers = null,
pasteIndex = -1,
copiedPageNumbers = null
) {
const prevPageNumbers = this.#prevPageNumbers;
const newPageNumberToId = this.#pageNumberToId;
const pasteEnd = pasteIndex + (copiedPageNumbers?.length ?? 0);
const idsIndices = new Map();
for (let i = 0, ii = this.#pagesNumber; i < ii; i++) {
const oldPageNumber = copiedPageMapping?.get(i + 1);
if (oldPageNumber) {
prevPageNumbers[i] = -oldPageNumber;
if (i >= pasteIndex && i < pasteEnd) {
// Negative value signals this page is a copy; encodes its source.
prevPageNumbers[i] = -copiedPageNumbers[i - pasteIndex];
continue;
}
const id = newPageNumberToId[i];
const j = idsIndices.get(id) || 0;
prevPageNumbers[i] = prevIdToPageNumber.get(id)?.[j];
const oldPositions = prevIdToPageNumber.get(id);
let j = idsIndices.get(id) || 0;
if (deletedPageNumbers && oldPositions) {
while (
j < oldPositions.length &&
deletedPageNumbers.has(oldPositions[j])
) {
j++;
}
}
prevPageNumbers[i] = oldPositions?.[j];
idsIndices.set(id, j + 1);
}
}
/**
* Checks if the page mappings have been altered from their initial state.
* @returns {boolean} True if the mappings have been altered, false otherwise.
* @returns {boolean}
*/
hasBeenAltered() {
return this.#pageNumberToId !== null;
@ -373,9 +311,11 @@ class PagesMapper {
/**
* Gets the current page mapping suitable for saving.
* @returns {Object} An object containing the page indices.
* @param {Map<number, Array<number>>} [idToPageNumber]
* @returns {Array<Object>}
*/
getPageMappingForSaving(idToPageNumber = this.#idToPageNumber) {
getPageMappingForSaving(idToPageNumber = null) {
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
// page 1 and 4, then it contains:
@ -441,28 +381,37 @@ class PagesMapper {
/**
* Gets the previous page number for a given page number.
* Negative values indicate a copied page (the absolute value is the source).
* @param {number} pageNumber
* @returns {number} The previous page number for the given page number, or 0
* if no mapping exists.
* @returns {number}
*/
getPrevPageNumber(pageNumber) {
return this.#prevPageNumbers[pageNumber - 1] ?? 0;
return this.#prevPageNumbers?.[pageNumber - 1] ?? 0;
}
/**
* Gets the page number for a given page ID.
* Gets the first page number that currently maps to the given page ID.
* @param {number} id - The page ID (1-indexed).
* @returns {number} The page number, or 0 if no mapping exists.
* @returns {number} The page number, or 0 if not found.
*/
getPageNumber(id) {
return this.#idToPageNumber ? (this.#idToPageNumber.get(id)?.[0] ?? 0) : id;
if (!this.#pageNumberToId) {
return id; // identity mapping
}
const pageNumberToId = this.#pageNumberToId;
for (let i = 0, ii = this.#pagesNumber; i < ii; i++) {
if (pageNumberToId[i] === id) {
return i + 1;
}
}
return 0;
}
/**
* Gets the page ID for a given page number.
* @param {number} pageNumber - The page number (1-indexed).
* @returns {number} The page ID, or the page number itself if no mapping
* exists.
* exists.
*/
getPageId(pageNumber) {
return this.#pageNumberToId?.[pageNumber - 1] ?? pageNumber;

View File

@ -32,6 +32,7 @@ import {
scrollIntoView,
showViewsManager,
waitAndClick,
waitForBrowserTrip,
waitForDOMMutation,
waitForTextToBe,
waitForTooltipToBe,
@ -1819,6 +1820,122 @@ describe("Reorganize Pages View", () => {
`In ${browserName}, dragging should be disabled when pasting`
)
.toBeUndefined();
// Wait a tick to ensure that the controller.abort() has taken effect
// before leaving.
await waitForBrowserTrip(page);
})
);
});
});
describe("Copy, paste and delete pages with keyboard shortcuts", () => {
let pages;
beforeEach(async () => {
pages = await loadAndWait(
"page_with_number.pdf",
"#viewsManagerToggleButton",
"page-width",
null,
{ enableSplitMerge: true }
);
});
afterEach(async () => {
await closePages(pages);
});
it("should copy, paste and delete pages correctly", async () => {
await Promise.all(
pages.map(async ([browserName, page]) => {
await waitForThumbnailVisible(page, 1);
// Check thumbnail 1 and copy it with Ctrl+C.
await waitAndClick(
page,
`.thumbnail:has(${getThumbnailSelector(1)}) input`
);
let handlePagesEdited = await waitForPagesEdited(page, "copy");
await kbCopy(page);
let pageIndices = await awaitPromise(handlePagesEdited);
let expected = [
1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17,
];
expect(pageIndices)
.withContext(`In ${browserName}`)
.toEqual(expected);
// Paste after page 3.
handlePagesEdited = await waitForPagesEdited(page);
await waitAndClick(page, `${getThumbnailSelector(3)}+button`);
pageIndices = await awaitPromise(handlePagesEdited);
expected = [
1, 2, 3, 1, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17,
];
expect(pageIndices)
.withContext(`In ${browserName}`)
.toEqual(expected);
// Check thumbnail 1 and delete it.
await waitAndClick(
page,
`.thumbnail:has(${getThumbnailSelector(1)}) input`
);
handlePagesEdited = await waitForPagesEdited(page);
await kbDelete(page);
pageIndices = await awaitPromise(handlePagesEdited);
expected = [
2, 3, 1, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17,
];
expect(pageIndices)
.withContext(`In ${browserName}`)
.toEqual(expected);
// Check thumbnail 4 and copy it with Ctrl+C.
await waitAndClick(
page,
`.thumbnail:has(${getThumbnailSelector(3)}) input`
);
handlePagesEdited = await waitForPagesEdited(page, "copy");
await kbCopy(page);
pageIndices = await awaitPromise(handlePagesEdited);
expected = [
2, 3, 1, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17,
];
expect(pageIndices)
.withContext(`In ${browserName}`)
.toEqual(expected);
// Paste before page 1.
handlePagesEdited = await waitForPagesEdited(page);
await waitAndClick(
page,
`button.thumbnailPasteButton:has(+ ${getThumbnailSelector(1)})`
);
pageIndices = await awaitPromise(handlePagesEdited);
expected = [
1, 2, 3, 1, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17,
];
expect(pageIndices)
.withContext(`In ${browserName}`)
.toEqual(expected);
// Check thumbnail 5 and delete it.
await waitAndClick(
page,
`.thumbnail:has(${getThumbnailSelector(4)}) input`
);
handlePagesEdited = await waitForPagesEdited(page);
await kbDelete(page);
pageIndices = await awaitPromise(handlePagesEdited);
expected = [
1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17,
];
expect(pageIndices)
.withContext(`In ${browserName}`)
.toEqual(expected);
})
);
});
@ -1967,4 +2084,483 @@ describe("Reorganize Pages View", () => {
);
});
});
describe("Dismissing undo after copy, paste and delete doesn't destroy the visible thumbnail", () => {
let pages;
beforeEach(async () => {
pages = await loadAndWait(
"two_pages.pdf",
"#viewsManagerToggleButton",
"page-fit",
null,
{ enableSplitMerge: true }
);
});
afterEach(async () => {
await closePages(pages);
});
it("should keep the pasted thumbnail visible after dismissing the undo bar", async () => {
await Promise.all(
pages.map(async ([browserName, page]) => {
await waitForThumbnailVisible(page, 1);
await page.waitForSelector("#viewsManagerStatusActionButton", {
visible: true,
});
// Wait for both thumbnails to be fully rendered.
for (let i = 1; i <= 2; i++) {
await page.waitForSelector(
`${getThumbnailSelector(i)} > img[src^="blob:http:"]`,
{ visible: true }
);
}
// Copy page 1.
await waitAndClick(
page,
`.thumbnail:has(${getThumbnailSelector(1)}) input`
);
let handlePagesEdited = await waitForPagesEdited(page, "copy");
await waitAndClick(page, "#viewsManagerStatusActionButton");
await waitAndClick(page, "#viewsManagerStatusActionCopy");
await awaitPromise(handlePagesEdited);
// Paste after page 1 (the pasted copy lands at position 2).
handlePagesEdited = await waitForPagesEdited(page);
await waitAndClick(page, `${getThumbnailSelector(1)}+button`);
await awaitPromise(handlePagesEdited);
// Select the original page 1 (still at position 1) and delete it.
await waitAndClick(
page,
`.thumbnail:has(${getThumbnailSelector(1)}) input`
);
handlePagesEdited = await waitForPagesEdited(page);
await waitAndClick(page, "#viewsManagerStatusActionButton");
await waitAndClick(page, "#viewsManagerStatusActionDelete");
await awaitPromise(handlePagesEdited);
await page.waitForSelector("#viewsManagerStatusUndo", {
visible: true,
});
// Dismiss the undo bar. Without the fix, this would destroy the
// thumbnail at position 1 (the pasted copy) because both the saved
// state and the current _thumbnails array pointed to the same object.
handlePagesEdited = await waitForPagesEdited(page, "cleanSavedData");
await waitAndClick(page, "#viewsManagerStatusUndoCloseButton");
await awaitPromise(handlePagesEdited);
// The thumbnail at position 1 (the pasted copy) must still be
// rendered and visible with a valid image source.
await page.waitForSelector(
`${getThumbnailSelector(1)} > img[src^="blob:http:"]`,
{ visible: true }
);
})
);
});
});
describe("Undo after drag-and-drop move followed by delete restores correct page mapping", () => {
let pages;
beforeEach(async () => {
pages = await loadAndWait(
"page_with_number.pdf",
"#viewsManagerToggleButton",
"1",
null,
{ enableSplitMerge: true }
);
});
afterEach(async () => {
await closePages(pages);
});
it("should restore the post-move mapping when undoing a delete", async () => {
await Promise.all(
pages.map(async ([browserName, page]) => {
await waitForThumbnailVisible(page, 1);
const rect1 = await getRect(page, getThumbnailSelector(1));
const rect2 = await getRect(page, getThumbnailSelector(2));
// Move page 1 after page 2: mapping becomes [2, 1, 3, …, 17].
let handlePagesEdited = await waitForPagesEdited(page);
await dragAndDrop(
page,
getThumbnailSelector(1),
[[0, rect2.y - rect1.y + rect2.height / 2]],
10
);
let pageIndices = await awaitPromise(handlePagesEdited);
let expected = [
2, 1, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17,
];
expect(pageIndices)
.withContext(`In ${browserName}`)
.toEqual(expected);
// Select the new first thumbnail (originally page 2) and delete it.
await waitAndClick(
page,
`.thumbnail:has(${getThumbnailSelector(1)}) input`
);
handlePagesEdited = await waitForPagesEdited(page);
await waitAndClick(page, "#viewsManagerStatusActionButton");
await waitAndClick(page, "#viewsManagerStatusActionDelete");
pageIndices = await awaitPromise(handlePagesEdited);
expected = [1, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17];
expect(pageIndices)
.withContext(`In ${browserName}`)
.toEqual(expected);
await page.waitForSelector("#viewsManagerStatusUndo", {
visible: true,
});
// Undo the delete. Without the fix, cancelDelete() would call the
// pagesNumber setter which triggers #reset(), wiping out the
// just-restored pageNumberToId and idToPageNumber mappings, leaving
// the mapper in identity state despite the prior move.
handlePagesEdited = await waitForPagesEdited(page, "cancelDelete");
await waitAndClick(page, "#viewsManagerStatusUndoButton");
pageIndices = await awaitPromise(handlePagesEdited);
expected = [
2, 1, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17,
];
expect(pageIndices)
.withContext(`In ${browserName}`)
.toEqual(expected);
await waitForHavingContents(page, expected);
})
);
});
});
describe("Keyboard Delete is ignored while in paste mode", () => {
let pages;
beforeEach(async () => {
pages = await loadAndWait(
"page_with_number.pdf",
"#viewsManagerToggleButton",
"1",
null,
{ enableSplitMerge: true }
);
});
afterEach(async () => {
await closePages(pages);
});
it("should not delete pages when pressing Delete while in paste mode", async () => {
await Promise.all(
pages.map(async ([browserName, page]) => {
await waitForThumbnailVisible(page, 1);
await page.waitForSelector("#viewsManagerStatusActionButton", {
visible: true,
});
// Copy page 1 to enter paste mode.
await waitAndClick(
page,
`.thumbnail:has(${getThumbnailSelector(1)}) input`
);
const handlePagesEdited = await waitForPagesEdited(page, "copy");
await waitAndClick(page, "#viewsManagerStatusActionButton");
await waitAndClick(page, "#viewsManagerStatusActionCopy");
await awaitPromise(handlePagesEdited);
// Paste mode must be active (paste buttons visible).
await page.waitForSelector("button.thumbnailPasteButton", {
visible: true,
});
// Focus an element inside the thumbnail container so that the
// keydown handler can fire. Checkboxes are hidden in paste mode
// (CSS: pasteMode > .thumbnail > input { display: none }), so we
// click the imageContainer itself instead.
await waitAndClick(page, getThumbnailSelector(1));
// Press Delete: must be a no-op while in paste mode.
await kbDelete(page);
await waitForBrowserTrip(page);
// The page count must remain at 17.
const pageCount = await page.$eval("#pageNumber", el => el.max);
expect(parseInt(pageCount, 10))
.withContext(`In ${browserName}`)
.toBe(17);
// Paste buttons must still be present (paste mode not disrupted).
const pasteButtons = await page.$$("button.thumbnailPasteButton");
expect(pasteButtons.length)
.withContext(`In ${browserName}`)
.toBeGreaterThan(0);
})
);
});
});
describe("Clicking Done after two copy+paste rounds must not remove the pasted pages (bug 2023934)", () => {
let pages;
beforeEach(async () => {
pages = await loadAndWait(
"page_with_number.pdf",
"#viewsManagerToggleButton",
"1",
null,
{ enableSplitMerge: true }
);
});
afterEach(async () => {
await closePages(pages);
});
it("should keep all pasted pages after clicking Done without pasting", async () => {
await Promise.all(
pages.map(async ([browserName, page]) => {
await waitForThumbnailVisible(page, 1);
await page.waitForSelector("#viewsManagerStatusActionButton", {
visible: true,
});
// Helper: copy page 1, click a paste button, await the paste event.
async function copyAndPaste() {
await waitAndClick(
page,
`.thumbnail:has(${getThumbnailSelector(1)}) input`
);
let handlePagesEdited = await waitForPagesEdited(page, "copy");
await waitAndClick(page, "#viewsManagerStatusActionButton");
await waitAndClick(page, "#viewsManagerStatusActionCopy");
await awaitPromise(handlePagesEdited);
handlePagesEdited = await waitForPagesEdited(page);
// Click the paste button that appears after thumbnail 1.
await waitAndClick(page, `${getThumbnailSelector(1)}+button`);
await awaitPromise(handlePagesEdited);
}
// Round 1: copy page 1 and paste → 18 pages.
await copyAndPaste();
let pageCount = await page.$eval("#pageNumber", el => el.max);
expect(parseInt(pageCount, 10))
.withContext(`In ${browserName} after 1st paste`)
.toBe(18);
// Round 2: copy page 1 and paste → 19 pages.
await copyAndPaste();
pageCount = await page.$eval("#pageNumber", el => el.max);
expect(parseInt(pageCount, 10))
.withContext(`In ${browserName} after 2nd paste`)
.toBe(19);
// STR step 4: select page 1, copy (enter paste mode), then click
// "Done" without pasting. Without the fix, clicking Done also
// triggers a restore of the saved pre-paste thumbnails, which removes
// one of the pasted pages and corrupts the state.
await waitAndClick(
page,
`.thumbnail:has(${getThumbnailSelector(1)}) input`
);
const handlePagesEdited = await waitForPagesEdited(page, "copy");
await waitAndClick(page, "#viewsManagerStatusActionButton");
await waitAndClick(page, "#viewsManagerStatusActionCopy");
await awaitPromise(handlePagesEdited);
const handleCancelCopy = await waitForPagesEdited(page, "cancelCopy");
await waitAndClick(page, "#viewsManagerStatusUndoButton");
await awaitPromise(handleCancelCopy);
// Page count must still be 19 Done should not have removed a page.
pageCount = await page.$eval("#pageNumber", el => el.max);
expect(parseInt(pageCount, 10))
.withContext(`In ${browserName} after Done`)
.toBe(19);
// STR step 5: copy page 1 and paste → must reach 20 without crash.
await copyAndPaste();
pageCount = await page.$eval("#pageNumber", el => el.max);
expect(parseInt(pageCount, 10))
.withContext(`In ${browserName} after 3rd paste`)
.toBe(20);
// Sidebar must still show all 20 thumbnails.
const thumbnails = await page.$$(
".thumbnail .thumbnailImageContainer"
);
expect(thumbnails.length)
.withContext(`In ${browserName} thumbnail count`)
.toBe(20);
})
);
});
});
describe("Selection counter must be cleared after drag-and-drop (bug 2022884)", () => {
let pages;
beforeEach(async () => {
pages = await loadAndWait(
"page_with_number.pdf",
"#viewsManagerToggleButton",
"1",
null,
{ enableSplitMerge: true }
);
});
afterEach(async () => {
await closePages(pages);
});
it("should hide the selection counter after dropping selected pages", async () => {
await Promise.all(
pages.map(async ([browserName, page]) => {
await waitForThumbnailVisible(page, 1);
// Select page 1 via its checkbox.
await waitAndClick(
page,
`.thumbnail:has(${getThumbnailSelector(1)}) input`
);
// The deselect button must now be visible ("1 page selected").
await page.waitForSelector(
"#viewsManagerStatusActionDeselectButton:not(.hidden)",
{ visible: true }
);
// Capture rects after the status-bar layout has settled so the
// drag displacement accounts for the correct positions.
const rect1 = await getRect(page, getThumbnailSelector(1));
const rect2 = await getRect(page, getThumbnailSelector(2));
// Drag page 1 to after page 2 (short displacement, stays in
// viewport).
const handlePagesEdited = await waitForPagesEdited(page);
await dragAndDrop(
page,
getThumbnailSelector(1),
[[0, rect2.y - rect1.y + rect2.height / 2]],
10
);
await awaitPromise(handlePagesEdited);
// After the drop the selection must be cleared: the deselect button
// must be hidden and the label must show the "no selection" state.
// Without the fix, the "2 selected" counter persists.
const deselectHidden = await page.$eval(
"#viewsManagerStatusActionDeselectButton",
el => el.classList.contains("hidden")
);
expect(deselectHidden)
.withContext(`In ${browserName}: deselect button hidden`)
.toBeTrue();
const labelId = await page.$eval(
"#viewsManagerStatusActionLabel",
el => el.getAttribute("data-l10n-id")
);
expect(labelId)
.withContext(`In ${browserName}: label l10n-id`)
.toBe("pdfjs-views-manager-pages-status-none-action-label");
})
);
});
});
describe("Undo bar must disappear and UI must recover after delete+undo (bug 2022824)", () => {
let pages;
beforeEach(async () => {
pages = await loadAndWait(
"page_with_number.pdf",
"#viewsManagerToggleButton",
"1",
null,
{ enableSplitMerge: true }
);
});
afterEach(async () => {
await closePages(pages);
});
it("should hide the undo bar and restore the action bar after undoing a delete", async () => {
await Promise.all(
pages.map(async ([browserName, page]) => {
await waitForThumbnailVisible(page, 1);
await page.waitForSelector("#viewsManagerStatusActionButton", {
visible: true,
});
// Select page 1 and delete it.
await waitAndClick(
page,
`.thumbnail:has(${getThumbnailSelector(1)}) input`
);
let handlePagesEdited = await waitForPagesEdited(page);
await waitAndClick(page, "#viewsManagerStatusActionButton");
await waitAndClick(page, "#viewsManagerStatusActionDelete");
await awaitPromise(handlePagesEdited);
// Undo bar must be visible with the "Undo" button.
await page.waitForSelector("#viewsManagerStatusUndo", {
visible: true,
});
// Click Undo.
handlePagesEdited = await waitForPagesEdited(page, "cancelDelete");
await waitAndClick(page, "#viewsManagerStatusUndoButton");
await awaitPromise(handlePagesEdited);
// Without the fix, the undo bar stays visible and buttons are
// unclickable. With the fix, the undo bar must be gone and the
// normal action bar must be back.
await page.waitForSelector("#viewsManagerStatusUndo", {
hidden: true,
});
await page.waitForSelector("#viewsManagerStatusAction", {
visible: true,
});
// The page count must be back to 17.
const pageCount = await page.$eval("#pageNumber", el => el.max);
expect(parseInt(pageCount, 10))
.withContext(`In ${browserName}`)
.toBe(17);
// The action button must be interactable: select a page and delete
// it again to confirm the full state is restored.
await waitAndClick(
page,
`.thumbnail:has(${getThumbnailSelector(1)}) input`
);
handlePagesEdited = await waitForPagesEdited(page);
await waitAndClick(page, "#viewsManagerStatusActionButton");
await waitAndClick(page, "#viewsManagerStatusActionDelete");
await awaitPromise(handlePagesEdited);
const pageCountAfterSecondDelete = await page.$eval(
"#pageNumber",
el => el.max
);
expect(parseInt(pageCountAfterSecondDelete, 10))
.withContext(`In ${browserName} after second delete`)
.toBe(16);
})
);
});
});
});

View File

@ -303,7 +303,7 @@ class PDFPageView extends BasePDFPageView {
l10n: this.l10n,
clonedFrom: this,
});
clone.setPdfPage(this.pdfPage);
clone.setPdfPage(this.pdfPage.clone(id - 1));
return clone;
}

View File

@ -80,6 +80,8 @@ class TempImageFactory {
class PDFThumbnailView extends RenderableView {
#renderingState = RenderingStates.INITIAL;
static foo = 0;
/**
* @param {PDFThumbnailViewOptions} options
*/
@ -97,6 +99,7 @@ class PDFThumbnailView extends RenderableView {
enableSplitMerge = false,
}) {
super();
this.foo = PDFThumbnailView.foo++;
this.id = id;
this.renderingId = `thumbnail${id}`;
this.pageLabel = null;
@ -166,7 +169,6 @@ class PDFThumbnailView extends RenderableView {
pageColors: this.pageColors,
enableSplitMerge: !!this.checkbox,
});
thumbnailView.setPdfPage(this.pdfPage);
const { imageContainer } = this;
if (!imageContainer.classList.contains("missingThumbnailImage")) {
thumbnailView.image.replaceWith(this.image.cloneNode(true));

View File

@ -157,6 +157,8 @@ class PDFThumbnailViewer {
#isInPasteMode = false;
#hasUndoBarVisible = false;
/**
* @param {PDFThumbnailViewerOptions} options
*/
@ -277,7 +279,7 @@ class PDFThumbnailViewer {
this.#undoButton?.addEventListener("click", this.#undo.bind(this));
this.#undoCloseButton?.addEventListener(
"click",
this.#dismissUndo.bind(this)
this.#dismissUndo.bind(this, /* mustUpdateStatus = */ true)
);
this.#deselectButton?.addEventListener("click", () => {
this.#clearSelection();
@ -693,7 +695,8 @@ class PDFThumbnailViewer {
selectedPages.clear();
this.#pageNumberToRemove = NaN;
this.#updateMenuEntries();
this.#toggleMenuEntries(false);
this.#updateStatus("select");
this.eventBus.dispatch("pagesedited", {
source: this,
@ -728,13 +731,13 @@ class PDFThumbnailViewer {
}
#undo() {
this.#clearSelection();
this.#toggleMenuEntries(false);
this.#updateStatus("select");
if (this.#copiedThumbnails) {
// We undo a copy or a cut.
this.#copiedThumbnails = null;
this.#pagesMapper.cancelCopy();
this.#clearSelection();
this.#toggleMenuEntries(false);
this.#updateStatus("select");
this.#togglePasteMode(false);
this.eventBus.dispatch("pagesedited", {
@ -766,17 +769,21 @@ class PDFThumbnailViewer {
}
}
#dismissUndo() {
#dismissUndo(mustUpdateStatus) {
this.#copiedThumbnails = null;
if (this.#deletedPageNumbers) {
for (const pageNumber of this.#deletedPageNumbers) {
this.#savedThumbnails[pageNumber - 1].destroy();
if (this.#savedThumbnails) {
for (const pageNumber of this.#deletedPageNumbers) {
this.#savedThumbnails[pageNumber - 1].destroy();
}
this.#savedThumbnails = null;
}
this.#deletedPageNumbers = null;
this.#savedThumbnails = null;
}
this.#isCut = false;
this.#updateStatus("select");
if (mustUpdateStatus) {
this.#updateStatus("select");
}
this.#togglePasteMode(false);
this.#pagesMapper.cleanSavedData();
@ -817,6 +824,12 @@ class PDFThumbnailViewer {
}
#copyPages(clearSelection = true) {
if (!this.#isCut) {
// Entering pure copy mode "commits" any pending paste/delete state so
// that clicking the "Done" button later only cancels the copy and does
// not accidentally restore a previous paste or delete.
this.#savedThumbnails = null;
}
this.#updateStatus(this.#isCut ? "cut" : "copy");
const pageNumbersToCopy = (this.#copiedPageNumbers = Uint32Array.from(
this.#selectedPages
@ -860,6 +873,7 @@ class PDFThumbnailViewer {
pagesMapper.pastePages(index);
this.#updateCurrentPage(this.#updateThumbnails(currentPageNumber));
this.#computeThumbnailsPosition();
this.eventBus.dispatch("pagesedited", {
source: this,
@ -944,6 +958,7 @@ class PDFThumbnailViewer {
}
this.#statusBar.classList.toggle("hidden", false);
this.#undoBar.classList.toggle("hidden", true);
this.#hasUndoBarVisible = false;
return;
}
@ -978,6 +993,7 @@ class PDFThumbnailViewer {
this.#statusBar.classList.toggle("hidden", true);
this.#undoBar.classList.toggle("hidden", false);
this.#hasUndoBarVisible = true;
}
#moveDraggedContainer(dx, dy) {
@ -1194,7 +1210,11 @@ class PDFThumbnailViewer {
break;
case "Delete":
case "Backspace":
if (this.#enableSplitMerge && this.#selectedPages?.size) {
if (
this.#enableSplitMerge &&
!this.#isInPasteMode &&
this.#selectedPages?.size
) {
this.#deletePages();
stopEvent(e);
}
@ -1217,12 +1237,16 @@ class PDFThumbnailViewer {
}
#selectPage(pageNumber, checked) {
if (this.#hasUndoBarVisible) {
this.#dismissUndo(/* mustUpdateStatus = */ false);
}
const set = (this.#selectedPages ??= new Set());
if (checked) {
set.add(pageNumber);
} else {
set.delete(pageNumber);
}
this.#updateMenuEntries();
this.#updateStatus("select");
}

View File

@ -1202,6 +1202,10 @@ class PDFViewer {
}
if (type === "cancelDelete") {
this.#deletedPageNumbers = null;
if (!this.#savedPageViews) {
return;
}
const viewerElement =
this._scrollMode === ScrollMode.PAGE ? null : this.viewer;
if (viewerElement) {
@ -1215,16 +1219,19 @@ class PDFViewer {
}
this._pages = this.#savedPageViews;
this.#savedPageViews = null;
this.#deletedPageNumbers = null;
return;
}
if (type === "cleanSavedData") {
for (const pageNum of this.#deletedPageNumbers) {
this.#savedPageViews[pageNum - 1].deleteMe();
if (this.#deletedPageNumbers) {
if (this.#savedPageViews) {
for (const pageNum of this.#deletedPageNumbers) {
this.#savedPageViews[pageNum - 1].deleteMe();
}
this.#savedPageViews = null;
}
this.#deletedPageNumbers = null;
}
this.#savedPageViews = null;
this.#deletedPageNumbers = null;
return;
}
@ -1248,7 +1255,7 @@ class PDFViewer {
page.updatePageNumber(i);
}
if (!isCut) {
if (type === "paste") {
this.#copiedPageViews = null;
}