Skip to content

Fix #5014: roll back default-date staging when Picker is cancelled on first open #5017

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
shai-almog merged 1 commit into from
May 23, 2026
Merged

Fix #5014: roll back default-date staging when Picker is cancelled on first open #5017

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 43 additions & 8 deletions CodenameOne/src/com/codename1/ui/spinner/Picker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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: {
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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;
Expand Down
58 changes: 58 additions & 0 deletions maven/core-unittests/src/test/java/com/codename1/ui/spinner/PickerDefaultDateTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Loading