Skip to content

Commit 1291f5a

Browse files
committed
Don't let the user delete/cut all the pages (bug 2021828)
And dispatch an event only when the context menu is displayed.
1 parent 98f7e85 commit 1291f5a

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
@@ -143,10 +143,6 @@ class PDFThumbnailViewer {
143143

144144
#scrollableContainerHeight = 0;
145145

146-
#previousStates = {
147-
hasSelectedPages: false,
148-
};
149-
150146
#statusLabel = null;
151147

152148
#statusBar = null;
@@ -240,6 +236,29 @@ class PDFThumbnailViewer {
240236
}
241237
});
242238

239+
this.container.addEventListener(
240+
"contextmenu",
241+
e => {
242+
this.eventBus.dispatch("editingstateschanged", {
243+
source: this,
244+
details: {
245+
thumbnailId:
246+
parseInt(
247+
e.target
248+
.closest(".thumbnailImageContainer")
249+
?.parentElement.getAttribute("page-number")
250+
) ?? -1,
251+
hasSelectedPages: !!this.#selectedPages?.size,
252+
canDeletePages: this.#canDelete(),
253+
},
254+
});
255+
},
256+
{
257+
signal: abortSignal,
258+
passive: true,
259+
}
260+
);
261+
243262
this.#undoButton?.addEventListener("click", this.#undo.bind(this));
244263
this.#undoCloseButton?.addEventListener(
245264
"click",
@@ -258,24 +277,6 @@ class PDFThumbnailViewer {
258277
this.#addEventListeners();
259278
}
260279

261-
/**
262-
* Update the different possible states of this manager, e.g. is there
263-
* something to copy, paste, ...
264-
* @param {Object} details
265-
*/
266-
#dispatchUpdateStates(details) {
267-
const hasChanged = Object.entries(details).some(
268-
([key, value]) => this.#previousStates[key] !== value
269-
);
270-
271-
if (hasChanged) {
272-
this.eventBus.dispatch("editingstateschanged", {
273-
source: this,
274-
details: Object.assign(this.#previousStates, details),
275-
});
276-
}
277-
}
278-
279280
#scrollUpdated() {
280281
this.renderingQueue.renderHighestPriority();
281282
}
@@ -764,6 +765,11 @@ class PDFThumbnailViewer {
764765
});
765766
}
766767

768+
#canDelete() {
769+
const size = this.#selectedPages?.size || 0;
770+
return size > 0 && size < this._thumbnails.length;
771+
}
772+
767773
#togglePasteMode(enable) {
768774
this.#isInPasteMode = enable;
769775
if (enable) {
@@ -813,6 +819,10 @@ class PDFThumbnailViewer {
813819
}
814820

815821
#cutPages() {
822+
if (!this.#canDelete()) {
823+
return;
824+
}
825+
816826
this.#isCut = true;
817827
this.#copyPages(false);
818828
this.#deletePages(/* type = */ "cut");
@@ -844,10 +854,11 @@ class PDFThumbnailViewer {
844854
}
845855

846856
#deletePages(type = "delete") {
847-
const selectedPages = this.#selectedPages;
848-
if (selectedPages.size === 0) {
857+
if (!this.#canDelete()) {
849858
return;
850859
}
860+
861+
const selectedPages = this.#selectedPages;
851862
if (type === "delete") {
852863
this.#updateStatus("delete");
853864
}
@@ -873,14 +884,10 @@ class PDFThumbnailViewer {
873884
}
874885

875886
#updateMenuEntries() {
876-
this.#manageSaveAsButton.disabled =
877-
this.#manageDeleteButton.disabled =
878-
this.#manageCopyButton.disabled =
879-
this.#manageCutButton.disabled =
880-
!this.#selectedPages?.size;
881-
this.#dispatchUpdateStates({
882-
hasSelectedPages: !!this.#selectedPages?.size,
883-
});
887+
const size = this.#selectedPages?.size || 0;
888+
this.#manageSaveAsButton.disabled = this.#manageCopyButton.disabled = !size;
889+
this.#manageDeleteButton.disabled = this.#manageCutButton.disabled =
890+
!this.#canDelete();
884891
}
885892

886893
#toggleMenuEntries(enable) {
@@ -889,9 +896,6 @@ class PDFThumbnailViewer {
889896
this.#manageCopyButton.disabled =
890897
this.#manageCutButton.disabled =
891898
!enable;
892-
this.#dispatchUpdateStates({
893-
hasSelectedPages: false,
894-
});
895899
}
896900

897901
#updateStatus(type) {
@@ -1107,16 +1111,6 @@ class PDFThumbnailViewer {
11071111
this.#computeThumbnailsPosition();
11081112
}
11091113
});
1110-
this.container.addEventListener("focusout", () => {
1111-
this.#dispatchUpdateStates({
1112-
hasSelectedPages: false,
1113-
});
1114-
});
1115-
this.container.addEventListener("focusin", () => {
1116-
this.#dispatchUpdateStates({
1117-
hasSelectedPages: !!this.#selectedPages?.size,
1118-
});
1119-
});
11201114
this.container.addEventListener("keydown", e => {
11211115
const { target } = e;
11221116
const isCheckbox =
@@ -1223,6 +1217,7 @@ class PDFThumbnailViewer {
12231217
if (
12241218
e.button !== 0 || // Skip right click.
12251219
this.#isInPasteMode ||
1220+
this._thumbnails.length === 1 ||
12261221
!isNaN(this.#lastDraggedOverIndex) ||
12271222
!draggedImage.classList.contains("thumbnailImageContainer")
12281223
) {

0 commit comments

Comments
 (0)