From d9e0b975f5de464be2d932b276c45dc22c5b2e8d Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Mon, 15 Jun 2026 10:40:35 +0200 Subject: [PATCH 1/5] WW-5256 docs: design to decouple FreeMarker whitespace stripping from devMode Fixes s:textarea rendering blank lines and HTML whitespace bloat in devMode by honoring struts.freemarker.whitespaceStripping unconditionally. Co-Authored-By: Claude Opus 4.8 --- ...er-whitespace-devmode-decoupling-design.md | 125 ++++++++++++++++++ 1 file changed, 125 insertions(+) create mode 100644 docs/superpowers/specs/2026-06-15-WW-5256-freemarker-whitespace-devmode-decoupling-design.md diff --git a/docs/superpowers/specs/2026-06-15-WW-5256-freemarker-whitespace-devmode-decoupling-design.md b/docs/superpowers/specs/2026-06-15-WW-5256-freemarker-whitespace-devmode-decoupling-design.md new file mode 100644 index 0000000000..bd74accd55 --- /dev/null +++ b/docs/superpowers/specs/2026-06-15-WW-5256-freemarker-whitespace-devmode-decoupling-design.md @@ -0,0 +1,125 @@ +--- +date: 2026-06-15 +ticket: WW-5256 +url: https://issues.apache.org/jira/browse/WW-5256 +status: design +--- + +# WW-5256 — Decouple FreeMarker whitespace stripping from devMode + +## Problem + +In `release/7.2.0-RC1`, `s:textarea` renders blank lines inside an empty textarea, and +the overall HTML output contains noticeably more whitespace than in 6.x. Reported symptom +on CRUD entry/add pages: an empty `bean.data` (null default) textarea shows up as two blank +lines on screen, and the raw HTML source has far more whitespace throughout than the 6.x +equivalent. + +## Root cause + +The FreeMarker templates are **not** the cause. `textarea.ftl` and all of its included +sub-templates (`css.ftl`, `scripting-events.ftl`, `common-attributes.ftl`, +`dynamic-attributes.ftl`) are byte-for-byte identical between `support/struts-6-x-x` and +`release/7.2.0-RC1` apart from the `parameters` → `attributes` rename. Their whitespace +structure is unchanged. + +The regression is in `FreemarkerManager.java:355`, introduced by WW-5256 +(commit `9305a5812`, `@since 7.2.0`): + +```java +// 6.x — whitespace stripping unconditionally on: +configuration.setWhitespaceStripping(true); + +// 7.2.0-RC1 — stripping turned off whenever devMode is on: +boolean enableWhitespaceStripping = whitespaceStripping && !devMode; +configuration.setWhitespaceStripping(enableWhitespaceStripping); +``` + +When `struts.devMode=true` (the normal development setting), whitespace stripping is +forced off. FreeMarker then stops collapsing directive-only lines (e.g. `<#if …>` / +``), which produces two distinct effects: + +1. **General whitespace bloat** — every directive-only line in every UI template now emits + its newline and indentation. +2. **Visible `s:textarea` breakage** — the directive lines around `nameValue` sit *inside* + ``, where whitespace is **significant content**. The collapsed + newlines that 6.x removed now render as blank lines in the browser. + +The `&& !devMode` term also **overrides** the `struts.freemarker.whitespaceStripping` +config flag: a developer running in devMode has no way to turn stripping back on. + +### Why the coupling was introduced (and why it was wrong) + +Per the WW-5256 research note +(`thoughts/shared/research/2025-09-24-WW-5256-freemarker-whitespace-compression.md`), the +devMode auto-disable was copied by analogy from the new `` tag, which +intentionally disables compression in devMode with a `force` override. The stated goal was +"readable output while debugging." The note itself raised this as an *open question* +("Should whitespace stripping be automatically disabled in DevMode … or require explicit +configuration?") rather than a settled decision. + +The analogy does not hold: for most tags the extra dev whitespace is harmless cosmetic +noise, but for `` with no internal blank lines when + `struts.devMode=true`. From aa9e5a1f14f3386d8ac3d3dc61be1078e2880d79 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Mon, 15 Jun 2026 10:45:21 +0200 Subject: [PATCH 2/5] WW-5256 docs: implementation plan to decouple whitespace stripping from devMode Co-Authored-By: Claude Opus 4.8 --- ...reemarker-whitespace-devmode-decoupling.md | 253 ++++++++++++++++++ 1 file changed, 253 insertions(+) create mode 100644 docs/superpowers/plans/2026-06-15-WW-5256-freemarker-whitespace-devmode-decoupling.md diff --git a/docs/superpowers/plans/2026-06-15-WW-5256-freemarker-whitespace-devmode-decoupling.md b/docs/superpowers/plans/2026-06-15-WW-5256-freemarker-whitespace-devmode-decoupling.md new file mode 100644 index 0000000000..42c11a85e4 --- /dev/null +++ b/docs/superpowers/plans/2026-06-15-WW-5256-freemarker-whitespace-devmode-decoupling.md @@ -0,0 +1,253 @@ +# FreeMarker Whitespace Stripping / devMode Decoupling Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Make FreeMarker whitespace stripping governed solely by `struts.freemarker.whitespaceStripping` (default `true`), removing the `devMode` auto-disable that broke `s:textarea` rendering and bloated HTML output in development. + +**Architecture:** Remove the `&& !devMode` term in `FreemarkerManager.buildConfiguration()` and delete the now-unused `devMode` field/setter/injection. Update the Javadoc and the unit tests accordingly. `TextareaTest` serves as the rendering-level regression guard. + +**Tech Stack:** Java, FreeMarker 2.3.34, JUnit 3-style tests (`testXxx` methods extending `StrutsInternalTestCase`), Maven. + +**Spec:** `docs/superpowers/specs/2026-06-15-WW-5256-freemarker-whitespace-devmode-decoupling-design.md` + +**Ticket:** [WW-5256](https://issues.apache.org/jira/browse/WW-5256) + +--- + +### Task 1: Prove the bug with a failing test (RED) + +This task documents the current broken behavior: with `devMode=true` and whitespace stripping +configured `true`, the manager wrongly reports stripping as disabled. This test is temporary — +it uses the `setDevMode` API that Task 2 removes, so Task 2 deletes it. + +**Files:** +- Test: `core/src/test/java/org/apache/struts2/views/freemarker/FreemarkerManagerTest.java` + +- [ ] **Step 1: Add the temporary failing test** + +Insert this method after `testWhitespaceStrippingEnabledWhenNotInDevMode` (currently ends at line 169, before the closing `}` of the class): + +```java + // TEMP (WW-5256): documents the pre-fix bug; removed in the same change that removes the devMode coupling. + public void testWhitespaceStrippingNotDisabledInDevMode() throws Exception { + // given + FreemarkerManager manager = new FreemarkerManager(); + container.inject(manager); + manager.setWhitespaceStripping("true"); + manager.setDevMode("true"); + + // when + manager.init(servletContext); + + // then + assertTrue(manager.config.getWhitespaceStripping()); + } +``` + +- [ ] **Step 2: Run the test to verify it fails** + +Run: `mvn test -DskipAssembly -pl core -Dtest=FreemarkerManagerTest#testWhitespaceStrippingNotDisabledInDevMode` +Expected: FAIL — assertion error, `getWhitespaceStripping()` returns `false` because the current code computes `whitespaceStripping && !devMode`. + +- [ ] **Step 3: Commit the failing test** + +```bash +git add core/src/test/java/org/apache/struts2/views/freemarker/FreemarkerManagerTest.java +git commit -m "WW-5256 test: prove whitespace stripping wrongly disabled in devMode" +``` + +--- + +### Task 2: Decouple stripping from devMode (GREEN) + +Apply the production fix and align the test suite. Because the `setDevMode` setter is removed, +the temporary test from Task 1 and the two original devMode-coupling tests are deleted, and the +config test drops its `setDevMode` call. + +**Files:** +- Modify: `core/src/main/java/org/apache/struts2/views/freemarker/FreemarkerManager.java` (field at line 180; setter at 216-219; coupling at 355-357) +- Modify: `core/src/test/java/org/apache/struts2/views/freemarker/FreemarkerManagerTest.java` (lines 129-169) + +- [ ] **Step 1: Replace the coupling in `FreemarkerManager`** + +Find (lines 355-357): + +```java + boolean enableWhitespaceStripping = whitespaceStripping && !devMode; + LOG.debug("Whitespace stripping: {} (configured: {}, devMode: {})", enableWhitespaceStripping, whitespaceStripping, devMode); + configuration.setWhitespaceStripping(enableWhitespaceStripping); +``` + +Replace with: + +```java + LOG.debug("Whitespace stripping: {}", whitespaceStripping); + configuration.setWhitespaceStripping(whitespaceStripping); +``` + +- [ ] **Step 2: Remove the unused `devMode` field** + +Find (line 180): + +```java + protected boolean devMode; +``` + +Delete this line. + +- [ ] **Step 3: Remove the unused `setDevMode` setter and its injection** + +Find (lines 216-219): + +```java + + @Inject(value = StrutsConstants.STRUTS_DEVMODE, required = false) + public void setDevMode(String devMode) { + this.devMode = BooleanUtils.toBoolean(devMode); + } +``` + +Delete this block (the setter plus the blank line preceding it). + +- [ ] **Step 4: Delete the temporary test from Task 1 and the two devMode-coupling tests** + +In `FreemarkerManagerTest.java`, delete these three methods entirely: +- `testWhitespaceStrippingNotDisabledInDevMode` (added in Task 1) +- `testWhitespaceStrippingDisabledInDevMode` (lines 143-155) +- `testWhitespaceStrippingEnabledWhenNotInDevMode` (lines 157-169) + +- [ ] **Step 5: Drop the `setDevMode` call from the config test** + +Find (lines 129-141): + +```java + public void testWhitespaceStrippingDisabledViaConfiguration() throws Exception { + // given + FreemarkerManager manager = new FreemarkerManager(); + container.inject(manager); + manager.setWhitespaceStripping("false"); + manager.setDevMode("false"); + + // when + manager.init(servletContext); + + // then + assertFalse(manager.config.getWhitespaceStripping()); + } +``` + +Replace with (remove the `manager.setDevMode("false");` line): + +```java + public void testWhitespaceStrippingDisabledViaConfiguration() throws Exception { + // given + FreemarkerManager manager = new FreemarkerManager(); + container.inject(manager); + manager.setWhitespaceStripping("false"); + + // when + manager.init(servletContext); + + // then + assertFalse(manager.config.getWhitespaceStripping()); + } +``` + +- [ ] **Step 6: Verify no remaining references to the removed API** + +Run: `grep -rn "setDevMode\|\.devMode" core/src/main/java/org/apache/struts2/views/freemarker/ core/src/test/java/org/apache/struts2/views/freemarker/` +Expected: no matches. + +- [ ] **Step 7: Run the FreemarkerManager tests** + +Run: `mvn test -DskipAssembly -pl core -Dtest=FreemarkerManagerTest` +Expected: PASS — `testWhitespaceStrippingEnabledByDefault` and `testWhitespaceStrippingDisabledViaConfiguration` both green; class compiles with no reference to `devMode`/`setDevMode`. + +- [ ] **Step 8: Commit** + +```bash +git add core/src/main/java/org/apache/struts2/views/freemarker/FreemarkerManager.java core/src/test/java/org/apache/struts2/views/freemarker/FreemarkerManagerTest.java +git commit -m "WW-5256 fix(freemarker): honor whitespaceStripping regardless of devMode" +``` + +--- + +### Task 3: Update the configuration Javadoc + +**Files:** +- Modify: `core/src/main/java/org/apache/struts2/StrutsConstants.java` (lines 335-342) + +- [ ] **Step 1: Remove the stale devMode sentence** + +Find (lines 335-342): + +```java + /** + * Controls FreeMarker whitespace stripping during template compilation. + * When enabled (default), removes indentation and trailing whitespace from lines containing only FTL tags. + * Automatically disabled when devMode is enabled. + * + * @since 7.2.0 + */ + public static final String STRUTS_FREEMARKER_WHITESPACE_STRIPPING = "struts.freemarker.whitespaceStripping"; +``` + +Replace with: + +```java + /** + * Controls FreeMarker whitespace stripping during template compilation. + * When enabled (default), removes indentation and trailing whitespace from lines containing only FTL tags. + * + * @since 7.2.0 + */ + public static final String STRUTS_FREEMARKER_WHITESPACE_STRIPPING = "struts.freemarker.whitespaceStripping"; +``` + +- [ ] **Step 2: Commit** + +```bash +git add core/src/main/java/org/apache/struts2/StrutsConstants.java +git commit -m "WW-5256 docs: drop devMode note from whitespaceStripping constant" +``` + +--- + +### Task 4: Confirm rendering regression guard and full module build + +`TextareaTest` renders the `s:textarea` tag against expected output fixtures. With stripping now +unconditionally on by default, it must stay green — this is the user-visible guarantee that the +textarea no longer emits blank lines. + +**Files:** +- Verify only: `core/src/test/java/org/apache/struts2/views/jsp/ui/TextareaTest.java` + +- [ ] **Step 1: Run the textarea rendering tests** + +Run: `mvn test -DskipAssembly -pl core -Dtest=TextareaTest` +Expected: PASS — rendered output matches `Textarea-1.txt` / `Textarea-2.txt` fixtures (no internal blank lines). + +- [ ] **Step 2: Run the full core test module** + +Run: `mvn test -DskipAssembly -pl core` +Expected: BUILD SUCCESS — no compilation errors from the removed `devMode` API, all tests pass. + +- [ ] **Step 3: No commit needed** + +This task is verification only; no source changes. + +--- + +## Self-Review + +**Spec coverage:** +- Decouple stripping from devMode → Task 2 Step 1. ✓ +- Remove `devMode` field/setter/`@Inject` → Task 2 Steps 2-3. ✓ +- `default.properties` unchanged → no task needed (intentional). ✓ +- Javadoc cleanup → Task 3. ✓ +- Test updates (remove 2 devMode tests, drop stray `setDevMode` call, keep default/explicit tests) → Task 2 Steps 4-5. ✓ +- Verification (`FreemarkerManagerTest`, textarea rendering, no internal blank lines) → Task 2 Step 7, Task 4. ✓ + +**Placeholder scan:** No TBD/TODO; every code step shows exact before/after content. The one `TEMP` marker is an intentional, scoped throwaway test removed within the same plan (Task 2 Step 4). ✓ + +**Type/signature consistency:** `whitespaceStripping` (existing `protected boolean`, default `true`) and `config.getWhitespaceStripping()` used consistently across tasks; `setDevMode`/`devMode` only referenced in the temporary Task 1 test and the deletions in Task 2. ✓ From 465f7a588dff02fa900508bccd03febfc87425d3 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Mon, 15 Jun 2026 10:46:48 +0200 Subject: [PATCH 3/5] WW-5256 test: prove whitespace stripping wrongly disabled in devMode --- .../views/freemarker/FreemarkerManagerTest.java | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/core/src/test/java/org/apache/struts2/views/freemarker/FreemarkerManagerTest.java b/core/src/test/java/org/apache/struts2/views/freemarker/FreemarkerManagerTest.java index d30e4e43b8..7c072cd068 100644 --- a/core/src/test/java/org/apache/struts2/views/freemarker/FreemarkerManagerTest.java +++ b/core/src/test/java/org/apache/struts2/views/freemarker/FreemarkerManagerTest.java @@ -167,6 +167,21 @@ public void testWhitespaceStrippingEnabledWhenNotInDevMode() throws Exception { // then assertTrue(manager.config.getWhitespaceStripping()); } + + // TEMP (WW-5256): documents the pre-fix bug; removed in the same change that removes the devMode coupling. + public void testWhitespaceStrippingNotDisabledInDevMode() throws Exception { + // given + FreemarkerManager manager = new FreemarkerManager(); + container.inject(manager); + manager.setWhitespaceStripping("true"); + manager.setDevMode("true"); + + // when + manager.init(servletContext); + + // then + assertTrue(manager.config.getWhitespaceStripping()); + } } class DummyFreemarkerManager extends FreemarkerManager { From 0380544b1545c91a3a1bde58d4e565c47f68cf32 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Mon, 15 Jun 2026 10:48:33 +0200 Subject: [PATCH 4/5] WW-5256 fix(freemarker): honor whitespaceStripping regardless of devMode --- .../views/freemarker/FreemarkerManager.java | 11 +---- .../freemarker/FreemarkerManagerTest.java | 44 ------------------- 2 files changed, 2 insertions(+), 53 deletions(-) diff --git a/core/src/main/java/org/apache/struts2/views/freemarker/FreemarkerManager.java b/core/src/main/java/org/apache/struts2/views/freemarker/FreemarkerManager.java index dcbebc767e..e6432d3373 100644 --- a/core/src/main/java/org/apache/struts2/views/freemarker/FreemarkerManager.java +++ b/core/src/main/java/org/apache/struts2/views/freemarker/FreemarkerManager.java @@ -177,7 +177,6 @@ public class FreemarkerManager { protected int mruMaxStrongSize; protected String templateUpdateDelay; protected boolean whitespaceStripping = true; - protected boolean devMode; protected Map tagLibraries; private FileManager fileManager; @@ -213,11 +212,6 @@ public void setWhitespaceStripping(String whitespaceStripping) { this.whitespaceStripping = BooleanUtils.toBoolean(whitespaceStripping); } - @Inject(value = StrutsConstants.STRUTS_DEVMODE, required = false) - public void setDevMode(String devMode) { - this.devMode = BooleanUtils.toBoolean(devMode); - } - @Inject public void setContainer(Container container) { Map map = new HashMap<>(); @@ -352,9 +346,8 @@ protected Configuration createConfiguration(ServletContext servletContext) throw } LOG.debug("Disabled localized lookups"); configuration.setLocalizedLookup(false); - boolean enableWhitespaceStripping = whitespaceStripping && !devMode; - LOG.debug("Whitespace stripping: {} (configured: {}, devMode: {})", enableWhitespaceStripping, whitespaceStripping, devMode); - configuration.setWhitespaceStripping(enableWhitespaceStripping); + LOG.debug("Whitespace stripping: {}", whitespaceStripping); + configuration.setWhitespaceStripping(whitespaceStripping); LOG.debug("Sets NewBuiltinClassResolver to TemplateClassResolver.SAFER_RESOLVER"); configuration.setNewBuiltinClassResolver(TemplateClassResolver.SAFER_RESOLVER); LOG.debug("Sets HTML as an output format and escaping policy"); diff --git a/core/src/test/java/org/apache/struts2/views/freemarker/FreemarkerManagerTest.java b/core/src/test/java/org/apache/struts2/views/freemarker/FreemarkerManagerTest.java index 7c072cd068..8abfafc191 100644 --- a/core/src/test/java/org/apache/struts2/views/freemarker/FreemarkerManagerTest.java +++ b/core/src/test/java/org/apache/struts2/views/freemarker/FreemarkerManagerTest.java @@ -131,7 +131,6 @@ public void testWhitespaceStrippingDisabledViaConfiguration() throws Exception { FreemarkerManager manager = new FreemarkerManager(); container.inject(manager); manager.setWhitespaceStripping("false"); - manager.setDevMode("false"); // when manager.init(servletContext); @@ -139,49 +138,6 @@ public void testWhitespaceStrippingDisabledViaConfiguration() throws Exception { // then assertFalse(manager.config.getWhitespaceStripping()); } - - public void testWhitespaceStrippingDisabledInDevMode() throws Exception { - // given - FreemarkerManager manager = new FreemarkerManager(); - container.inject(manager); - manager.setWhitespaceStripping("true"); - manager.setDevMode("true"); - - // when - manager.init(servletContext); - - // then - assertFalse(manager.config.getWhitespaceStripping()); - } - - public void testWhitespaceStrippingEnabledWhenNotInDevMode() throws Exception { - // given - FreemarkerManager manager = new FreemarkerManager(); - container.inject(manager); - manager.setWhitespaceStripping("true"); - manager.setDevMode("false"); - - // when - manager.init(servletContext); - - // then - assertTrue(manager.config.getWhitespaceStripping()); - } - - // TEMP (WW-5256): documents the pre-fix bug; removed in the same change that removes the devMode coupling. - public void testWhitespaceStrippingNotDisabledInDevMode() throws Exception { - // given - FreemarkerManager manager = new FreemarkerManager(); - container.inject(manager); - manager.setWhitespaceStripping("true"); - manager.setDevMode("true"); - - // when - manager.init(servletContext); - - // then - assertTrue(manager.config.getWhitespaceStripping()); - } } class DummyFreemarkerManager extends FreemarkerManager { From 133cbbd5c6b38610661f653be98d949090cbb217 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Mon, 15 Jun 2026 10:51:47 +0200 Subject: [PATCH 5/5] WW-5256 docs: drop devMode note from whitespaceStripping constant --- core/src/main/java/org/apache/struts2/StrutsConstants.java | 1 - 1 file changed, 1 deletion(-) diff --git a/core/src/main/java/org/apache/struts2/StrutsConstants.java b/core/src/main/java/org/apache/struts2/StrutsConstants.java index 8b9ff58c18..0e9ca4fc31 100644 --- a/core/src/main/java/org/apache/struts2/StrutsConstants.java +++ b/core/src/main/java/org/apache/struts2/StrutsConstants.java @@ -335,7 +335,6 @@ public final class StrutsConstants { /** * Controls FreeMarker whitespace stripping during template compilation. * When enabled (default), removes indentation and trailing whitespace from lines containing only FTL tags. - * Automatically disabled when devMode is enabled. * * @since 7.2.0 */