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
88 changes: 84 additions & 4 deletions Ports/JavaSE/src/com/codename1/impl/javase/JavaSEPort.java
Original file line number Diff line number Diff line change
Expand Up @@ -4678,7 +4678,7 @@ public void actionPerformed(ActionEvent ae) {
public void actionPerformed(ActionEvent e) {
JavaSEPort.this.darkMode = Boolean.TRUE;
pref.put("cn1.simulator.darkMode", "dark");
refreshSkin(frm);
refreshThemeOnly();
}
});

Expand All @@ -4687,7 +4687,7 @@ public void actionPerformed(ActionEvent e) {
public void actionPerformed(ActionEvent e) {
JavaSEPort.this.darkMode = Boolean.FALSE;
pref.put("cn1.simulator.darkMode", "light");
refreshSkin(frm);
refreshThemeOnly();
}
});

Expand All @@ -4696,7 +4696,7 @@ public void actionPerformed(ActionEvent e) {
public void actionPerformed(ActionEvent e) {
JavaSEPort.this.darkMode = null;
pref.put("cn1.simulator.darkMode", "unsupported");
refreshSkin(frm);
refreshThemeOnly();
}
});
darkLightModeMenu.add(darkModeItem);
Expand Down Expand Up @@ -5793,7 +5793,9 @@ public void actionPerformed(ActionEvent e) {
largerTextScale = scale;
largerTextEnabled = scale > 1.0f + 0.001f;
pref.putFloat(PREF_LARGER_TEXT_SCALE, scale);
refreshSkin(frm);
// Theme-only refresh so the app's CSS-compiled theme survives; see
// refreshThemeOnly() for why refreshSkin breaks the app theme + canvas size.
refreshThemeOnly();
}
});
group.add(item);
Expand Down Expand Up @@ -5904,6 +5906,44 @@ static Rectangle parsePersistedBounds(String s) {
}
}

/// Theme-only refresh used by the Dark/Light Mode and Larger Text menu actions.
///
/// These toggles change how the *current* theme should be resolved, but the skin (the
/// phone-shell image, the canvas pixel size, the screen-fit zoom) hasn't moved. Routing them
/// through {@link #refreshSkin} caused two visible regressions:
///
/// 1. `refreshSkin` calls `Display.installNativeTheme()`, which delegates to
/// `UIManager.setThemeProps(nativeProps)`. `setThemeProps` **replaces** the installed
/// theme wholesale, so any CSS-compiled app theme (custom UIID fonts, custom margins,
/// app-only style keys) is wiped to the bare native theme. Visually this read as "fonts
/// are completely wrong" until the simulator was relaunched.
///
/// 2. `refreshSkin` recomputes the canvas size from `canvas.getWidth() * retinaScale /
/// skin.getWidth()`, which equals `retinaScale` on a HiDPI display whose skin already
/// fits the screen. The canvas then grows to `skin * retinaScale` while subsequent
/// drawing still uses `zoomLevel == 1`, so the simulator appeared to "activate zoom mode"
/// after a Dark/Light click.
///
/// Master PR #4935 (`isUsableWindowBounds`/`parsePersistedBounds` above) adds defense in
/// depth against the resulting bad window-bounds prefs from the Larger Text path. This
/// helper removes the trigger entirely by leaving the canvas alone --
/// `UIManager.refreshTheme()` re-applies the currently-installed themeProps (so the app
/// theme survives) and clears the style cache (so `shouldUseDarkStyle` resolves correctly
/// against the new `Display.isDarkMode()`).
private void refreshThemeOnly() {
Display.getInstance().callSerially(new Runnable() {
public void run() {
UIManager.getInstance().refreshTheme();
Form curr = Display.getInstance().getCurrent();
if (curr != null) {
deepRevaliate(curr);
curr.revalidate();
curr.repaint();
}
}
});
}


private ArrayList<Runnable> deinitializeHooks = new ArrayList<>();
public void addDeinitializeHook(Runnable r) {
Expand Down Expand Up @@ -14674,6 +14714,21 @@ private static class AutoLocalizationBundle extends Hashtable<String, String> {
private File bundleFile;
private final java.util.Properties properties = new java.util.Properties();
private boolean dirty;
/// Mtime to restore on the bundle file after each {@link #persist}.
///
/// The auto-bundle persists every time the app reads a missing key (see
/// `get`/`storeEntry`), so during normal development the file is rewritten on most
/// simulator launches even when no human ever edited it. Without this restoration,
/// `CompileCSSMojo.getLocalizationModificationTime` -- which trusts the file mtime as
/// a proxy for "user edited" -- would treat each runtime persist as a fresh
/// localization edit and force a ~30s CSS recompile on every subsequent `cn1:run`.
///
/// Strategy: before each persist, sample the file's current mtime. If it matches the
/// last value we restored (i.e. the file is exactly as we left it, no external editor
/// touched it), restore to that value after the write. If it differs, the user edited
/// outside the simulator -- capture their mtime as the new target so their edit still
/// flows into the staleness comparison.
private long preservedMtime = -1L;

AutoLocalizationBundle(File bundleFile, Map<String, String> base) {
this.bundleFile = bundleFile;
Expand Down Expand Up @@ -14744,12 +14799,37 @@ private void ensureParentExists() {

private void persist() {
ensureParentExists();
// Sample the file's mtime BEFORE writing so we can carry the user's last-edit
// timestamp forward. If the file doesn't exist yet (first persist of a fresh
// project), there's no original mtime to preserve -- fall through and let the
// post-write mtime become the new baseline.
long mtimeBeforeWrite = bundleFile.exists() ? bundleFile.lastModified() : -1L;
if (mtimeBeforeWrite > 0L
&& (preservedMtime <= 0L || mtimeBeforeWrite != preservedMtime)) {
// Either this is the first persist of the session, or the user edited the
// file externally since our last restoration. Adopt the current mtime as the
// new preserved target so their edit propagates to the staleness check.
preservedMtime = mtimeBeforeWrite;
}
try (OutputStream out = new FileOutputStream(bundleFile)) {
properties.store(out, "Codename One auto-generated bundle");
} catch (IOException err) {
Log.e(err);
}
dirty = false;
if (preservedMtime > 0L) {
// Best-effort: setLastModified can fail on read-only filesystems or unusual
// platforms. If it does, the worst case is the user gets one extra CSS
// recompile, which is the pre-fix behavior -- not a regression.
if (!bundleFile.setLastModified(preservedMtime)) {
// Capture the post-write mtime so subsequent persists in this session
// don't treat the failure as an external edit.
preservedMtime = bundleFile.lastModified();
}
} else {
// File was freshly created: use this write's mtime as the future baseline.
preservedMtime = bundleFile.lastModified();
}
}

private void persistIfNeeded(boolean force) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,24 @@ protected void executeImpl() throws MojoExecutionException, MojoFailureException
}
}

/**
* The localization directory is bundled into the same `theme.res` as the CSS rules
* (see the `-l` argument passed to `CN1CSSCLI` below). The shared
* {@link AbstractCN1Mojo#getCSSSourcesModificationTime} only walks `src/main/css`,
* so it would happily treat l10n edits as "no change" and let the up-to-date check
* at the top of {@link #executeImpl(String)} skip recompilation, leaving the user's
* `.properties` updates trapped in the stale `theme.res` until a `mvn clean` forces
* a rebuild. Roll the l10n directory's recursive modtime into the comparison so
* touching any `Messages.properties` invalidates the cached output.
*/
protected long getLocalizationModificationTime() {
File localizationDir = findLocalizationDirectory();
if (localizationDir == null || !localizationDir.exists()) {
return 0L;
}
return lastModifiedRecursive(localizationDir, ALL_FILES_FILTER);
}

/**
* Gets the source CSS directory (src/main/css). The theme.css file should be inside this directory.
* @return
Expand Down Expand Up @@ -168,7 +186,9 @@ private void executeImpl(String themePrefix) throws MojoExecutionException, Mojo
File mergeFile = new File(cssBuildDir, themePrefix + "theme.css");
mergeFile.getParentFile().mkdirs();
try {
if (themeResOutput.exists() && getCSSSourcesModificationTime() < themeResOutput.lastModified()) {
long sourcesModTime = Math.max(getCSSSourcesModificationTime(),
getLocalizationModificationTime());
if (themeResOutput.exists() && sourcesModTime < themeResOutput.lastModified()) {
getLog().info("CSS sources unchanged since last compile. Skipping CSS compilation");
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;

class CompileCSSMojoTest {
Expand Down Expand Up @@ -78,6 +80,94 @@ void addsI18nDirectoryToDesignerInvocation(@TempDir Path tempDir) throws Excepti
assertEquals(projectDir.resolve("src/main/i18n").toFile().getAbsolutePath(), args.get(index + 1));
}

/// Reproduces the staleness bug where edits to a `.properties` localization bundle do not
/// trigger CSS re-compilation. `getCSSSourcesModificationTime` only walks `src/main/css`, but
/// the CSS compiler also reads the l10n directory (it bakes the resource bundle into the same
/// `theme.res`), so a newer l10n file with an older CSS file must still recompile.
@Test
void recompilesWhenLocalizationFileNewerThanThemeRes(@TempDir Path tempDir) throws Exception {
Path projectDir = setupMultiModuleCommonProject(tempDir, "l10n");
Path themeRes = projectDir.resolve("target/classes/theme.res");
Path l10nFile = projectDir.resolve("src/main/l10n/Messages.properties");

Files.createDirectories(themeRes.getParent());
Files.write(themeRes, new byte[]{0x42});

long base = System.currentTimeMillis() - 60_000L;
ageAllInputs(projectDir, base);
assertTrue(themeRes.toFile().setLastModified(base + 5_000L));
assertTrue(l10nFile.toFile().setLastModified(base + 10_000L));

TestCompileCSSMojo mojo = createMojo(projectDir);
mojo.executeImpl();

assertNotNull(mojo.getRecordingJava(),
"CSS compiler should have been invoked because the l10n file is newer than theme.res");
}

/// Sanity-check: when only the merged-output theme.res is newer than every CSS/l10n input,
/// the mojo correctly skips the subprocess. This guards against an over-eager fix that just
/// disables the up-to-date check entirely.
@Test
void skipsCompilationWhenAllInputsOlderThanThemeRes(@TempDir Path tempDir) throws Exception {
Path projectDir = setupMultiModuleCommonProject(tempDir, "l10n");
Path themeRes = projectDir.resolve("target/classes/theme.res");

Files.createDirectories(themeRes.getParent());
Files.write(themeRes, new byte[]{0x42});

long base = System.currentTimeMillis() - 60_000L;
ageAllInputs(projectDir, base);
assertTrue(themeRes.toFile().setLastModified(base + 10_000L));

TestCompileCSSMojo mojo = createMojo(projectDir);
mojo.executeImpl();

assertNull(mojo.getRecordingJava(),
"CSS compiler should be skipped when neither CSS nor l10n changed since the last build");
}

/// Companion to the l10n test: CSS edits already invalidate the cache today, but lock the
/// behavior down so it does not regress alongside the l10n fix.
@Test
void recompilesWhenCssFileNewerThanThemeRes(@TempDir Path tempDir) throws Exception {
Path projectDir = setupMultiModuleCommonProject(tempDir, null);
Path themeRes = projectDir.resolve("target/classes/theme.res");
Path cssFile = projectDir.resolve("src/main/css/theme.css");

Files.createDirectories(themeRes.getParent());
Files.write(themeRes, new byte[]{0x42});

long base = System.currentTimeMillis() - 60_000L;
ageAllInputs(projectDir, base);
assertTrue(themeRes.toFile().setLastModified(base));
assertTrue(cssFile.toFile().setLastModified(base + 10_000L));

TestCompileCSSMojo mojo = createMojo(projectDir);
mojo.executeImpl();

assertNotNull(mojo.getRecordingJava(),
"CSS compiler should have been invoked because the CSS file is newer than theme.res");
}

/// `getCSSSourcesModificationTime` walks `src/main/css` recursively (directory mtimes count too)
/// and also stats the project pom and codenameone_settings. Files just created by the test
/// helper land with `now` as their mtime, which would always win the comparison and force
/// compilation regardless of the CSS/l10n setup we want to assert against. Walk every file
/// and directory under the project and age them to a known baseline so each test can drive
/// the modtime ordering it cares about.
private static void ageAllInputs(Path projectDir, long baseMillis) throws IOException {
Files.walk(projectDir).forEach(p -> {
// The target/ directory is set up per-test (theme.res gets an explicit mtime later),
// so leave it alone here.
if (p.startsWith(projectDir.resolve("target"))) {
return;
}
assertTrue(p.toFile().setLastModified(baseMillis),
"Failed to set mtime on " + p);
});
}

private TestCompileCSSMojo createMojo(Path projectDir) throws IOException {
MavenProject mavenProject = new MavenProject();
mavenProject.setFile(projectDir.resolve("pom.xml").toFile());
Expand Down Expand Up @@ -129,6 +219,43 @@ private Path setupProject(Path tempDir, String localizationDirName) throws IOExc
return projectDir;
}

/// Sets up the canonical multi-module Codename One layout: a parent dir containing a `common/`
/// child that owns the CSS and (optionally) the localization. `getCSSSourcesModificationTime`
/// derives `root = projectBaseDir.getParentFile()` and then looks for `root/common/src/main/css`,
/// so naming the basedir literally "common" is what makes that lookup resolve back to this
/// project's CSS directory. The single-module helper above puts CSS at `project/src/main/css`,
/// which leaves `getCSSSourcesModificationTime` returning zero and silently masks the staleness
/// bug we want to test.
private Path setupMultiModuleCommonProject(Path tempDir, String localizationDirName) throws IOException {
Path parentDir = tempDir.resolve("parent");
Files.createDirectories(parentDir);
Path commonDir = parentDir.resolve("common");
Files.createDirectories(commonDir);
Files.createDirectories(commonDir.resolve("src/main/java"));
Path cssDir = Files.createDirectories(commonDir.resolve("src/main/css"));
Files.write(cssDir.resolve("theme.css"), Arrays.asList("/* test css */"));
Files.write(commonDir.resolve("codenameone_settings.properties"), Arrays.asList("codename1.cssTheme=true"));
Files.createDirectories(commonDir.resolve("target/classes"));
Files.createFile(commonDir.resolve("designer.jar"));
Files.write(commonDir.resolve("pom.xml"), Arrays.asList(
"<project xmlns=\"http://maven.apache.org/POM/4.0.0\"",
" xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\"",
" xsi:schemaLocation=\"http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd\">",
" <modelVersion>4.0.0</modelVersion>",
" <groupId>com.codename1</groupId>",
" <artifactId>test-common</artifactId>",
" <version>1.0-SNAPSHOT</version>",
"</project>"
));

if (localizationDirName != null) {
Path localizationDir = Files.createDirectories(commonDir.resolve("src/main").resolve(localizationDirName));
Files.write(localizationDir.resolve("Messages.properties"), Arrays.asList("greeting=Hello"));
}

return commonDir;
}

private static class TestCompileCSSMojo extends CompileCSSMojo {
private final File designerJar;
private RecordingJava recordingJava;
Expand Down
Loading
Loading