diff --git a/.github/workflows/scripts-javascript.yml b/.github/workflows/scripts-javascript.yml index e6aa710c32..1b6c4b95c8 100644 --- a/.github/workflows/scripts-javascript.yml +++ b/.github/workflows/scripts-javascript.yml @@ -107,7 +107,7 @@ jobs: uses: actions/cache@v4 with: path: ~/.cache/ms-playwright - key: ${{ runner.os }}-playwright-chromium-v1 + key: ${{ runner.os }}-playwright-chromium-v2 restore-keys: | ${{ runner.os }}-playwright- @@ -116,11 +116,16 @@ jobs: cd scripts npm init -y 2>/dev/null || true npm install playwright - if [ "${{ steps.playwright-cache.outputs.cache-hit }}" = "true" ]; then - npx playwright install-deps chromium - else - npx playwright install --with-deps chromium - fi + # OS-level deps (apt packages) aren't cached so always install them. + npx playwright install-deps chromium + # `npm install playwright` resolves the floating version, so the + # cached browser binary can drift away from what the freshly + # installed Playwright expects (the cache only stores the binary). + # `install chromium` is a no-op when the versions match and + # re-downloads the right build when they don't, so it makes the + # job resilient to Playwright revving its bundled chromium build + # in between cache writes. + npx playwright install chromium - name: Install Xvfb for headless Java AWT run: sudo apt-get update && sudo apt-get install -y xvfb diff --git a/CodenameOne/src/com/codename1/ui/spinner/Picker.java b/CodenameOne/src/com/codename1/ui/spinner/Picker.java index b5be0ac298..ef9337a6c1 100644 --- a/CodenameOne/src/com/codename1/ui/spinner/Picker.java +++ b/CodenameOne/src/com/codename1/ui/spinner/Picker.java @@ -124,6 +124,13 @@ public class Picker extends Button { /// so that setters propagate into the visible wheels and getters read the wheel /// position. Null whenever the popup is not showing. private InternalPickerWidget currentSpinner; + /// Snapshot of `value` taken right before the lightweight popup is shown. + /// Setter calls from custom popup buttons (e.g. a "+7 days" action that does + /// `setDate(getDate() + 7d)`) stage their result into `value`, but if the user + /// dismisses the popup with Cancel we roll `value` back to this snapshot so + /// `getDate()` returns what the picker held before editing began. Non-null only + /// while the popup is on screen. + private Object preEditValue; /// Placement options for custom lightweight popup buttons. public static final class LightweightPopupButtonPlacement { @@ -493,7 +500,15 @@ private void endEditing(int command, InteractionDialog dlg, InternalPickerWidget } restoreContentPane(); dlg.disposeToTheBottom(); - if (command != COMMAND_CANCEL) { + if (command == COMMAND_CANCEL) { + // Roll back any setX calls made while the popup was showing + // (e.g. a custom "+7 days" button) so getDate() returns the + // value the picker held before editing began. + value = preEditValue; + preEditValue = null; + updateValue(); + } else { + preEditValue = null; value = spinner.getValue(); updateValue(); // (x, y) = (-99, -99) signals the built-in action listner @@ -545,6 +560,13 @@ 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; final InteractionDialog dlg = new InteractionDialog() { ActionListener keyListener; @@ -737,7 +759,7 @@ public void actionPerformed(ActionEvent evt) { dlg.setY(Display.getInstance().getDisplayHeight()); dlg.setX(0); dlg.setRepositionAnimation(false); - registerAsInputDevice(dlg); + registerAsInputDevice(dlg, spinner); if (Display.getInstance().isTablet()) { getComponentForm().getAnimationManager().flushAnimation(new Runnable() { @@ -1238,6 +1260,9 @@ public Date getDate() { /// If the lightweight popup is currently on screen the visible scroll wheels are also /// moved to the new value, so a custom popup button can do `setDate(getDate() + n)` and /// have the wheels reflect it (see `#addLightweightPopupButton(String, Runnable)`). + /// Such changes are *staged*: if the user dismisses the popup with Cancel the picker + /// rolls back to the date it held before the popup was shown; if the user presses Done + /// the staged value is committed. /// The native picker is read-only while shown - calling `setDate` against a Picker whose /// native popup is open updates the committed `value` but leaves the on-screen wheels /// unchanged until the user dismisses and re-opens the picker. @@ -1329,7 +1354,7 @@ public void run() { } } - private void registerAsInputDevice(final InteractionDialog dlg) { + private void registerAsInputDevice(final InteractionDialog dlg, final InternalPickerWidget spinner) { final Form f = this.getComponentForm(); if (f != null) { @@ -1374,8 +1399,19 @@ public void run() { @Override public void close() throws Exception { - currentInput = null; - currentSpinner = null; + // Only null the picker-wide fields if they still point at THIS popup. + // When a second popup opens before the form has finished switching + // input devices, Form#setCurrentInputDevice calls close() on the + // previous device AFTER showInteractionDialog has already assigned + // currentSpinner / currentInput to the new popup; unconditionally + // nulling them here would clobber the new popup's live spinner and + // break setX propagation. + if (currentInput == this) { //NOPMD CompareObjectsWithEquals + currentInput = null; + } + if (currentSpinner == spinner) { //NOPMD CompareObjectsWithEquals + currentSpinner = null; + } if (sizeChanged != null) { f.removeSizeChangedListener(sizeChanged); } @@ -1515,8 +1551,10 @@ public String getSelectedString() { } /// Sets the current value in a string array picker. If the lightweight popup is on screen - /// the visible scroll wheel is also moved to the new value; the native picker is read-only - /// while shown so against a native popup this updates only the committed value. + /// the visible scroll wheel is also moved to the new value, and the change is *staged*: + /// a Cancel press rolls back to the value the picker held before the popup was shown, + /// while a Done press commits it. The native picker is read-only while shown so against + /// a native popup this updates only the committed value. /// /// #### Parameters /// @@ -1558,8 +1596,10 @@ public int getSelectedStringIndex() { } /// Sets the index of the selected string. If the lightweight popup is on screen the - /// visible wheel is also moved to the new value; the native picker is read-only while - /// shown so against a native popup this updates only the committed value. + /// visible wheel is also moved to the new value, and the change is *staged*: a Cancel + /// press rolls back to the value the picker held before the popup was shown, while a + /// Done press commits it. The native picker is read-only while shown so against a + /// native popup this updates only the committed value. /// /// #### Parameters /// @@ -1676,8 +1716,10 @@ public int getTime() { /// This value is only used for time type and is ignored in the case of date and time where /// both are embedded within the date. If the lightweight popup is on screen the visible - /// scroll wheels are also moved to the new value; the native picker is read-only while - /// shown so against a native popup this updates only the committed value. + /// scroll wheels are also moved to the new value, and the change is *staged*: a Cancel + /// press rolls back to the value the picker held before the popup was shown, while a + /// Done press commits it. The native picker is read-only while shown so against a native + /// popup this updates only the committed value. /// /// #### Parameters /// @@ -1738,8 +1780,10 @@ public long getDuration() { } /// This value is only used for duration type. If the lightweight popup is on screen the - /// visible scroll wheels are also moved to the new value; the native picker is read-only - /// while shown so against a native popup this updates only the committed value. + /// visible scroll wheels are also moved to the new value, and the change is *staged*: + /// a Cancel press rolls back to the value the picker held before the popup was shown, + /// while a Done press commits it. The native picker is read-only while shown so against + /// a native popup this updates only the committed value. /// /// #### Parameters /// diff --git a/scripts/hellocodenameone/common/src/main/java/com/codenameone/examples/hellocodenameone/tests/Cn1ssDeviceRunner.java b/scripts/hellocodenameone/common/src/main/java/com/codenameone/examples/hellocodenameone/tests/Cn1ssDeviceRunner.java index a62561c108..5071520dcd 100644 --- a/scripts/hellocodenameone/common/src/main/java/com/codenameone/examples/hellocodenameone/tests/Cn1ssDeviceRunner.java +++ b/scripts/hellocodenameone/common/src/main/java/com/codenameone/examples/hellocodenameone/tests/Cn1ssDeviceRunner.java @@ -176,6 +176,7 @@ private static int testTimeoutMs() { new TextAreaAlignmentScreenshotTest(), new ValidatorLightweightPickerScreenshotTest(), new LightweightPickerButtonsScreenshotTest(), + new PickerCancelRestoreTest(), new ToastBarTopPositionScreenshotTest(), // Native-theme fidelity tests (Phase 7): each emits a light+dark PNG pair // so the iOS Modern and Android Material themes get exercised per UIID. diff --git a/scripts/hellocodenameone/common/src/main/java/com/codenameone/examples/hellocodenameone/tests/PickerCancelRestoreTest.java b/scripts/hellocodenameone/common/src/main/java/com/codenameone/examples/hellocodenameone/tests/PickerCancelRestoreTest.java new file mode 100644 index 0000000000..ac6150d1ac --- /dev/null +++ b/scripts/hellocodenameone/common/src/main/java/com/codenameone/examples/hellocodenameone/tests/PickerCancelRestoreTest.java @@ -0,0 +1,176 @@ +package com.codenameone.examples.hellocodenameone.tests; + +import com.codename1.ui.Button; +import com.codename1.ui.Component; +import com.codename1.ui.Container; +import com.codename1.ui.Display; +import com.codename1.ui.Form; +import com.codename1.ui.layouts.BoxLayout; +import com.codename1.ui.spinner.Picker; +import com.codename1.ui.util.UITimer; + +import java.time.LocalDate; +import java.time.ZoneId; +import java.util.Date; + +/** + * Regression test for the staging behavior layered on top of #4897: + * `setX` calls made via custom lightweight popup buttons while the + * picker is showing must be staged. If the user dismisses with Cancel + * the staged value rolls back to whatever the picker held before the + * popup was shown; if the user presses Done the staged value is + * committed. Drives the picker programmatically and asserts + * {@code getDate()} after each dismiss path. + */ +public class PickerCancelRestoreTest extends BaseTest { + private Form form; + private Picker picker; + private Date initialDate; + private Date stagedDate; + + @Override + public boolean shouldTakeScreenshot() { + return false; + } + + @Override + public boolean runTest() { + initialDate = toDate(LocalDate.of(2026, 4, 11)); + stagedDate = toDate(LocalDate.of(2026, 4, 18)); + + form = new Form("Picker Cancel Restore", BoxLayout.y()) { + @Override + protected void onShowCompleted() { + runCancelScenario(); + } + }; + picker = new Picker(); + picker.setType(Display.PICKER_TYPE_DATE); + picker.setUseLightweightPopup(true); + picker.setDate(initialDate); + picker.addLightweightPopupButton("+7 Days", new Runnable() { + @Override + public void run() { + picker.setDate(stagedDate); + } + }); + form.add(picker); + form.show(); + return true; + } + + private void runCancelScenario() { + picker.startEditingAsync(); + UITimer.timer(600, false, form, new Runnable() { + @Override + public void run() { + if (!clickPopupButton("+7 Days")) { + return; + } + if (!datesEqual(stagedDate, picker.getDate())) { + fail("Staged setDate did not take effect during edit. Expected " + + stagedDate + " but got " + picker.getDate()); + return; + } + if (!clickPopupButton("Cancel")) { + return; + } + // Cancel dismisses the popup; give the dialog a frame to + // settle so the picker has run its rollback before we read. + UITimer.timer(600, false, form, new Runnable() { + @Override + public void run() { + if (!datesEqual(initialDate, picker.getDate())) { + fail("Cancel did not restore initial date. Expected " + + initialDate + " but got " + picker.getDate()); + return; + } + runDoneScenario(); + } + }); + } + }); + } + + private void runDoneScenario() { + picker.startEditingAsync(); + UITimer.timer(600, false, form, new Runnable() { + @Override + public void run() { + if (!clickPopupButton("+7 Days")) { + return; + } + if (!clickPopupButton("Done")) { + return; + } + UITimer.timer(600, false, form, new Runnable() { + @Override + public void run() { + if (!datesEqual(stagedDate, picker.getDate())) { + fail("Done did not commit staged date. Expected " + + stagedDate + " but got " + picker.getDate()); + return; + } + done(); + } + }); + } + }); + } + + private boolean clickPopupButton(String text) { + Button btn = findButtonByText(form, text); + if (btn == null) { + fail("Could not find button with text '" + text + "' in the picker popup"); + return false; + } + btn.pressed(); + btn.released(); + return true; + } + + private static Button findButtonByText(Container c, String text) { + for (Component child : c) { + if (child instanceof Button && matchesButtonText((Button) child, text)) { + return (Button) child; + } + if (child instanceof Container) { + Button result = findButtonByText((Container) child, text); + if (result != null) { + return result; + } + } + } + return null; + } + + private static boolean matchesButtonText(Button btn, String text) { + // Button.setText() uppercases when capsTextDefault is on (Material + // theme default), so a case-insensitive compare matches both + // "Cancel" and "CANCEL"; the pre-caps original is also stashed in + // a client property which we check as a second chance. + if (text.equalsIgnoreCase(btn.getText())) { + return true; + } + Object orig = btn.getClientProperty("cn1$origText"); + return orig instanceof String && text.equalsIgnoreCase((String) orig); + } + + private static boolean datesEqual(Date expected, Date actual) { + if (expected == actual) { + return true; + } + if (expected == null || actual == null) { + return false; + } + // The picker's DateSpinner3D commits midnight-of-day, so a tolerant + // day-granularity compare keeps the test stable against the wheel + // wrapping the time component when Done is pressed. + return expected.getTime() / (24L * 60 * 60 * 1000) + == actual.getTime() / (24L * 60 * 60 * 1000); + } + + private static Date toDate(LocalDate date) { + return new Date(date.atStartOfDay(ZoneId.systemDefault()).toInstant().toEpochMilli()); + } +}