Fix CSSWatcher live reload: drop stale bindings + extract m2 designer jar#4929
Merged
Conversation
… jar 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>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Collaborator
Author
|
Compared 7 screenshots: 7 matched. |
Collaborator
Author
|
Compared 15 screenshots: 15 matched. |
Collaborator
Author
|
Compared 105 screenshots: 105 matched. Native Android coverage
✅ Native Android screenshot tests passed. Native Android coverage
Benchmark ResultsDetailed Performance Metrics
|
- MavenUtils.extractInnerJar no longer derives a File path from ZipEntry.getName(). CodeQL flagged the previous loop as a Zip Slip risk because a wrapper containing `../../etc/passwd` would have been written outside the extraction directory. The wrapper produced by maven/designer/pom.xml has a single designer_1.jar entry by design, so the extractor now (a) writes only to a single fixed destination path under destDir and (b) only matches entries whose literal name equals "designer_1.jar". Anything else is skipped; if the canonical entry is absent, the method throws. Two new MavenUtilsTest cases: refusesPathTraversalEntriesAndDoesNotWriteOutsideExtractDir packs a `../../escaped.txt` entry and asserts no escaped file appears in the temp root; skipsUnexpectedEntriesAndStillExtractsDesignerJar mixes a README and a subdir/other.jar with the real designer_1.jar and asserts only the inner jar lands on disk. - pr.yml's new "Run JavaSE port unit tests" step failed with "Could not find artifact com.codenameone:sqlite-jdbc:jar:8.0-SNAPSHOT" on all three matrix entries (Java 8/17/21). The earlier "Build Codename One" step builds core-unittests with -am, which doesn't install sqlite-jdbc into the local repo. Split the new step into two mvn invocations: first install javase's transitive deps without running their tests, then run javase's tests in isolation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
✅ Continuous Quality ReportTest & Coverage
Static Analysis
Generated automatically by the PR CI workflow. |
The previous fix ran the stale-binding preprocessing inside buildTheme, which is also called from the @includeNativeBool layered initial load (setThemePropsImpl -> buildTheme -> Display.installNativeTheme() -> buildTheme(native) -> outer buildTheme(userTheme) continues). After the native theme installs its bindings into themeConstants, the outer call's preprocessing would drop them whenever the user's app theme.css set a literal value for the same UIID -- which the existing iOS / Android screenshot goldens were captured against. The iOS PR check hit this: the device-runner log shows the suite ran fine through ChartCubicLineScreenshotTest and then hung in ChartBarScreenshotTest setup until the 30-minute timeout fired. The inconsistent themeConstants state left over once the layered native bindings were dropped manifests as a hang in chart-component initialization (presumably a Style.derive cycle or similar) rather than as a pixel diff. Move the drop pre-pass out of buildTheme and into a new dropSupersededBindings() called only from addThemeProps. This keeps the CSSWatcher reload fix (the actual reported regression) and the companion regression tests passing, while restoring the original behavior of the layered initial-load path -- bindings declared by the native theme via @includeNativeBool stay live, user-app literals don't silently strip them out. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
Compared 104 screenshots: 104 matched. Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
Collaborator
Author
|
Compared 105 screenshots: 105 matched. Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
shai-almog
added a commit
that referenced
this pull request
May 16, 2026
PR #4929 (May 12) bumped codenameone-javase's surefire to 3.2.5 so its JUnit Jupiter tests would run. Surefire 3.x renamed `failIfNoTests` -> `failIfNoSpecifiedTests`, and designer.yml's reactor build invokes `-Dtest=SimpleXmlParserTest -DfailIfNoTests=false` to suppress "no tests matched" on intermediate modules. The flag was silently ignored by 3.2.5, so the codenameone-javase test phase began failing on the next run. designer.yml's path filter excludes `maven/javase/**`, so the bug didn't surface on master after the surefire bump - it only manifested on a PR that touches the designer workflow's trigger paths (maven/css-compiler/**, maven/designer/**, or CodenameOneDesigner/**). This PR touches css-compiler, so it surfaces here. Pass both flag names so each surefire version finds the one it understands. codenameone-javase (3.2.5) reads `surefire.failIfNoSpecifiedTests`; peer modules still on parent-pom 2.21.0 read `failIfNoTests`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
shai-almog
added a commit
that referenced
this pull request
May 16, 2026
Three reviewer-driven changes:
1. Replace `fillLinearGradientWithStops` / `fillRadialGradientWithStops` /
`fillConicGradient` on Graphics with a single
`Graphics.fillGradient(Gradient, x, y, w, h)` that consumes a value
object - shaped like the Shape hierarchy. Three concrete subclasses:
* LinearGradient(angleDegrees, colors, positions)
* RadialGradient(colors, positions) + shape/extent/center/radius setters
* ConicGradient(colors, positions) + fromAngle/center setters
`Gradient` is a `Paint` subclass with shared stops, cycle method
(NONE / REPEAT / REFLECT) and a `sampleArgb` hook the base impl uses
for the software-rasterizer fallback. Ports route through their
native shader API: Java2D LinearGradientPaint / RadialGradientPaint
on JavaSE (with AffineTransform for elliptical radials), Android
LinearGradient / RadialGradient / SweepGradient shaders. iOS still
falls back to the software rasterizer.
Style.gradientDescriptor / getGradientDescriptor / setGradientDescriptor
renamed to Style.gradient / getGradient / setGradient. The .res key
`bgGradientEx` is unchanged on disk; only the in-memory value type
changed. The deleted `com.codename1.ui.plaf.GradientDescriptor` had
no callers outside this branch.
2. Pin maven-surefire-plugin to 3.2.5 uniformly in the parent pom
(instead of per-module in maven/javase/pom.xml as PR #4929 did).
Revert the dual-flag hack in designer.yml; the single new
`surefire.failIfNoSpecifiedTests` flag now suffices everywhere.
3. Fix Android instrumentation suite hang at DrawGradientStops. The
previous AndroidImplementation.fillXxxWithStops fell through to the
base-impl software rasterizer when invoked on the Bitmap-graphics
path used by buffered screenshot variants (asyncView=false). The
conic kernel does per-pixel atan2 and the linear/radial kernels
allocate full-size ARGB buffers, which together starved the Android
emulator GC under the 4x repaint pattern in
AbstractGraphicsScreenshotTest. After the refactor
AndroidImplementation.fillGradient unconditionally routes to
AndroidGraphics.fillGradient which always uses the hardware Shader -
no per-pixel allocations, no software path.
Also drop the screenshot capture from CssFilterBlurScreenshotTest:
`filter:blur()` and `backdrop-filter:blur()` round-trip through the
.res into Style fields, but Component.paint doesn't yet consume the
radius (that's a follow-up using Graphics.gaussianBlur). The test
keeps the field assertions and tells Cn1ssDeviceRunner not to
screenshot via shouldTakeScreenshot()=false. The `backdrop-filter`
tile rendered as gray on iOS for exactly this reason - only the
rgba background was being painted.
Verified by full reactor `mvn install -Plocal-dev-javase`, Android
`mvn -pl android -am compile` under JDK17, hellocodenameone common
compile, and `mvn -pl core-unittests test -Dtest=CSSBorderTest` - all
exit 0.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
CSSWatcher's live reload stopped applying CSS edits in two distinct ways after the recent CSS/localization push. This PR fixes both, adds regression tests, and wires the maven/javase test module into CI so similar regressions are caught next time.
1.
addThemePropswas stomping user edits with stale@cn1-bindentriesPR #4884 added
applyThemeBindings()insideUIManager.buildThemeso a singleaddThemeProps({"@accent-color": ...})override could retune everyvar()-bound theme key. CSSWatcher reloads the recompiledtheme.resthrough the same code path — butaddThemePropsnever clearsthemeConstants.When the user replaced a
var()CSS rule with a literal, the recompiledtheme.resno longer emitted the matching@cn1-bind:<key>entry, but the previous binding was still sitting inthemeConstantsfrom the initial load.applyThemeBindingshappily 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. Pure@accent-color-only overrides keep working because they don't carry style keys, so no bindings are considered stale.2.
MavenUtils.findDesignerJarInM2returned the unrunnable wrapper zipPR #4852 added an m2 fallback for the CSSWatcher's designer-jar lookup, used whenever
-Dcodename1.designer.jarisn't passed in (e.g. simulator launched from the IDE rather thanmvn cn1:run). The helper returnedcodenameone-designer-<v>-jar-with-dependencies.jardirectly from m2 — but that artifact is a zip wrapper containing a single innerdesigner_1.jar(seemaven/designer/pom.xml's antrun step), with no top-levelMain-Classmanifest.java -jar wrapper.zipfails 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 innerdesigner_1.jarsojava -jaractually launches.Tests
UIManagerThemeBindingsTestgains three regression cases:cssReloadDropsStaleBindingWhenRuleBecomesLiteral— the actual reproducer; fails before the fixcssReloadKeepsBindingWhenStillEmittedTogether— guard against an over-eager fix that drops bindings on every reloadoverrideOnlyReloadKeepsBindings— repeated@accent-colorretunes still workMavenUtilsTest(new) covers the wrapper-vs-inner-jar resolution with five cases: happy path, re-use of extracted 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.maven-surefire-pluginto 3.2.5 (the parent's 2.21.0 doesn't auto-discover JUnit Jupiter — that's why the pre-existingCSSWatcherTestetc. compiled but never ran).pr.ymlgets a new "Run JavaSE port unit tests" step. Without it, the CSSWatcher/MavenUtils/JavaSEPort helper regressions would continue to slip through — which is the original gap behind this issue.Test plan
mvn -pl core-unittests test— all binding tests pass, including the new reproducer; pre-existingBorderAndPlafTest.testRoundBorderShadowSpreadAndPaintingCachesflake reproduces on master (cross-test pollution, unrelated to this PR)mvn -pl javase test -Plocal-dev-javase— 15 tests pass (CSSWatcherTest× 2,LocationSimulationTest× 3,JavaSEPortFontMappingTest× 5,MavenUtilsTest× 5)theme.cssin a generated initializr project, confirm the simulator's running form picks up both literal andvar()changes without restart🤖 Generated with Claude Code