Fix CSSWatcher live reload: drop stale bindings + extract m2 designer jar

shai-almog · claude · shai-almog · commit 1c6a2f7261d5 · 2026-05-12T14:33:59.000+03:00
Two recent CSS/localization changes regressed the simulator's live CSS reload, in different ways. 1. addThemeProps stomped user edits with stale @cn1-bind entries. PR #4884 added applyThemeBindings() inside UIManager.buildTheme so a single addThemeProps({"@accent-color": ...}) override could retune every var()-bound theme key. But CSSWatcher reloads the theme through the same code path -- and addThemeProps never clears themeConstants. When the user replaced a `var()` rule with a literal in their CSS, the recompiled theme.res no longer emitted the matching `@cn1-bind:<key>` entry, but the previous binding was still sitting in themeConstants. applyThemeBindings happily re-overlaid the user's fresh literal value with the stale binding's resolved value, so the visible change disappeared on every reload. Fix: in buildTheme, before iterating the incoming Hashtable, detect any binding whose subject style key the new load is re-setting without re-asserting the binding alongside, and drop those bindings before the overlay pass runs. Pure `@accent-color` overrides keep working because they don't carry style keys, so no bindings are considered stale. 2. MavenUtils.findDesignerJarInM2 returned the unrunnable wrapper zip. PR #4852 added an m2 fallback for the CSSWatcher's designer-jar lookup, used whenever -Dcodename1.designer.jar isn't passed in (e.g. simulator launched from the IDE rather than `mvn cn1:run`). The helper returned `codenameone-designer-<v>-jar-with-dependencies.jar` directly from m2 -- but that artifact is a zip wrapper containing a single inner designer_1.jar (see maven/designer/pom.xml's antrun step), with no top-level Main-Class manifest. `java -jar wrapper.zip` fails with "no main manifest attribute", the CSS subprocess never starts, and the watcher silently waits for ::refresh:: lines that never come. Fix: mirror AbstractCN1Mojo.getDesignerJar's pattern -- unzip the wrapper to an `<artifact>.jar-extracted/` sibling on demand and return the inner designer_1.jar so `java -jar` actually launches. Tests: - UIManagerThemeBindingsTest gains three regression cases: cssReloadDropsStaleBindingWhenRuleBecomesLiteral (the actual reproducer), cssReloadKeepsBindingWhenStillEmittedTogether (guard against an over-eager fix), and overrideOnlyReloadKeepsBindings (repeated `@accent-color` retunes still work). The first fails before the UIManager fix; all three pass after. - MavenUtilsTest is new and covers the wrapper-vs-inner-jar resolution with five cases: happy path, re-use of extracted inner jar when the wrapper hasn't changed, re-extract when the wrapper mtime advances, null when the core jar isn't in an m2 layout, and null when the designer artifact is missing. To make these actually executable, the javase pom now pins maven-surefire-plugin to 3.2.5 (the parent's 2.21.0 doesn't auto-discover JUnit Jupiter). The pre-existing CSSWatcherTest + LocationSimulationTest + JavaSEPortFontMappingTest in the same module also start running as a side effect. - pr.yml gets a new "Run JavaSE port unit tests" step so this whole test class -- which compiled but never executed -- is wired into CI. Without it, regressions in CSSWatcher/MavenUtils/JavaSEPort helpers would continue to slip through, which was the original gap the user flagged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml
@@ -113,6 +113,19 @@ jobs:
             -Dcodename1.platform=javase \
             -Dcn1.binaries="${CN1_BINARIES}" \
             -pl codenameone-maven-plugin -am -Plocal-dev-javase test
+      - name: Run JavaSE port unit tests
+        # Surface regressions in the simulator-side helpers (CSSWatcher
+        # localization, MavenUtils designer-jar resolution, JavaSEPort font
+        # mapping). Without this step the tests in maven/javase compile but
+        # never execute on PRs.
+        working-directory: maven
+        env:
+          CN1_BINARIES: ${{ github.workspace }}/maven/target/cn1-binaries
+        run: |
+          mvn -B -Dmaven.javadoc.skip=true \
+            -Dcodename1.platform=javase \
+            -Dcn1.binaries="${CN1_BINARIES}" \
+            -pl javase -Plocal-dev-javase test
       - name: Run SpotBugs for ports and Maven plugin
         if: ${{ matrix.java-version == 8 }}
         working-directory: maven
diff --git a/CodenameOne/src/com/codename1/ui/plaf/UIManager.java b/CodenameOne/src/com/codename1/ui/plaf/UIManager.java
@@ -1790,6 +1790,31 @@ private void buildTheme(Hashtable themeProps) {
             Display.getInstance().installNativeTheme();
             accessible = a;
         }
+        // CSSWatcher's live-reload path funnels every recompile through
+        // addThemeProps -> buildTheme without first clearing themeConstants.
+        // When the user replaces a `var()`-bound CSS rule with a literal, the
+        // recompiled theme.res carries the new style value but no longer emits
+        // the matching `@cn1-bind:<key>` entry. The stale binding left over in
+        // themeConstants would otherwise let applyThemeBindings stomp the
+        // user's literal change with the previous binding's resolved value.
+        // Drop any binding whose subject key is being re-set by this load
+        // unless the same load also re-asserts the binding.
+        java.util.List<String> bindingsToDrop = null;
+        Enumeration ek = themeProps.keys();
+        while (ek.hasMoreElements()) {
+            String key = (String) ek.nextElement();
+            if (key.startsWith("@")) {
+                continue;
+            }
+            String boundConstant = "cn1-bind:" + key;
+            if (themeConstants.containsKey(boundConstant)
+                    && !themeProps.containsKey("@" + boundConstant)) {
+                if (bindingsToDrop == null) {
+                    bindingsToDrop = new java.util.ArrayList<String>();
+                }
+                bindingsToDrop.add(boundConstant);
+            }
+        }
         Enumeration e = themeProps.keys();
         while (e.hasMoreElements()) {
             String key = (String) e.nextElement();
@@ -1801,6 +1826,11 @@ private void buildTheme(Hashtable themeProps) {
             }
             this.themeProps.put(key, themeProps.get(key));
         }
+        if (bindingsToDrop != null) {
+            for (String stale : bindingsToDrop) {
+                themeConstants.remove(stale);
+            }
+        }
 
         applyThemeBindings();
 
diff --git a/Ports/JavaSE/src/com/codename1/impl/javase/util/MavenUtils.java b/Ports/JavaSE/src/com/codename1/impl/javase/util/MavenUtils.java
@@ -8,7 +8,14 @@
 import com.codename1.io.Log;
 import com.codename1.ui.Display;
 import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
 import java.net.URL;
+import java.util.zip.ZipEntry;
+import java.util.zip.ZipInputStream;
 
 /**
  *
@@ -85,7 +92,20 @@ public static File findDesignerJarInM2() {
             if (location == null) {
                 return null;
             }
-            File coreJar = new File(location.toURI());
+            return findDesignerJarInM2(new File(location.toURI()));
+        } catch (Throwable t) {
+            // Best-effort lookup. Any unexpected layout means we can't resolve via m2.
+        }
+        return null;
+    }
+
+    /**
+     * Test seam for {@link #findDesignerJarInM2()}: takes the codenameone-core jar
+     * path explicitly so the resolution logic can be exercised against a fake m2
+     * layout in a temp directory.
+     */
+    static File findDesignerJarInM2(File coreJar) {
+        try {
             // Expected layout: <repo>/com/codenameone/codenameone-core/<version>/codenameone-core-<version>.jar
             File versionDir = coreJar.getParentFile();
             if (versionDir == null) return null;
@@ -98,16 +118,68 @@ public static File findDesignerJarInM2() {
             }
             String version = versionDir.getName();
             File designerVersionDir = new File(codenameoneGroupDir, "codenameone-designer" + File.separator + version);
-            File designer = new File(designerVersionDir, "codenameone-designer-" + version + "-jar-with-dependencies.jar");
-            if (designer.isFile()) {
-                return designer;
+            // The published jar-with-dependencies artifact is *not* directly runnable:
+            // maven/designer/pom.xml's antrun step renames the shaded jar to
+            // designer_1.jar and re-zips it, so this file is a zip wrapper containing
+            // a single designer_1.jar entry with no top-level Main-Class manifest.
+            // AbstractCN1Mojo.getDesignerJar (in the maven plugin) unzips it on demand
+            // and returns the inner jar; we mirror that here so the CSSWatcher
+            // fallback path receives a path that `java -jar` can actually launch.
+            File wrapperZip = new File(designerVersionDir, "codenameone-designer-" + version + "-jar-with-dependencies.jar");
+            if (!wrapperZip.isFile()) {
+                return null;
+            }
+            File extracted = new File(wrapperZip.getParentFile(), wrapperZip.getName() + "-extracted");
+            File innerJar = new File(extracted, "designer_1.jar");
+            if (!innerJar.isFile() || innerJar.lastModified() < wrapperZip.lastModified()) {
+                extractInnerJar(wrapperZip, extracted);
+            }
+            if (innerJar.isFile()) {
+                return innerJar;
             }
         } catch (Throwable t) {
             // Best-effort lookup. Any unexpected layout means we can't resolve via m2.
         }
         return null;
     }
 
+    private static void extractInnerJar(File wrapperZip, File destDir) throws IOException {
+        if (!destDir.exists() && !destDir.mkdirs() && !destDir.isDirectory()) {
+            throw new IOException("Could not create designer extraction directory: " + destDir.getAbsolutePath());
+        }
+        InputStream in = new FileInputStream(wrapperZip);
+        try {
+            ZipInputStream zis = new ZipInputStream(in);
+            try {
+                ZipEntry entry;
+                while ((entry = zis.getNextEntry()) != null) {
+                    if (entry.isDirectory()) {
+                        continue;
+                    }
+                    File out = new File(destDir, entry.getName());
+                    File parent = out.getParentFile();
+                    if (parent != null && !parent.exists() && !parent.mkdirs() && !parent.isDirectory()) {
+                        throw new IOException("Could not create directory: " + parent.getAbsolutePath());
+                    }
+                    OutputStream fos = new FileOutputStream(out);
+                    try {
+                        byte[] buf = new byte[8192];
+                        int n;
+                        while ((n = zis.read(buf)) > 0) {
+                            fos.write(buf, 0, n);
+                        }
+                    } finally {
+                        fos.close();
+                    }
+                }
+            } finally {
+                zis.close();
+            }
+        } finally {
+            in.close();
+        }
+    }
+
     public static boolean isRunningInJDK() {
         if (!isRunningInJDKChecked) {
             isRunningInJDKChecked = true;
diff --git a/maven/core-unittests/src/test/java/com/codename1/ui/plaf/UIManagerThemeBindingsTest.java b/maven/core-unittests/src/test/java/com/codename1/ui/plaf/UIManagerThemeBindingsTest.java
@@ -126,4 +126,77 @@ public void invalidColorOverrideLeavesDefaultIntact() {
         b.setUIID("Button");
         assertEquals(0x007aff, b.getUnselectedStyle().getFgColor());
     }
+
+    /// Simulates CSSWatcher's live-reload sequence: an initial theme load
+    /// (`setThemeProps`) followed by `addThemeProps` carrying a fresh
+    /// theme.res whose source CSS has dropped a `var()` reference in favor of
+    /// a literal. The reloaded Hashtable contains the new literal value for
+    /// `Button.fgColor` but no `@cn1-bind:Button.fgColor` entry, because the
+    /// CSS compiler only emits bindings for properties that still reference a
+    /// `var()`. The stale binding left in `themeConstants` from the first
+    /// load must not stomp the user's new literal value.
+    @Test
+    public void cssReloadDropsStaleBindingWhenRuleBecomesLiteral() {
+        Hashtable initial = new Hashtable();
+        initial.put("Button.fgColor", "ff0000");
+        initial.put("@cn1-bind:Button.fgColor", "accent-color");
+        initial.put("@accent-color", "ff0000");
+        UIManager.getInstance().setThemeProps(initial);
+
+        Hashtable reloaded = new Hashtable();
+        reloaded.put("Button.fgColor", "0000ff");
+        UIManager.getInstance().addThemeProps(reloaded);
+
+        Button b = new Button();
+        b.setUIID("Button");
+        assertEquals(0x0000ff, b.getUnselectedStyle().getFgColor());
+    }
+
+    /// Companion to [#cssReloadDropsStaleBindingWhenRuleBecomesLiteral]: when
+    /// the reload Hashtable carries BOTH the property and a fresh binding,
+    /// the binding still applies. This guards against an over-eager fix that
+    /// would drop bindings every time a style key shows up in the reload.
+    @Test
+    public void cssReloadKeepsBindingWhenStillEmittedTogether() {
+        Hashtable initial = new Hashtable();
+        initial.put("Button.fgColor", "ff0000");
+        initial.put("@cn1-bind:Button.fgColor", "accent-color");
+        initial.put("@accent-color", "ff0000");
+        UIManager.getInstance().setThemeProps(initial);
+
+        Hashtable reloaded = new Hashtable();
+        reloaded.put("Button.fgColor", "0000ff");
+        reloaded.put("@cn1-bind:Button.fgColor", "accent-color");
+        reloaded.put("@accent-color", "00ff00");
+        UIManager.getInstance().addThemeProps(reloaded);
+
+        Button b = new Button();
+        b.setUIID("Button");
+        assertEquals(0x00ff00, b.getUnselectedStyle().getFgColor());
+    }
+
+    /// A pure override Hashtable (no style keys, only a single `@varname`
+    /// constant) must not invalidate the existing bindings. This is the
+    /// canonical "user rebrands the accent" call path and the existing
+    /// [#boundThemeKeyPicksUpAccentOverride] covers a single hop; this test
+    /// adds a follow-up override to make sure repeated retunes keep working.
+    @Test
+    public void overrideOnlyReloadKeepsBindings() {
+        Hashtable initial = new Hashtable();
+        initial.put("Button.fgColor", "007aff");
+        initial.put("@cn1-bind:Button.fgColor", "accent-color");
+        UIManager.getInstance().setThemeProps(initial);
+
+        Hashtable firstOverride = new Hashtable();
+        firstOverride.put("@accent-color", "ff2d95");
+        UIManager.getInstance().addThemeProps(firstOverride);
+
+        Hashtable secondOverride = new Hashtable();
+        secondOverride.put("@accent-color", "00aa66");
+        UIManager.getInstance().addThemeProps(secondOverride);
+
+        Button b = new Button();
+        b.setUIID("Button");
+        assertEquals(0x00aa66, b.getUnselectedStyle().getFgColor());
+    }
 }
diff --git a/maven/javase/pom.xml b/maven/javase/pom.xml
@@ -125,6 +125,17 @@
             </resource>
         </resources>
         <plugins>
+            <plugin>
+                <!-- The parent pom pins surefire to 2.21.0 which lacks the
+                     JUnit Jupiter provider this module's tests rely on (the
+                     pre-existing CSSWatcherTest and the MavenUtilsTest added
+                     alongside the CSSWatcher live-reload fixes). Bump to a
+                     surefire version with native Jupiter discovery so these
+                     tests actually execute. -->
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-surefire-plugin</artifactId>
+                <version>3.2.5</version>
+            </plugin>
             <plugin>
                 <artifactId>maven-antrun-plugin</artifactId>
                 <executions>
diff --git a/maven/javase/src/test/java/com/codename1/impl/javase/util/MavenUtilsTest.java b/maven/javase/src/test/java/com/codename1/impl/javase/util/MavenUtilsTest.java
