From 60518148edcdebdf62813d47fe955695305c0278 Mon Sep 17 00:00:00 2001 From: Ladislav Nemec <316699645+LadaN62@users.noreply.github.com> Date: Sat, 19 Sep 2026 11:42:05 -0400 Subject: [PATCH 1/2] Fix output suffix handling when loading presets --- .../jsignpdf/fx/view/MainWindowController.java | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/jsignpdf/src/main/java/net/sf/jsignpdf/fx/view/MainWindowController.java b/jsignpdf/src/main/java/net/sf/jsignpdf/fx/view/MainWindowController.java index ed5c1483..1d449f31 100644 --- a/jsignpdf/src/main/java/net/sf/jsignpdf/fx/view/MainWindowController.java +++ b/jsignpdf/src/main/java/net/sf/jsignpdf/fx/view/MainWindowController.java @@ -227,6 +227,14 @@ public void setStage(Stage stage) { public void initFromOptions(BasicSignerOptions opts) { this.options = opts; signingVM.syncFromOptions(opts); + + // The persisted output file belongs to the previous document/session. + // Start each application run with a derived output name so the first + // opened document follows the current suffix instead of inheriting a + // stale explicit base name. + signingVM.outBaseNameProperty().set(null); + signingVM.outFileProperty().set(null); + // No document is loaded at startup, so the visible-signature toggle must // start disabled. The persisted position coordinates on signingVM are // preserved so we can auto-place at the last-known location once a @@ -650,10 +658,19 @@ private void loadPreset(Preset preset) { if (options == null) { options = new BasicSignerOptions(); } + // Preserve whether the output filename is currently derived automatically. + // Loading a preset changes the suffix, and syncFromOptions() must not turn + // the previously derived filename into an explicit user-selected name. + boolean derivedOutputName = isDerivedBaseName(signingVM.outBaseNameProperty().get(), options.getInFile()); + // Flush any pending edits from the VM so that "load" is clearly "replace current". signingVM.syncToOptions(options); presetManager.load(preset, options); signingVM.syncFromOptions(options); + + if (derivedOutputName) { + signingVM.outBaseNameProperty().set(null); + } resolveOutputFile(); // Move the on-screen rectangle to match the preset's position. applySigningVMPositionToPlacement(); From fe80752b688e20837f5ba1313da155d279a9196d Mon Sep 17 00:00:00 2001 From: "Josef (kwart) Cacek" Date: Tue, 22 Sep 2026 11:24:51 +0200 Subject: [PATCH 2/2] Extend the fixes --- .../fx/view/MainWindowController.java | 43 ++++--- .../fx/viewmodel/SigningOptionsViewModel.java | 14 +++ .../fx/view/MainWindowOutputNameTest.java | 107 ++++++++++++++++++ .../SigningOptionsViewModelTest.java | 56 +++++++++ 4 files changed, 198 insertions(+), 22 deletions(-) create mode 100644 jsignpdf/src/test/java/net/sf/jsignpdf/fx/view/MainWindowOutputNameTest.java diff --git a/jsignpdf/src/main/java/net/sf/jsignpdf/fx/view/MainWindowController.java b/jsignpdf/src/main/java/net/sf/jsignpdf/fx/view/MainWindowController.java index 1d449f31..b3b53c7f 100644 --- a/jsignpdf/src/main/java/net/sf/jsignpdf/fx/view/MainWindowController.java +++ b/jsignpdf/src/main/java/net/sf/jsignpdf/fx/view/MainWindowController.java @@ -228,12 +228,11 @@ public void initFromOptions(BasicSignerOptions opts) { this.options = opts; signingVM.syncFromOptions(opts); - // The persisted output file belongs to the previous document/session. - // Start each application run with a derived output name so the first - // opened document follows the current suffix instead of inheriting a - // stale explicit base name. + // The output file name is session state: the persisted one names the document signed last time, so it is + // dropped (unlike the output directory and the suffix, which are settings) and every run starts derived. + opts.setOutFile(null); signingVM.outBaseNameProperty().set(null); - signingVM.outFileProperty().set(null); + resolveOutputFile(); // No document is loaded at startup, so the visible-signature toggle must // start disabled. The persisted position coordinates on signingVM are @@ -658,20 +657,10 @@ private void loadPreset(Preset preset) { if (options == null) { options = new BasicSignerOptions(); } - // Preserve whether the output filename is currently derived automatically. - // Loading a preset changes the suffix, and syncFromOptions() must not turn - // the previously derived filename into an explicit user-selected name. - boolean derivedOutputName = isDerivedBaseName(signingVM.outBaseNameProperty().get(), options.getInFile()); - // Flush any pending edits from the VM so that "load" is clearly "replace current". signingVM.syncToOptions(options); presetManager.load(preset, options); - signingVM.syncFromOptions(options); - - if (derivedOutputName) { - signingVM.outBaseNameProperty().set(null); - } - resolveOutputFile(); + reloadFromOptionsKeepingOutputName(); // Move the on-screen rectangle to match the preset's position. applySigningVMPositionToPlacement(); updateStatus(java.text.MessageFormat.format( @@ -978,6 +967,16 @@ private boolean isDerivedBaseName(String baseName, String previousInFile) { return baseName.equals(derived); } + /** + * Reloads the signing view model from {@code options} after they were changed behind its back (a preset load, an + * owner-password retry) and recomposes the output path. The output file name is carried over rather than re-read, + * because {@code options} hold it as a path composed with the suffix and input they had before the change. + */ + private void reloadFromOptionsKeepingOutputName() { + signingVM.syncFromOptionsKeepingOutBaseName(options); + resolveOutputFile(); + } + /** * Resolves the effective output path from the two user-facing fields — the output directory ({@code outPath}) and the * output file name ({@code outBaseName}) — plus the input file and suffix, and writes it to {@code outFile} (the value @@ -1535,17 +1534,17 @@ private void openDocument(File file) { saveViewStateToConfig(); try { if (options == null) { - options = new BasicSignerOptions(); - options.loadOptions(); - signingVM.syncFromOptions(options); + BasicSignerOptions opts = new BasicSignerOptions(); + opts.loadOptions(); + initFromOptions(opts); } // Reset visible signature and placement from previous document signingVM.visibleProperty().set(false); placementVM.reset(); - // A derived output name is refreshed for the new document; a name the user deliberately chose is kept, like - // the output directory. The two are told apart by comparing against the name the previous input derived — + // A derived output name is refreshed for the new document; a name the user deliberately chose is kept for + // the rest of the session. The two are told apart by comparing against the name the previous input derived — // captured before setInFile below overwrites it. The path resolves via the document-file listener once // documentVM is updated further down. if (isDerivedBaseName(signingVM.outBaseNameProperty().get(), options.getInFile())) { @@ -1616,7 +1615,7 @@ private int promptPasswordAndRetry(File file) { options.setPdfOwnerPwd(password.get().toCharArray()); options.setAdvanced(true); - signingVM.syncFromOptions(options); + reloadFromOptionsKeepingOutputName(); try { PdfExtraInfo extraInfo = new PdfExtraInfo(options); diff --git a/jsignpdf/src/main/java/net/sf/jsignpdf/fx/viewmodel/SigningOptionsViewModel.java b/jsignpdf/src/main/java/net/sf/jsignpdf/fx/viewmodel/SigningOptionsViewModel.java index e234eaf9..2352f362 100644 --- a/jsignpdf/src/main/java/net/sf/jsignpdf/fx/viewmodel/SigningOptionsViewModel.java +++ b/jsignpdf/src/main/java/net/sf/jsignpdf/fx/viewmodel/SigningOptionsViewModel.java @@ -274,6 +274,20 @@ public void syncFromOptions(BasicSignerOptions opts) { proxyPort.set(opts.getProxyPort()); } + /** + * Syncs from a BasicSignerOptions instance, keeping the output file name currently held by this ViewModel. + *

+ * {@link #syncFromOptions(BasicSignerOptions)} has to tell a derived output name from a deliberately chosen one by + * comparing the stored path against the one the options derive — which misreads a derived name as chosen whenever + * the reload brings a different suffix or input file. Mid-session the ViewModel already knows which of the two it + * holds, so callers that reload live options (preset load, owner-password retry) keep that answer instead. + */ + public void syncFromOptionsKeepingOutBaseName(BasicSignerOptions opts) { + String currentBaseName = outBaseName.get(); + syncFromOptions(opts); + outBaseName.set(currentBaseName); + } + /** * Resets all ViewModel properties to their default values. */ diff --git a/jsignpdf/src/test/java/net/sf/jsignpdf/fx/view/MainWindowOutputNameTest.java b/jsignpdf/src/test/java/net/sf/jsignpdf/fx/view/MainWindowOutputNameTest.java new file mode 100644 index 00000000..111a4b2c --- /dev/null +++ b/jsignpdf/src/test/java/net/sf/jsignpdf/fx/view/MainWindowOutputNameTest.java @@ -0,0 +1,107 @@ +package net.sf.jsignpdf.fx.view; + +import static org.junit.Assert.assertNull; + +import java.lang.reflect.Field; +import java.util.Locale; +import java.util.ResourceBundle; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; + +import javafx.application.Platform; +import javafx.fxml.FXMLLoader; +import javafx.scene.layout.BorderPane; + +import org.junit.BeforeClass; +import org.junit.Test; + +import net.sf.jsignpdf.BasicSignerOptions; +import net.sf.jsignpdf.Constants; +import net.sf.jsignpdf.fx.MonocleAssumption; +import net.sf.jsignpdf.fx.viewmodel.SigningOptionsViewModel; + +/** + * The output file name is session state. A name carried over from the previous run would otherwise be applied to the + * first document opened in this one, and - being read back as a deliberate choice - would then stop following the + * suffix for the rest of the session. + */ +public class MainWindowOutputNameTest { + + @BeforeClass + public static void initFx() throws Exception { + MonocleAssumption.assumeUsable(); + CountDownLatch latch = new CountDownLatch(1); + try { + Platform.startup(latch::countDown); + } catch (IllegalStateException e) { + latch.countDown(); + } + latch.await(5, TimeUnit.SECONDS); + } + + @Test + public void persistedOutputNameIsDroppedAtStartup() throws Exception { + AtomicReference baseName = new AtomicReference<>(); + AtomicReference outFile = new AtomicReference<>(); + AtomicReference persisted = new AtomicReference<>(); + runOnFxThread(() -> { + MainWindowController controller = loadMainWindow(); + + BasicSignerOptions opts = new BasicSignerOptions(); + opts.setInFile("/docs/drawing.pdf"); + opts.setOutSuffix("_EM"); + opts.setOutFile("/docs/final.pdf"); + controller.initFromOptions(opts); + + SigningOptionsViewModel vm = signingViewModel(controller); + baseName.set(vm.outBaseNameProperty().get()); + outFile.set(vm.outFileProperty().get()); + persisted.set(opts.getOutFile()); + }); + + assertNull("the persisted output name must not survive a restart", baseName.get()); + assertNull("no document is open, so there is no output path yet", outFile.get()); + assertNull("the options must not keep the stale path either", persisted.get()); + } + + private static MainWindowController loadMainWindow() { + try { + FXMLLoader loader = new FXMLLoader( + MainWindowOutputNameTest.class.getResource("/net/sf/jsignpdf/fx/view/MainWindow.fxml"), + ResourceBundle.getBundle(Constants.RESOURCE_BUNDLE_BASE, Locale.ENGLISH)); + loader.load(); + return loader.getController(); + } catch (Exception e) { + throw new IllegalStateException(e); + } + } + + private static SigningOptionsViewModel signingViewModel(MainWindowController controller) { + try { + Field field = MainWindowController.class.getDeclaredField("signingVM"); + field.setAccessible(true); + return (SigningOptionsViewModel) field.get(controller); + } catch (ReflectiveOperationException e) { + throw new IllegalStateException(e); + } + } + + private static void runOnFxThread(Runnable action) throws Exception { + CountDownLatch latch = new CountDownLatch(1); + AtomicReference error = new AtomicReference<>(); + Platform.runLater(() -> { + try { + action.run(); + } catch (Throwable t) { + error.set(t); + } finally { + latch.countDown(); + } + }); + latch.await(10, TimeUnit.SECONDS); + if (error.get() != null) { + throw new AssertionError(error.get()); + } + } +} diff --git a/jsignpdf/src/test/java/net/sf/jsignpdf/fx/viewmodel/SigningOptionsViewModelTest.java b/jsignpdf/src/test/java/net/sf/jsignpdf/fx/viewmodel/SigningOptionsViewModelTest.java index 11756af0..9582fced 100644 --- a/jsignpdf/src/test/java/net/sf/jsignpdf/fx/viewmodel/SigningOptionsViewModelTest.java +++ b/jsignpdf/src/test/java/net/sf/jsignpdf/fx/viewmodel/SigningOptionsViewModelTest.java @@ -351,4 +351,60 @@ public void syncFromOptions_customOutputNameSurvives() { assertEquals("A deliberately chosen name must survive the round trip", "final.pdf", vm.outBaseNameProperty().get()); } + + /** + * Loading a preset replaces the suffix, which leaves the output path in the options composed with the old one. + * A cleared name must stay cleared so it is recomposed with the new suffix instead of accumulating both. + */ + @Test + public void syncFromOptionsKeepingOutBaseName_clearedNameStaysCleared() { + BasicSignerOptions opts = new BasicSignerOptions(); + opts.setInFile("/docs/drawing.pdf"); + opts.setOutSuffix("_EM"); + opts.setOutFile(opts.getOutFileX()); + + SigningOptionsViewModel vm = new SigningOptionsViewModel(); + vm.syncFromOptions(opts); + opts.setOutSuffix("_DL"); + vm.syncFromOptionsKeepingOutBaseName(opts); + + assertNull("The loaded suffix must not turn a derived name into a chosen one", + vm.outBaseNameProperty().get()); + } + + /** + * The owner-password retry re-syncs while the options already name the new input but still hold the output path + * of the previous document. + */ + @Test + public void syncFromOptionsKeepingOutBaseName_staleOutputOfPreviousInputIsIgnored() { + BasicSignerOptions opts = new BasicSignerOptions(); + opts.setInFile("/docs/drawing.pdf"); + opts.setOutSuffix("_signed"); + opts.setOutFile(opts.getOutFileX()); + + SigningOptionsViewModel vm = new SigningOptionsViewModel(); + vm.syncFromOptions(opts); + opts.setInFile("/docs/encrypted.pdf"); + vm.syncFromOptionsKeepingOutBaseName(opts); + + assertNull("The previous document's output name must not become the new document's chosen name", + vm.outBaseNameProperty().get()); + } + + @Test + public void syncFromOptionsKeepingOutBaseName_chosenNameSurvives() { + BasicSignerOptions opts = new BasicSignerOptions(); + opts.setInFile("/docs/drawing.pdf"); + opts.setOutSuffix("_EM"); + opts.setOutFile("/docs/final.pdf"); + + SigningOptionsViewModel vm = new SigningOptionsViewModel(); + vm.syncFromOptions(opts); + opts.setOutSuffix("_DL"); + vm.syncFromOptionsKeepingOutBaseName(opts); + + assertEquals("A deliberately chosen name must survive a preset load", + "final.pdf", vm.outBaseNameProperty().get()); + } }