From fe7a2a5ac7c2aa7560b70fd52daa5acbe2ebd58b Mon Sep 17 00:00:00 2001 From: Anthony Stirling <77850077+Frooodle@users.noreply.github.com> Date: Fri, 19 Jun 2026 15:19:51 +0100 Subject: [PATCH] Fix Multi Tool page rotation lost on save (#6733) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description of Changes Rotating a page in the Multi Tool and saving could leave the page at its original rotation (the change appeared lost), with inconsistent results across pages. - Page rotation is now always written on export, including 0°, so rotating a page that already had a non-zero rotation in the source PDF (e.g. a 270° page rotated back to upright) is no longer dropped. - Per-page rotation is always read when building the Multi Tool document, so pages keep their true orientation regardless of file size. - Rotation is only applied after a page imports successfully, avoiding a misaligned or failed export when an import fails. --- ## Checklist ### General - [ ] I have read the [Contribution Guidelines](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/CONTRIBUTING.md) - [ ] I have read the [Stirling-PDF Developer Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/DeveloperGuide.md) (if applicable) - [ ] I have read the [How to add new languages to Stirling-PDF](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/HowToAddNewLanguage.md) (if applicable) - [x] I have performed a self-review of my own code - [x] My changes generate no new warnings ### Documentation - [ ] I have updated relevant docs on [Stirling-PDF's doc repo](https://github.com/Stirling-Tools/Stirling-Tools.github.io/blob/main/docs/) (if functionality has heavily changed) - [ ] I have read the section [Add New Translation Tags](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/HowToAddNewLanguage.md#add-new-translation-tags) (for new translation tags only) ### Translations (if applicable) - [ ] I ran [`scripts/counter_translation.py`](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/docs/counter_translation.md) ### UI Changes (if applicable) - [ ] Screenshots or videos demonstrating the UI changes are attached (e.g., as comments or direct attachments in the PR) ### Testing (if applicable) - [ ] I have run `task check` to verify linters, typechecks, and tests pass - [ ] I have tested my changes locally. Refer to the [Testing Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/DeveloperGuide.md#7-testing) for more details. --- .../components/shared/HoverActionMenu.tsx | 1 + .../src/core/services/pdfExportService.ts | 50 +++++---- .../stubbed/page-editor-rotation.spec.ts | 97 ++++++++++++++++++ .../tests/test-fixtures/rotated-pages.pdf | Bin 0 -> 2829 bytes .../editor/src/core/utils/thumbnailUtils.ts | 5 +- 5 files changed, 129 insertions(+), 24 deletions(-) create mode 100644 frontend/editor/src/core/tests/stubbed/page-editor-rotation.spec.ts create mode 100644 frontend/editor/src/core/tests/test-fixtures/rotated-pages.pdf diff --git a/frontend/editor/src/core/components/shared/HoverActionMenu.tsx b/frontend/editor/src/core/components/shared/HoverActionMenu.tsx index f4cfb247a1..fafebdf8f2 100644 --- a/frontend/editor/src/core/components/shared/HoverActionMenu.tsx +++ b/frontend/editor/src/core/components/shared/HoverActionMenu.tsx @@ -64,6 +64,7 @@ const HoverActionMenu: React.FC = ({ disabled={action.disabled} onClick={action.onClick} c={action.color} + aria-label={action.label} style={{ color: action.color || "var(--text-secondary)" }} data-tour={action.dataTour} > diff --git a/frontend/editor/src/core/services/pdfExportService.ts b/frontend/editor/src/core/services/pdfExportService.ts index da7bbc9a0b..1883d9efc9 100644 --- a/frontend/editor/src/core/services/pdfExportService.ts +++ b/frontend/editor/src/core/services/pdfExportService.ts @@ -135,11 +135,12 @@ export class PDFExportService { if (page.isBlankPage || page.originalPageNumber === -1) { // Insert a blank A4 page await addNewPage(destDocPtr, insertIdx, A4_WIDTH, A4_HEIGHT); - // Apply rotation - const pdfiumRotation = degreesToPdfiumRotation(page.rotation); - if (pdfiumRotation !== 0) { - await setPageRotation(destDocPtr, insertIdx, pdfiumRotation); - } + // Set absolute rotation (incl. 0) so editor rotation wins over source. + await setPageRotation( + destDocPtr, + insertIdx, + degreesToPdfiumRotation(page.rotation), + ); insertIdx++; } else if (page.originalFileId && loadedDocs.has(page.originalFileId)) { const srcDocPtr = loadedDocs.get(page.originalFileId)!; @@ -155,17 +156,18 @@ export class PDFExportService { pageRange, insertIdx, ); - if (!imported) { + if (imported) { + // Set absolute rotation (incl. 0) so editor rotation wins over source. + await setPageRotation( + destDocPtr, + insertIdx, + degreesToPdfiumRotation(page.rotation), + ); + } else { console.warn( `[PDFExport] importPages failed for fileId=${page.originalFileId} pageRange=${pageRange} — page will be missing from output.`, ); } - - // Apply rotation - const pdfiumRotation = degreesToPdfiumRotation(page.rotation); - if (pdfiumRotation !== 0) { - await setPageRotation(destDocPtr, insertIdx, pdfiumRotation); - } insertIdx++; } } else { @@ -211,10 +213,12 @@ export class PDFExportService { for (const page of pages) { if (page.isBlankPage || page.originalPageNumber === -1) { await addNewPage(destDocPtr, insertIdx, A4_WIDTH, A4_HEIGHT); - const pdfiumRotation = degreesToPdfiumRotation(page.rotation); - if (pdfiumRotation !== 0) { - await setPageRotation(destDocPtr, insertIdx, pdfiumRotation); - } + // Set absolute rotation (incl. 0) so editor rotation wins over source. + await setPageRotation( + destDocPtr, + insertIdx, + degreesToPdfiumRotation(page.rotation), + ); insertIdx++; } else { const sourcePageIndex = page.originalPageNumber - 1; @@ -227,16 +231,18 @@ export class PDFExportService { pageRange, insertIdx, ); - if (!imported) { + if (imported) { + // Set absolute rotation (incl. 0) so editor rotation wins over source. + await setPageRotation( + destDocPtr, + insertIdx, + degreesToPdfiumRotation(page.rotation), + ); + } else { console.warn( `[PDFExport] importPages failed for page ${page.originalPageNumber} pageRange=${pageRange} — page will be missing from output.`, ); } - - const pdfiumRotation = degreesToPdfiumRotation(page.rotation); - if (pdfiumRotation !== 0) { - await setPageRotation(destDocPtr, insertIdx, pdfiumRotation); - } insertIdx++; } } diff --git a/frontend/editor/src/core/tests/stubbed/page-editor-rotation.spec.ts b/frontend/editor/src/core/tests/stubbed/page-editor-rotation.spec.ts new file mode 100644 index 0000000000..5912c89211 --- /dev/null +++ b/frontend/editor/src/core/tests/stubbed/page-editor-rotation.spec.ts @@ -0,0 +1,97 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +import { PDFDocument } from "@cantoo/pdf-lib"; + +import { test, expect } from "@app/tests/helpers/stub-test-base"; +import { uploadFiles, dismissTourTooltip } from "@app/tests/helpers/ui-helpers"; + +// Fixture: 4 portrait pages whose intrinsic /Rotate is 0, 90, 270, 180. +// Page 3 (index 2) is 270 so a single rotate-right lands on a net-0 target - +// the exact case the export used to drop, leaving the source rotation behind. +const ROTATED_PDF = path.join(__dirname, "../test-fixtures/rotated-pages.pdf"); +const SOURCE_ROTATIONS = [0, 90, 270, 180]; + +/** Read the rotation each thumbnail is currently displaying (= page.rotation). */ +async function readEditorRotations(page: import("@playwright/test").Page) { + const imgs = page.locator("[data-page-id] img[data-original-rotation]"); + await expect(imgs).toHaveCount(SOURCE_ROTATIONS.length, { timeout: 30_000 }); + const count = await imgs.count(); + const rots: number[] = []; + for (let i = 0; i < count; i++) { + rots.push( + parseInt( + (await imgs.nth(i).getAttribute("data-original-rotation")) || "NaN", + 10, + ), + ); + } + return rots; +} + +// Skip the fixture's 30s auto-goto; vite's cold on-demand compile can exceed it. +test.use({ autoGoto: false }); + +test.describe("PageEditor (multitool) rotation save", () => { + test("rotating a page persists the correct absolute rotation on export", async ({ + page, + }) => { + await page.goto("/", { waitUntil: "domcontentloaded", timeout: 120_000 }); + await uploadFiles(page, ROTATED_PDF); + // Enter the multitool via in-app navigation, NOT page.goto: a full reload + // wipes the in-memory workbench before PageEditorContext's "entering page + // editor" effect can auto-select the file, leaving the editor empty. + await dismissTourTooltip(page); + await page.getByText("PDF Multi Tool", { exact: true }).first().click(); + + // 1. Baseline: the multitool must seed page.rotation from the source /Rotate, + // otherwise rotated pages render upright and every rotate is off-baseline. + const baseline = await readEditorRotations(page); + expect( + baseline, + "editor must show pages at their true source rotation", + ).toEqual(SOURCE_ROTATIONS); + + // 2. Rotate page 3 (index 2, source /Rotate 270) right once via its + // per-page hover menu. Target rotation is (270 + 90) % 360 = 0. + const page3 = page.locator("[data-page-id]").nth(2); + await page3.scrollIntoViewIfNeeded(); + await page3.hover(); + const rotateRight = page3.getByRole("button", { name: "Rotate Right" }); + await expect(rotateRight).toBeVisible({ timeout: 5_000 }); + await rotateRight.click(); + + // Only page 3 changes (270 -> 0); the others keep their source rotation. + await expect(page3.locator("img[data-original-rotation]")).toHaveAttribute( + "data-original-rotation", + "0", + { timeout: 10_000 }, + ); + expect(await readEditorRotations(page)).toEqual([0, 90, 0, 180]); + + // 3. Ensure all pages are selected, then export, capturing the PDF. + // Pages load all-selected, so "Select All" is disabled - only click it + // if some pages got deselected. + const selectAll = page.getByRole("button", { + name: "Select All", + exact: true, + }); + if (await selectAll.isEnabled()) { + await selectAll.click(); + } + const tmpOut = path.join(os.tmpdir(), `rot-export-${process.pid}.pdf`); + const [download] = await Promise.all([ + page.waitForEvent("download", { timeout: 30_000 }), + page.getByRole("button", { name: "Export Selected Pages" }).click(), + ]); + await download.saveAs(tmpOut); + + // 4. The exported /Rotate must match what the editor showed: page 3 upright + // (0), the untouched pages keeping their source rotation. + const outDoc = await PDFDocument.load(fs.readFileSync(tmpOut)); + const outRotations = outDoc.getPages().map((p) => p.getRotation().angle); + fs.rmSync(tmpOut, { force: true }); + expect(outRotations).toEqual([0, 90, 0, 180]); + }); +}); diff --git a/frontend/editor/src/core/tests/test-fixtures/rotated-pages.pdf b/frontend/editor/src/core/tests/test-fixtures/rotated-pages.pdf new file mode 100644 index 0000000000000000000000000000000000000000..53f7a3ac5ac0f2c25264d2b71fc8a1700dc18b87 GIT binary patch literal 2829 zcmY!laBW|#WD1uNhH=h`C5but>0A(V6~LO2 zb$~1YX+?DaNO@6eUI|paeo$(0erZv1YOw-Hom+lh37FxLnp2iql9`;S>y)39qHAto zX<=evU~XZlU;=V0mp+;TOLG$=GYcbAGYcFFj0_A6EDTI7EzF<_?CiMoBm9%HQj@_p zg1D{~CGNo`i6yBZC)nA6JfrWMnv$95lwY9`tze*FpkQKPpkQWT0170>yuADpP)LFU zEUzRr56m+Ixep{Cln*w_0Avg-6hV@(FmcPwDM>9-(09v8EJ<}qP0mkA<_%}X!I zP%t#N1ZgWSDN0Su<*Jxddj5PClOqq?hw6Ds=?8!LnoU0G^l`%$Z~nNJO2&`!(?kQ0 zJb!=Y(b648ENT;;94}w0C%j|d?TKRczaMVgKY!co%5urF&n?%kPTZ4PdX8P_>7GRf z=h)2(SS;qhGuplgvHTx2?_&8f(^&ThhwlsK1{qGzIQR93NTR>f)_#GEpCMUI zE4^E1C9pIoZ_CrENENwbGkM9eUm}wul2v=Z1bVF(^SW9Rzw|@@Kjy0kJ;Ffg2I_j4 z-$6+Y=66JbBriQ!nwuJ!m{}Sbm_uU|Jw2G4nj4uK8=JwS8(o2giK&61g@Ga51W*E? zMS8G6OAnSP2?8Vw3l#hb!W=C@9OOD=z{BDrNGd_#KP3b#L(OnnxJTv91KCF2)K}imJdee zCxS*~m8O&yVi`>IypV zD;tk)dwcKg>nfX&uZyii?OFtS71dWTUY3k7e0zZBGn?!})k7uiOI1x@T{Y@lXK+{H zY{KLO{=W=o4%;jJ(pWW7dD`vZCTZ5I$Al(7^Pi|TWz&?UoUOL3!pU3sX4XlnPBH8d zzh3eBX6Na|9UJFVm#T$xZaf7^&%j)$fpW;-8ciT$0PB@26nI zrSFzmR9vE9XaKKKjf~7t0_JLKJ)Re6 ztWtdK$*lW5Ua<#uu2Rw3zGfy*?9*9G^zU6=y6^dkxVbj5-&U1A+rBe1Y<~5L{ohtb zH~J^OYRh=I?zYFIStWtuL7N``jeAhH&6(%__Z_MA;(ykQZ(HlJ|HrStnT)<40w0|I zsjO$B%H6p8VajBt9~u4eX1fY2ZnQpD+3-Di#vFTweU|&bs+apN*=GOvW`hB?fCM$2 zV2LO=vno}=(3DF*C_leM0hGLgxb!{q(()BR!5st&K`hM^NJvKnrKZ6H*35!SKU^VN z!BD|S!3fmu@yts}g*2B8O%-BM0zEk);m7~^6CE2G8z(BxYi#Un6n8Q(GV)>RGSp)g z^5Je|`D~_ea1*1)203=2oZ}5Fzhe~+77BY5Xc`(YFeG+zW@7PHaYn0LCcXod5s; literal 0 HcmV?d00001 diff --git a/frontend/editor/src/core/utils/thumbnailUtils.ts b/frontend/editor/src/core/utils/thumbnailUtils.ts index a4efafbff5..838992e38b 100644 --- a/frontend/editor/src/core/utils/thumbnailUtils.ts +++ b/frontend/editor/src/core/utils/thumbnailUtils.ts @@ -192,15 +192,16 @@ export async function generateThumbnailWithMetadata( } const scale = calculateScaleFromFileSize(file.size); - const isVeryLarge = file.size >= 100 * 1024 * 1024; // 100MB threshold try { const arrayBuffer = await file.arrayBuffer(); + // Always read per-page rotation: PageEditor renders thumbnails upright and + // uses this as the rotation baseline, so skipping it corrupts saves. const result = await renderPdfThumbnailPdfium( arrayBuffer, scale, applyRotation, - !isVeryLarge, + true, ); if (result.isEncrypted) {