ImageCropper: dispose the state built for the previous image on re-set

[imnasnainaec]

The Image setter holds two things for the image it is cropping: a temp file with the original at full resolution, and a possibly-downscaled bitmap to display. It mishandles both when given a new image.

It overwrites the two fields without disposing what was there, leaking a temp file and a bitmap on every re-set — the normal path, since re-entering the Crop tab re-sets Image. The leaked bitmap is a clone of a file-backed image, so it also holds a lock on the previous source file: disposing that PalasoImage trips Debug.Fail("Not able to delete image temp file") with IOException: the process cannot access the file ... because it is being used by another process.

It also assigns _savedOriginalImage before value.Image.Save can throw. If it does, the field is left pointing at the new, empty temp file while the cropper still displays the previous image, so GetCroppedImage crops from the empty file.

The change

Build the temp file and the cropping bitmap into locals and store them only once both are ready — disposing the previous pair at that point, or the partial state if anything throws. The BL-2680-tolerant disposal is factored out of Dispose so the three call sites share it.

SetImage likewise commits _originalFormat only after the setter succeeds, so a failure no longer leaves it on the new image while everything else is on the old.

Tests

Both fail without the change.

• SetImage_Reassigned_DisposesStateBuiltForPreviousImage — the previous temp file is still on disk after a re-set.
• Image_NewImageFailsToLoad_LeavesPreviousImageStateIntact — after a failed assignment the cropper still holds the same temp file and bitmap.

--filter ImageToolbox is green on net8.0-windows and net48: 47 passed, 1 pre-existing skip.

🤖 Generated with Claude Code

---

This change is 

[github-actions]

Palaso Tests

     4 files  ±0       4 suites  ±0   11m 30s ⏱️ +57s
 5 121 tests +2   4 887 ✅ +2  234 💤 ±0  0 ❌ ±0 
16 675 runs  +6  15 954 ✅ +6  721 💤 ±0  0 ❌ ±0 

Results for commit 0589d8f. ± Comparison against base commit a8bc5a7.

♻️ This comment has been updated with latest results.

[tombogle]

@tombogle reviewed 3 files and all commit messages, and made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on imnasnainaec and tombogle).

---

SIL.Windows.Forms.Tests/ImageToolbox/ImageCropperTests.cs line 202 at r2 (raw file):

		}

		private static object GetPrivateField(ImageCropper cropper, string fieldName)

I think this could use the existing ReflectionHelper.GetField rather than recreating it.

[imnasnainaec]

@imnasnainaec reviewed all commit messages and made 1 comment.
Reviewable status: 2 of 3 files reviewed, 1 unresolved discussion (waiting on tombogle).

---

SIL.Windows.Forms.Tests/ImageToolbox/ImageCropperTests.cs line 202 at r2 (raw file):

Previously, tombogle (Tom Bogle) wrote…

I think this could use the existing ReflectionHelper.GetField rather than recreating it.

Thanks! Done.

[tombogle]

@tombogle reviewed 1 file and all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on imnasnainaec).

[tombogle]

@tombogle resolved 1 discussion.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on imnasnainaec).