jamesfredley commented on PR #15713: URL: https://github.com/apache/grails-core/pull/15713#issuecomment-4615532118
Gaps to review and potentially address before SM3 becomes default # | Gap / Open item | SM2 / Current state | SM3 / Needed | | --- | --- | --- | | `g:applyLayout` attributes (`template`,`url`,`action`,`controller`,`params`,`model`,`contentType`,`encoding`,`parse`) | Full support - `RenderGrailsLayoutTagLib.groovy:100-173` | `name` + body only; other attrs silently ignored - `RenderSitemeshTagLib.groovy:77-106` | | Default-layout config + implicit `application` fallback | Reads `grails.views.layout.default` (`LayoutGrailsPlugin:37`); falls back to layout `application` (`GroovyPageLayoutFinder:180`) | Reads only `grails.sitemesh.default.layout` (`Sitemesh3GrailsPlugin:83`); no `application` fallback | | Partial/template render suppression (`GrailsRenderViewMutator` + `GrailsLayoutSelector`) | Registers both (`LayoutGrailsPlugin:74-75`); consumed by core `ResponseRenderer:340-341,563` | Registers neither → beans null → behavior lost | | `grails.views.layout.enable.nongsp` (non-GSP/JSP layouts) | `LayoutGrailsPlugin:38` + `GroovyPageLayoutFinder` `enableNonGspViews`/`viewMustExist` | `Sitemesh3LayoutFinder:192-194` GSP locator only; flag unsupported | | Captured-page isolation across nested/sibling `applyLayout` | N/A (different mechanism) | `GrailsSiteMeshViewContext.dispatch:81` sets fresh page, no restore; taglib saves only `LAYOUT_ATTRIBUTE` (`RenderSitemeshTagLib:78,100-104`) | | No-body captured page + `layoutBody` | Handled | `CaptureAwareContentProcessor:82` returns used page without data; `layoutBody` reads only `body` (`RenderSitemeshTagLib:218-225`) | | `<title>` strip robustness | Regex-based, well-formed | `extractHead()` matches `"<title"` as prefix - mis-slices `<titlebar>`/`<title-x>` (`Sitemesh3CapturedPage.java:232`) | | Aggregate web starter still defaults to SM2 | `starter-web/build.gradle:53` → `:grails-layout` unless `SITEMESH3_TESTING_ENABLED=true` | Default branch must be `:grails-sitemesh3` | | ~35 test examples default to SM2 | `else → grails-layout` in every example (e.g. `app1/build.gradle:42-47`) | Default = SM3; SM2 via forced lane | | Main unit suite defaults to SM2 | `grails-test-suite-uber/build.gradle:53-58` → `:grails-layout` | Default = SM3 | | SM2-only examples with **no** SM3 coverage | Hardcoded `grails-layout`: `test-phases:42`, `scaffolding:49`, `scaffolding-fields:38`, `jetty:41`, `database-cleanup:37`, `gsp-layout:50` | Keep as SM2 anchors; add SM3 variants where parity matters | | Default path ≠ tested path | `SITEMESH3_TESTING_ENABLED` gates SM3; default build never runs SM3 | Retire/invert flag so default == tested | | No CI lane exercising SM3 as default | Only opt-in env runs SM3 | Add SM3-default lane + SM2 lane | | Fragile upstream auto-config suppression | Disabled `NoopSitemeshFilter` named `sitemesh` suppresses upstream `SiteMeshAutoConfiguration` (`Sitemesh3GrailsPlugin:106-124`) | Robust suppression + guard test | | Servlet 6.1 / Tomcat 11 forward→include dispatch | SM3 bypasses `RequestDispatcher.forward()` for absolute layout paths (`GrailsSiteMeshViewContext.java:88-93`) | Functional coverage on Tomcat 11 | | `grails-gsp-spring-boot` unpublished | In `settings.gradle:174`, depends on SM3 (`spring-boot/build.gradle:37`), but absent from `publish-root-config.gradle` (only `grails-sitemesh3` at `:75`) | Publish or don't rely on it | ## Not gaps (verified parity) Resolution order (6 steps), `NONE_LAYOUT`, `layoutTitle`/`layoutHead`, `pageProperty`/`ifPageProperty`, meta extraction, `body.*` attrs, `grailsLayout:parameter`, comma/chained decoration (happy path), unbounded layout cache (SM2 identical). ## Minor / cosmetic Cache-interval key differs: SM3 `grails.sitemesh.layout.cache.interval` vs SM2 `grails.gsp.reload.interval`. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
