Skip to content

Commit fbfcceb

Browse files
authored
Merge pull request #20850 from calixteman/bug2021828
Don't let the user delete/cut all the pages (bug 2021828)
2 parents b7698d6 + 1291f5a commit fbfcceb

2 files changed

Lines changed: 220 additions & 45 deletions

File tree

test/integration/reorganize_pages_spec.mjs

Lines changed: 180 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -967,6 +967,42 @@ describe("Reorganize Pages View", () => {
967967
})
968968
);
969969
});
970+
971+
it("should disable delete and cut entries when all pages are selected", async () => {
972+
await Promise.all(
973+
pages.map(async ([browserName, page]) => {
974+
await waitForThumbnailVisible(page, 1);
975+
976+
// Select all pages.
977+
const totalPages = await page.evaluate(
978+
() =>
979+
document.querySelectorAll("#thumbnailsView .thumbnail input")
980+
.length
981+
);
982+
for (let i = 1; i <= totalPages; i++) {
983+
await waitAndClick(
984+
page,
985+
`.thumbnail:has(${getThumbnailSelector(i)}) input`
986+
);
987+
}
988+
989+
await waitAndClick(page, "#viewsManagerStatusActionButton");
990+
991+
await page.waitForSelector(
992+
"#viewsManagerStatusActionDelete:disabled"
993+
);
994+
await page.waitForSelector("#viewsManagerStatusActionCut:disabled");
995+
await page.waitForSelector(
996+
"#viewsManagerStatusActionCopy:not(:disabled)"
997+
);
998+
await page.waitForSelector(
999+
"#viewsManagerStatusActionSaveAs:not(:disabled)"
1000+
);
1001+
1002+
await page.keyboard.press("Escape");
1003+
})
1004+
);
1005+
});
9701006
});
9711007

9721008
describe("Keyboard shortcuts for cut and copy (bug 2018139)", () => {
@@ -1746,4 +1782,148 @@ describe("Reorganize Pages View", () => {
17461782
);
17471783
});
17481784
});
1785+
1786+
describe("Context menu triggers editingstateschanged event (bug 2021828)", () => {
1787+
let pages;
1788+
1789+
beforeEach(async () => {
1790+
pages = await loadAndWait(
1791+
"page_with_number.pdf",
1792+
"#viewsManagerToggleButton",
1793+
"1",
1794+
null,
1795+
{ enableSplitMerge: true }
1796+
);
1797+
});
1798+
1799+
afterEach(async () => {
1800+
await closePages(pages);
1801+
});
1802+
1803+
function getContextMenuPromise(page) {
1804+
return createPromise(page, resolve => {
1805+
window.addEventListener(
1806+
"contextmenu",
1807+
e => {
1808+
e.preventDefault();
1809+
resolve();
1810+
},
1811+
{ once: true }
1812+
);
1813+
});
1814+
}
1815+
1816+
it("should dispatch editingstateschanged with correct payload on right-click with no selection", async () => {
1817+
await Promise.all(
1818+
pages.map(async ([browserName, page]) => {
1819+
await waitForThumbnailVisible(page, 1);
1820+
1821+
const handleEditingStatesChanged = await createPromise(
1822+
page,
1823+
resolve => {
1824+
window.PDFViewerApplication.eventBus.on(
1825+
"editingstateschanged",
1826+
({ details }) => resolve(details),
1827+
{ once: true }
1828+
);
1829+
}
1830+
);
1831+
1832+
const contextMenuPromise = await getContextMenuPromise(page);
1833+
await page.click(getThumbnailSelector(1), { button: "right" });
1834+
await awaitPromise(contextMenuPromise);
1835+
1836+
const details = await awaitPromise(handleEditingStatesChanged);
1837+
expect(details.thumbnailId).withContext(`In ${browserName}`).toBe(1);
1838+
expect(details.hasSelectedPages)
1839+
.withContext(`In ${browserName}`)
1840+
.toBeFalse();
1841+
expect(details.canDeletePages)
1842+
.withContext(`In ${browserName}`)
1843+
.toBeFalse();
1844+
})
1845+
);
1846+
});
1847+
1848+
it("should dispatch editingstateschanged with correct payload on right-click with some pages selected", async () => {
1849+
await Promise.all(
1850+
pages.map(async ([browserName, page]) => {
1851+
await waitForThumbnailVisible(page, 1);
1852+
await waitAndClick(
1853+
page,
1854+
`.thumbnail:has(${getThumbnailSelector(1)}) input`
1855+
);
1856+
1857+
const handleEditingStatesChanged = await createPromise(
1858+
page,
1859+
resolve => {
1860+
window.PDFViewerApplication.eventBus.on(
1861+
"editingstateschanged",
1862+
({ details }) => resolve(details),
1863+
{ once: true }
1864+
);
1865+
}
1866+
);
1867+
1868+
const contextMenuPromise = await getContextMenuPromise(page);
1869+
await page.click(getThumbnailSelector(1), { button: "right" });
1870+
await awaitPromise(contextMenuPromise);
1871+
1872+
const details = await awaitPromise(handleEditingStatesChanged);
1873+
expect(details.thumbnailId).withContext(`In ${browserName}`).toBe(1);
1874+
expect(details.hasSelectedPages)
1875+
.withContext(`In ${browserName}`)
1876+
.toBeTrue();
1877+
expect(details.canDeletePages)
1878+
.withContext(`In ${browserName}`)
1879+
.toBeTrue();
1880+
})
1881+
);
1882+
});
1883+
1884+
it("should dispatch editingstateschanged with canDeletePages false when all pages are selected", async () => {
1885+
await Promise.all(
1886+
pages.map(async ([browserName, page]) => {
1887+
await waitForThumbnailVisible(page, 1);
1888+
1889+
// Select all 17 pages.
1890+
const totalPages = await page.evaluate(
1891+
() =>
1892+
document.querySelectorAll("#thumbnailsView .thumbnail input")
1893+
.length
1894+
);
1895+
for (let i = 1; i <= totalPages; i++) {
1896+
await waitAndClick(
1897+
page,
1898+
`.thumbnail:has(${getThumbnailSelector(i)}) input`
1899+
);
1900+
}
1901+
1902+
const handleEditingStatesChanged = await createPromise(
1903+
page,
1904+
resolve => {
1905+
window.PDFViewerApplication.eventBus.on(
1906+
"editingstateschanged",
1907+
({ details }) => resolve(details),
1908+
{ once: true }
1909+
);
1910+
}
1911+
);
1912+
1913+
const contextMenuPromise = await getContextMenuPromise(page);
1914+
await page.click(getThumbnailSelector(1), { button: "right" });
1915+
await awaitPromise(contextMenuPromise);
1916+
1917+
const details = await awaitPromise(handleEditingStatesChanged);
1918+
expect(details.thumbnailId).withContext(`In ${browserName}`).toBe(1);
1919+
expect(details.hasSelectedPages)
1920+
.withContext(`In ${browserName}`)
1921+
.toBeTrue();
1922+
expect(details.canDeletePages)
1923+
.withContext(`In ${browserName}`)
1924+
.toBeFalse();
1925+
})
1926+
);
1927+
});
1928+
});
17491929
});

web/pdf_thumbnail_viewer.js

Lines changed: 40 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -141,10 +141,6 @@ class PDFThumbnailViewer {
141141

142142
#scrollableContainerHeight = 0;
143143

144-
#previousStates = {
145-
hasSelectedPages: false,
146-
};
147-
148144
#statusLabel = null;
149145

150146
#statusBar = null;
@@ -236,6 +232,29 @@ class PDFThumbnailViewer {
236232
}
237233
});
238234

235+
this.container.addEventListener(
236+
"contextmenu",
237+
e => {
238+
this.eventBus.dispatch("editingstateschanged", {
239+
source: this,
240+
details: {
241+
thumbnailId:
242+
parseInt(
243+
e.target
244+
.closest(".thumbnailImageContainer")
245+
?.parentElement.getAttribute("page-number")
246+
) ?? -1,
247+
hasSelectedPages: !!this.#selectedPages?.size,
248+
canDeletePages: this.#canDelete(),
249+
},
250+
});
251+
},
252+
{
253+
signal: abortSignal,
254+
passive: true,
255+
}
256+
);
257+
239258
this.#undoButton?.addEventListener("click", this.#undo.bind(this));
240259
this.#undoCloseButton?.addEventListener(
241260
"click",
@@ -254,24 +273,6 @@ class PDFThumbnailViewer {
254273
this.#addEventListeners();
255274
}
256275

257-
/**
258-
* Update the different possible states of this manager, e.g. is there
259-
* something to copy, paste, ...
260-
* @param {Object} details
261-
*/
262-
#dispatchUpdateStates(details) {
263-
const hasChanged = Object.entries(details).some(
264-
([key, value]) => this.#previousStates[key] !== value
265-
);
266-
267-
if (hasChanged) {
268-
this.eventBus.dispatch("editingstateschanged", {
269-
source: this,
270-
details: Object.assign(this.#previousStates, details),
271-
});
272-
}
273-
}
274-
275276
#scrollUpdated() {
276277
this.renderingQueue.renderHighestPriority();
277278
}
@@ -759,6 +760,11 @@ class PDFThumbnailViewer {
759760
});
760761
}
761762

763+
#canDelete() {
764+
const size = this.#selectedPages?.size || 0;
765+
return size > 0 && size < this._thumbnails.length;
766+
}
767+
762768
#togglePasteMode(enable) {
763769
this.#isInPasteMode = enable;
764770
if (enable) {
@@ -808,6 +814,10 @@ class PDFThumbnailViewer {
808814
}
809815

810816
#cutPages() {
817+
if (!this.#canDelete()) {
818+
return;
819+
}
820+
811821
this.#isCut = true;
812822
this.#copyPages(false);
813823
this.#deletePages(/* type = */ "cut");
@@ -839,10 +849,11 @@ class PDFThumbnailViewer {
839849
}
840850

841851
#deletePages(type = "delete") {
842-
const selectedPages = this.#selectedPages;
843-
if (selectedPages.size === 0) {
852+
if (!this.#canDelete()) {
844853
return;
845854
}
855+
856+
const selectedPages = this.#selectedPages;
846857
if (type === "delete") {
847858
this.#updateStatus("delete");
848859
}
@@ -868,14 +879,10 @@ class PDFThumbnailViewer {
868879
}
869880

870881
#updateMenuEntries() {
871-
this.#manageSaveAsButton.disabled =
872-
this.#manageDeleteButton.disabled =
873-
this.#manageCopyButton.disabled =
874-
this.#manageCutButton.disabled =
875-
!this.#selectedPages?.size;
876-
this.#dispatchUpdateStates({
877-
hasSelectedPages: !!this.#selectedPages?.size,
878-
});
882+
const size = this.#selectedPages?.size || 0;
883+
this.#manageSaveAsButton.disabled = this.#manageCopyButton.disabled = !size;
884+
this.#manageDeleteButton.disabled = this.#manageCutButton.disabled =
885+
!this.#canDelete();
879886
}
880887

881888
#toggleMenuEntries(enable) {
@@ -884,9 +891,6 @@ class PDFThumbnailViewer {
884891
this.#manageCopyButton.disabled =
885892
this.#manageCutButton.disabled =
886893
!enable;
887-
this.#dispatchUpdateStates({
888-
hasSelectedPages: false,
889-
});
890894
}
891895

892896
#updateStatus(type) {
@@ -1102,16 +1106,6 @@ class PDFThumbnailViewer {
11021106
this.#computeThumbnailsPosition();
11031107
}
11041108
});
1105-
this.container.addEventListener("focusout", () => {
1106-
this.#dispatchUpdateStates({
1107-
hasSelectedPages: false,
1108-
});
1109-
});
1110-
this.container.addEventListener("focusin", () => {
1111-
this.#dispatchUpdateStates({
1112-
hasSelectedPages: !!this.#selectedPages?.size,
1113-
});
1114-
});
11151109
this.container.addEventListener("keydown", e => {
11161110
const { target } = e;
11171111
const isCheckbox =
@@ -1218,6 +1212,7 @@ class PDFThumbnailViewer {
12181212
if (
12191213
e.button !== 0 || // Skip right click.
12201214
this.#isInPasteMode ||
1215+
this._thumbnails.length === 1 ||
12211216
!isNaN(this.#lastDraggedOverIndex) ||
12221217
!draggedImage.classList.contains("thumbnailImageContainer")
12231218
) {

0 commit comments

Comments
 (0)