Fix Multi Tool page rotation lost on save (#6733)
# 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.
This commit is contained in:
@@ -64,6 +64,7 @@ const HoverActionMenu: React.FC<HoverActionMenuProps> = ({
|
||||
disabled={action.disabled}
|
||||
onClick={action.onClick}
|
||||
c={action.color}
|
||||
aria-label={action.label}
|
||||
style={{ color: action.color || "var(--text-secondary)" }}
|
||||
data-tour={action.dataTour}
|
||||
>
|
||||
|
||||
@@ -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++;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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]);
|
||||
});
|
||||
});
|
||||
Binary file not shown.
@@ -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) {
|
||||
|
||||
Reference in New Issue
Block a user