From 165a5c744ba81894f164a5c8d6a3b9cf198746da Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Sat, 23 May 2026 17:55:52 +0300 Subject: [PATCH] Fix #5014: roll back default-date staging when Picker is cancelled on first open The default-date support added in RFE #4973 stages the resolved default into Picker.value via applyDefaultDateIfNeeded() so the show paths can display it. The Cancel-restore snapshot in showInteractionDialog ran *after* that staging, so a Cancel on the first open restored the leaked default into value - flipping the picker text from "..." to today's date and making subsequent getDate() / setDate(null) behavior look as if the user had committed. The same leak existed silently in the native picker and synchronous heavyweight Dialog branches. Take the snapshot in actionPerformed before applyDefaultDateIfNeeded runs, and add the matching cancel-time rollback in the native picker branch and the heavyweight Dialog PICKER_TYPE_DATE / DATE_AND_TIME cases. The existing custom-button cancel rollback (#4897) still works because both snapshots are now sourced from the same earlier capture. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/com/codename1/ui/spinner/Picker.java | 51 +++++++++++++--- .../ui/spinner/PickerDefaultDateTest.java | 58 +++++++++++++++++++ 2 files changed, 101 insertions(+), 8 deletions(-) diff --git a/CodenameOne/src/com/codename1/ui/spinner/Picker.java b/CodenameOne/src/com/codename1/ui/spinner/Picker.java index 5c2abe6b54..2150cabb8e 100644 --- a/CodenameOne/src/com/codename1/ui/spinner/Picker.java +++ b/CodenameOne/src/com/codename1/ui/spinner/Picker.java @@ -245,6 +245,16 @@ public void actionPerformed(ActionEvent evt) { evt.consume(); return; } + // Snapshot the pre-tap state BEFORE applyDefaultDateIfNeeded() + // mutates `value`, so a Cancel can roll back to what the picker + // showed when the user tapped it (including a null placeholder + // that renders as "...") rather than to the just-staged default. + // The lightweight popup, native picker, and synchronous + // heavyweight Dialog branches below all use this snapshot - see + // the matching cancel-time restore in endEditing() (lightweight) + // and inline in this method (native + heavyweight). Issue #5014. + preEditValue = value; + preEditDateValueExplicitlySet = dateValueExplicitlySet; // For date-type pickers that haven't been pinned with setDate, fold the // resolved default into `value` before any show path reads it. Both // showInteractionDialog() and the native/heavyweight branches below @@ -280,8 +290,17 @@ public void actionPerformed(ActionEvent evt) { updateValue(); } else { // cancel pressed. Don't send the rest of the events. + // Roll back the default-date staging done before the + // native picker was shown, otherwise a Cancel on the + // first open of a setDefaultDate-configured picker + // would pin today's date into `value`. Issue #5014. + value = preEditValue; + dateValueExplicitlySet = preEditDateValueExplicitlySet; + updateValue(); evt.consume(); } + preEditValue = null; + preEditDateValueExplicitlySet = false; setEnabled(true); } else { Dialog pickerDlg = new Dialog(); @@ -336,8 +355,16 @@ public void actionPerformed(ActionEvent evt) { value = cld.getTime(); dateValueExplicitlySet = true; } else { + // Roll back the default-date staging from + // applyDefaultDateIfNeeded() so a Cancel on + // an unset picker doesn't pin the default + // into `value`. Issue #5014. + value = preEditValue; + dateValueExplicitlySet = preEditDateValueExplicitlySet; evt.consume(); } + preEditValue = null; + preEditDateValueExplicitlySet = false; break; } case Display.PICKER_TYPE_TIME: { @@ -399,8 +426,15 @@ public void actionPerformed(ActionEvent evt) { value = cld.getTime(); dateValueExplicitlySet = true; } else { + // Roll back the default-date staging from + // applyDefaultDateIfNeeded() on Cancel. + // Issue #5014. + value = preEditValue; + dateValueExplicitlySet = preEditDateValueExplicitlySet; evt.consume(); } + preEditValue = null; + preEditDateValueExplicitlySet = false; break; } case Display.PICKER_TYPE_DURATION_HOURS: @@ -612,14 +646,15 @@ private void showInteractionDialog() { throw new IllegalArgumentException("Unsupported picker type " + type); } currentSpinner = spinner; - // Snapshot the committed value so a Cancel press can restore it. - // Custom popup buttons stage their result through setDate / setTime / - // setDuration / setSelectedString etc., which now mutate `value` - // directly so that getDate() during the edit returns the staged - // value; without this snapshot a Cancel after such a button would - // leak the staged value into the picker (#4897 follow-up). - preEditValue = value; - preEditDateValueExplicitlySet = dateValueExplicitlySet; + // The Cancel-restore snapshot (`preEditValue` / + // `preEditDateValueExplicitlySet`) is taken in the parent + // actionPerformed() *before* applyDefaultDateIfNeeded() runs, + // so it reflects the state the picker had when the user + // tapped it - not the post-default-staging `value`. Custom + // popup buttons that stage via setDate / setTime / + // setDuration / setSelectedString continue to be rolled back + // by endEditing(COMMAND_CANCEL) the same way. Issues #4897, + // #5014. final InteractionDialog dlg = new InteractionDialog() { ActionListener keyListener; diff --git a/maven/core-unittests/src/test/java/com/codename1/ui/spinner/PickerDefaultDateTest.java b/maven/core-unittests/src/test/java/com/codename1/ui/spinner/PickerDefaultDateTest.java index 16218ad921..c704bfaa7d 100644 --- a/maven/core-unittests/src/test/java/com/codename1/ui/spinner/PickerDefaultDateTest.java +++ b/maven/core-unittests/src/test/java/com/codename1/ui/spinner/PickerDefaultDateTest.java @@ -141,6 +141,64 @@ public Date get() { "A getter that returns null must fall back to a fresh Date, never null"); } + /// Regression for #5014: with a default-date getter installed and no + /// explicit `setDate(non-null)`, tapping the picker stages the resolved + /// default into `value` so the popup can show it. Pressing Cancel must + /// roll that staging back so the picker keeps showing its placeholder + /// ("...") and a re-open re-resolves the getter, exactly as before the + /// RFE #4973 change. + @FormTest + public void cancelOnFirstOpenKeepsPlaceholderAndDoesNotPinValue() { + TestCodenameOneImplementation impl = TestCodenameOneImplementation.getInstance(); + if (impl != null) { + impl.setTablet(false); + } + final Picker picker = new Picker(); + picker.setDefaultDate(new Picker.DateGetter() { + @Override + public Date get() { + return date(2030, Calendar.JUNE, 15); + } + }); + picker.setType(Display.PICKER_TYPE_DATE); + picker.setUseLightweightPopup(true); + picker.setDate(null); + Assertions.assertEquals("...", picker.getText(), + "Before the first tap the picker must show its placeholder"); + + Form f = new Form(new BoxLayout(BoxLayout.Y_AXIS)); + f.add(picker); + f.show(); + + picker.pressed(); + picker.released(); + runAnimations(f); + + InteractionDialog dlg = findInteractionDialog(f); + Assertions.assertNotNull(dlg, "Lightweight popup should be open"); + Button cancel = findButtonWithText(dlg, "Cancel"); + Assertions.assertNotNull(cancel, "Cancel button should be present"); + cancel.pressed(); + cancel.released(); + DisplayTest.flushEdt(); + runAnimations(f); + + Assertions.assertEquals("...", picker.getText(), + "Cancel on the first open must leave the placeholder intact (#5014)"); + // Bump the slot the getter would return so we can prove the next + // getDate() call still resolves through the getter rather than + // returning a leaked staged value from the cancelled open. + final Date[] slot = new Date[] { date(2031, Calendar.FEBRUARY, 2) }; + picker.setDefaultDate(new Picker.DateGetter() { + @Override + public Date get() { + return slot[0]; + } + }); + Assertions.assertEquals(slot[0], picker.getDate(), + "After Cancel the default getter must still drive getDate()"); + } + @FormTest public void cancelAfterCustomButtonRollsBackExplicitFlag() { TestCodenameOneImplementation impl = TestCodenameOneImplementation.getInstance();