Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Yet another Journal issue #114

Open
xeladotbe opened this issue Sep 17, 2024 · 8 comments
Open

Yet another Journal issue #114

xeladotbe opened this issue Sep 17, 2024 · 8 comments
Assignees

Comments

@xeladotbe
Copy link
Contributor

xeladotbe commented Sep 17, 2024

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
Copy link
Contributor Author

xeladotbe commented Sep 17, 2024

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
Copy link
Collaborator

jamie-lemon commented Sep 17, 2024

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
Copy link
Collaborator

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

@xeladotbe
Copy link
Contributor Author

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

@jamie-lemon
Copy link
Collaborator

@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
Copy link
Collaborator

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

@xeladotbe
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants
@ccxvii @xeladotbe @mipo1357 @jamie-lemon and others