Pinta icon indicating copy to clipboard operation
Pinta copied to clipboard

Handle dirty state correctly when closing document

Open ericksson opened this issue 4 months ago • 4 comments

Fixes https://bugs.launchpad.net/pinta/+bug/1435226

The implementation was committing tool state first, which can finalize editable shapes and push a history item, marking the document dirty right before closing.

This was changed so that:

  • If the document is not dirty, close it immediately without committing
  • Only commit pending changes when the document is dirty and we might prompt to save

This prevents creating a last-second history item that triggers the prompt after a successful save.

Tested locally the fact that the bug was still reproducing before the change and does not reproduce after this. The image is still correctly saved after using any tool, saving and closing the app.

ericksson avatar Aug 08 '25 19:08 ericksson

Thanks for investigating this! I agree we shouldn't be committing before checking the dirty state, since after saving the tool will have rolled back any history items added while committing, and the state will be marked as clean https://github.com/PintaProject/Pinta/blob/bfc905a6b6590391d6b08488e3c7742e7935d16a/Pinta/Actions/File/SaveDocumentImplementationAction.cs#L275

I'm thinking we should also go further though and remove the tools.Commit() entirely here - this already happens in the actual code for saving the file so it seems unnecessary to do it up front. https://github.com/PintaProject/Pinta/blob/bfc905a6b6590391d6b08488e3c7742e7935d16a/Pinta/Actions/File/SaveDocumentImplementationAction.cs#L238

This would avoid adding the "Finalize" history item if the user decides to cancel closing the document, for example:

https://github.com/user-attachments/assets/7042604b-cbcc-48b5-962b-9d215016f899

cameronwhite avatar Aug 09 '25 14:08 cameronwhite

Thank you! I didn't catch that detail. I wonder why this call was added in the CloseDocumentationAction class to begin with. Maybe as an extra precaution in case some tool doesn't call Commit.

With the call to tools.Commit () removed entirely everything still seems to work, but the Finalize state is still in the History when cancelling that save dialogue. I proposed a fix in https://github.com/PintaProject/Pinta/pull/1656/commits/87c57b688cf2c381732e690df77f1cd2e3ae34df, though there maybe could be a better way to handle that, but I couldn't figure out anything else.

ericksson avatar Aug 09 '25 23:08 ericksson

While trying to replicate the example in your screen recording, I noticed that upon canceling or saving the current tab, the shape is Finalized and added to the other tab that is already opened. This is a bug that seems like it was there before the changes in this PR, but it seems like a pretty bad one. I proposed a fix in https://github.com/PintaProject/Pinta/pull/1656/commits/e8a281e3c849cc6fad12a54655e8630c7019b1ee

This fix and the fix for Finalize above can probably be split into a separate PR if needed.

So far, these are the changes in this PR:

  • No Tool commit on close document prompt.
  • Suppressed commit when switching active document during tab close.
  • Ignored transient tab selection changes during close.
  • Shape finalization and shape drawing and handles prevented on wrong tab.

ericksson avatar Aug 10 '25 00:08 ericksson

It looks like there are some merge conflicts, so I suspect your branch is behind the latest master branch?

I wasn't able to reproduce the Finalize state is still in the History when cancelling that save dialogue issue, but perhaps I'm missing some steps or it was fixed by some other changes

Would you also maybe be able to record a screen grab of the other issue with shape is Finalized and added to the other tab? e.g. does the other document need to also have shapes in it?

cameronwhite avatar Aug 10 '25 02:08 cameronwhite