ArtifexSoftware / mupdf.js

JavaScript bindings for MuPDF
https://mupdfjs.readthedocs.io
GNU Affero General Public License v3.0
381 stars 22 forks source link

Yet another Journal issue #114

Open xeladotbe opened 1 month ago

xeladotbe commented 1 month ago

Hello,

I am rearranging pages in a PDF document and individual pages are also being rotated. If I use the undo function of the journal to undo individual changes, the pages in the PDF no longer correspond to the original pages.

I have written a test to reproduce the problem.

    it("should undo all operations", () => {
        const structedTextFirstPage = document
            .loadPage(0)
            .toStructuredText()
            .asJSON();
        const structedTextSecondPage = document
            .loadPage(1)
            .toStructuredText()
            .asJSON();
        const structedTextThirdPage = document
            .loadPage(2)
            .toStructuredText()
            .asJSON();

        document.beginOperation("rearrangePages");
        document.rearrangePages([2, 1, 0]);
        document.endOperation();

        document.beginOperation("Rotate page");
        const pageObject = document.loadPage(0).getObject();
        const currentRotation = pageObject.getInheritable("Rotate").asNumber();

        pageObject.put("Rotate", (currentRotation + 90) % 360);
        document.endOperation();

        document.undo();
        document.undo();

        expect(document.canUndo()).toBe(false);
        expect(document.countPages()).toBe(3);
        expect(document.hasUnsavedChanges()).toBe(true);
        expect(document.getJournal()).toStrictEqual({
            position: 0,
            steps: ["rearrangePages", "Rotate page"],
        });
        expect(document.loadPage(0).toStructuredText().asJSON()).toStrictEqual(
            structedTextFirstPage
        );
        expect(document.loadPage(1).toStructuredText().asJSON()).toStrictEqual(
            structedTextSecondPage
        );
        expect(document.loadPage(2).toStructuredText().asJSON()).toStrictEqual(
            structedTextThirdPage
        );
    });

The test is based on the existing test “should undo an operation” and without the rotation, the test is successful.

This also happens with mupdf 0.2.3 but also with 0.3.0

Regards, Alex

xeladotbe commented 1 month ago

Also the following test fails:

    it("should undo an operation", () => {
        const structedTextFirstPage = document
            .loadPage(0)
            .toStructuredText()
            .asJSON();
        const structedTextSecondPage = document
            .loadPage(1)
            .toStructuredText()
            .asJSON();
        const structedTextThirdPage = document
            .loadPage(2)
            .toStructuredText()
            .asJSON();

        document.beginOperation("rearrange page");
        const page = document.loadPage(0);
        document.deletePage(0);
        document.insertPage(-1, page.getObject());
        document.endOperation();

        document.undo();

        expect(document.countPages()).toBe(3);
        expect(document.hasUnsavedChanges()).toBe(false);
        expect(document.getJournal()).toStrictEqual({
            position: 0,
            steps: ["rearrange page"],
        });
        expect(document.loadPage(0).toStructuredText().asJSON()).toStrictEqual(
            structedTextFirstPage
        );
        expect(document.loadPage(1).toStructuredText().asJSON()).toStrictEqual(
            structedTextSecondPage
        );
        expect(document.loadPage(2).toStructuredText().asJSON()).toStrictEqual(
            structedTextThirdPage
        );
    });

Am I doing something wrong?

jamie-lemon commented 1 month ago

I don't think you are doing anything wrong here.

I simplified the test even further:

it("should undo an operation", () => {

        const structedTextFirstPage = document
            .loadPage(0)
            .toStructuredText()
            .asJSON();
        const structedTextSecondPage = document
            .loadPage(1)
            .toStructuredText()
            .asJSON();
        const structedTextThirdPage = document
            .loadPage(2)
            .toStructuredText()
            .asJSON();

        console.log("fp="+structedTextFirstPage);

        document.beginOperation("delete page");
        document.deletePage(0);
        document.endOperation();

        let canUndo = document.canUndo();

        console.log("canUndo="+canUndo);

        document.undo();

        console.log("1="+document.loadPage(0).toStructuredText().asJSON())
        console.log("2="+document.loadPage(1).toStructuredText().asJSON())
        console.log("3="+document.loadPage(2).toStructuredText().asJSON())

        expect(document.countPages()).toBe(3);
        expect(document.hasUnsavedChanges()).toBe(false);
        expect(document.getJournal()).toStrictEqual({
            position: 0,
            steps: ["delete page"],
        });
        expect(document.loadPage(0).toStructuredText().asJSON()).toStrictEqual(
            structedTextFirstPage
        );
        expect(document.loadPage(1).toStructuredText().asJSON()).toStrictEqual(
            structedTextSecondPage
        );
        expect(document.loadPage(2).toStructuredText().asJSON()).toStrictEqual(
            structedTextThirdPage
        );
    });

If you look at the console logs for pages 1,2 & 3 you'll see that page 1 seems to have gone ( you can tell by the text value of the footer text - the last two pages are "Page 3 footer".)

So my assessment here is that this is a bug with journalling and deleting and undoing on pages.

jamie-lemon commented 1 month ago

@ccxvii Think we have discovered a bug with journaling here.

xeladotbe commented 1 month ago

Are there any workarounds I can use until the issue is fixed?

jamie-lemon commented 4 weeks ago

@xeladotbe So it appears to be a kind of "runtime"issue. In fact the undo operation does appear to undo what happened, it is just that at runtime that does not seem to be being interpreted/read correctly. i.e, I f we do:

        document.beginOperation("delete page");
        document.deletePage(0);
        document.endOperation();

        let canUndo = document.canUndo();

        console.log("canUndo="+canUndo);

        document.undo();

        console.log("we undo ....");

        fs.writeFileSync("output.pdf", document.saveToBuffer("").asUint8Array())

Then the saved document has the correct pages in the correct (original) order.

Of course I don't know if this helps you or not for your situation and we still need to fix the bug.

jamie-lemon commented 4 weeks ago

Created a BugZilla issue as this requires a fix in core MuPDF, see: https://bugs.ghostscript.com/show_bug.cgi?id=708052

xeladotbe commented 4 weeks ago

Hey @jamie-lemon, unfortunately this doesn't fix my problem, but I would temporarily just re-implement the rotation with css until the bug is fixed