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();