Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
50 changes: 36 additions & 14 deletions CodenameOne/src/com/codename1/ui/spinner/Picker.java
Original file line number Diff line number Diff line change
Expand Up @@ -116,11 +116,14 @@ public class Picker extends Button {
/// default tracks "one hour before the current due date" can keep returning
/// fresh values as the due date moves. Null means fall back to `new Date()`.
private DateGetter defaultDateGetter;
/// True once the picker's date has been pinned by an explicit
/// `setDate(non-null)` call or a successful user selection. While false the
/// picker treats `value` as a placeholder and prefers `defaultDateGetter`
/// (when set) as the displayed value, so callers can install a default
/// after construction and still see it take effect.
/// True once the picker's date has been pinned by an explicit `setDate(...)`
/// call (including `setDate(null)` to clear) or a successful user selection.
/// While false the picker treats `value` as a placeholder and prefers
/// `defaultDateGetter` (when set) as the displayed value, so callers can
/// install a default after construction and still see it take effect. Once
/// the caller has explicitly set a value - even `null` - `getDate()` returns
/// exactly that value; the default getter only continues to seed the popup
/// UI when the picker is opened with a `null` value. Issue #5024.
private boolean dateValueExplicitlySet;

// Variables to store the form's previous margins before showing
Expand Down Expand Up @@ -1367,6 +1370,9 @@ public Date getDate() {
}
// No explicit setDate yet -> let the default getter take over so the
// value returned here matches what the picker would show if opened now.
// Once the caller has explicitly set a value (including null via
// setDate(null)) we return it as-is; the default is reserved for
// seeding the popup UI in applyDefaultDateIfNeeded(). Issue #5024.
if (!dateValueExplicitlySet && defaultDateGetter != null) {
return resolveDefaultDate();
}
Expand All @@ -1375,13 +1381,19 @@ public Date getDate() {

/// Primes `value` with the resolved default before a date-type picker is
/// shown, so the existing show paths (which read `value` directly) display
/// the configured default. Does nothing for non-date types, when the
/// caller has already pinned a date with `setDate(Date)`, or when no
/// default getter has been configured - in that last case the picker
/// the configured default. Applies whenever the picker has no usable date
/// to show - either the caller never pinned one with `setDate(Date)`, or
/// they explicitly cleared it with `setDate(null)`. Does nothing for
/// non-date types, when there's already a non-null pinned date, or when
/// no default getter has been configured - in that last case the picker
/// keeps the legacy behavior of showing whatever was in `value` (typically
/// the construction-time `new Date()`).
/// the construction-time `new Date()`, or `null` after a `setDate(null)`).
/// Issue #5024.
private void applyDefaultDateIfNeeded() {
if (dateValueExplicitlySet || defaultDateGetter == null) {
if (defaultDateGetter == null) {
return;
}
if (dateValueExplicitlySet && value != null) {
return;
}
switch (type) {
Expand Down Expand Up @@ -1428,14 +1440,24 @@ private void markDateExplicitlySetIfDateType() {
/// native popup is open updates the committed `value` but leaves the on-screen wheels
/// unchanged until the user dismisses and re-opens the picker.
///
/// Passing `null` explicitly clears the picker's value: subsequent `getDate()` calls
/// return `null` (the picker text falls back to the `...` placeholder) until either
/// `setDate(non-null)` is called or the user opens the popup and commits a selection.
/// A configured `setDefaultDate` getter still seeds the popup wheels on the next open
/// so the user has a sensible starting point, but it no longer leaks into `getDate()`
/// as an unselected value. Issue #5024.
///
/// #### Parameters
///
/// - `d`: the new date
/// - `d`: the new date, or `null` to clear the picker
public void setDate(Date d) {
value = d;
// A null clears the explicit value, restoring the default-date behavior
// (so a subsequent setDefaultDate / new Date() fallback takes effect again).
dateValueExplicitlySet = d != null;
// Mark as explicitly set even when d is null, so getDate() returns the
// caller's chosen value (null included) instead of silently substituting
// the default getter's result. The default still primes the popup wheels
// through applyDefaultDateIfNeeded() when the picker is opened against a
// null value, so the UI keeps a usable starting point. Issue #5024.
dateValueExplicitlySet = true;
if (currentSpinner != null) {
currentSpinner.setValue(d);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,16 +65,42 @@ public void explicitSetDateOverridesDefault() {
"Once a date is explicitly pinned, the default getter must not be consulted");
}

/// Regression for #5024: `setDate(null)` is an explicit clear, not a request
/// to fall back to the default getter for `getDate()`. Before the fix,
/// `getDate()` after `setDate(null)` returned the default-getter result,
/// effectively pretending the user had accepted a date they never saw.
@Test
public void setDateNullReEnablesDefault() {
public void setDateNullClearsValueEvenWhenDefaultGetterInstalled() {
Picker p = new Picker();
p.setType(Display.PICKER_TYPE_DATE);
Date defaultDate = date(2030, Calendar.JUNE, 15);
p.setDefaultDate(defaultDate);
p.setDate(date(2028, Calendar.MARCH, 20));
p.setDate(null);
Assertions.assertEquals(defaultDate, p.getDate(),
"setDate(null) should clear the explicit pin and restore the default");
Assertions.assertNull(p.getDate(),
"setDate(null) must clear the picker's value rather than silently restoring the default (#5024)");
Assertions.assertEquals("...", p.getText(),
"setDate(null) must leave the picker text showing the placeholder");
}

/// Regression for #5024: even when the caller never pinned a non-null
/// value first, `setDate(null)` should still latch as an explicit clear.
/// This matches the original report, where the picker was constructed,
/// configured with `setDefaultDate(...)`, and then `setDate(null)` was
/// called before the user ever interacted with it.
@Test
public void setDateNullOnFreshPickerReturnsNull() {
Picker p = new Picker();
p.setType(Display.PICKER_TYPE_DATE);
p.setDefaultDate(new Picker.DateGetter() {
@Override
public Date get() {
return new Date();
}
});
p.setDate(null);
Assertions.assertNull(p.getDate(),
"After setDate(null), getDate() must return null even if the picker was never opened (#5024)");
}

@Test
Expand Down Expand Up @@ -185,18 +211,14 @@ public Date get() {

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()");
// Proof the staged default did not leak past Cancel: the explicit
// setDate(null) state must survive, so getDate() returns null rather
// than the staged default that briefly populated `value` while the
// popup was shown. With the #5024 fix, setDate(null) is a latched
// explicit clear; a non-null result here would indicate Cancel left
// the staged default behind.
Assertions.assertNull(picker.getDate(),
"After Cancel the explicit setDate(null) state must survive (#5024, #5014)");
}

@FormTest
Expand Down
Loading