fix!: auto-apply context:// for @StyleSheet values#24235
Conversation
@Stylesheet currently produces broken <link> elements when the Vaadin servlet is mapped at a non-root path (vaadin.urlMapping=/ui/* etc.): the browser resolves the bare relative href against <base> (which points to the servlet mapping path), so a file at META-INF/resources/styles.css is fetched as /ui/styles.css and 404s. Users had to know to write @Stylesheet("context://styles.css") explicitly to step out of the servlet path. Frame the right behavior into the framework instead: - New FrontendDependencyUrlResolver.resolveToContextRoot extracts the prefix-handling rules into one place: http(s)://, //, context://, base:// pass through unchanged; "/" is treated as an absolute server path; "./" leads strip to a context-root-relative value; everything else gets a context:// prefix prepended. Path traversals are rejected. - UIInternals.addComponentDependencies normalizes @Stylesheet values through the resolver before storing them on the dependency list, so component-level @Stylesheet now renders correctly under any servlet mapping. The same normalization keys ActiveStyleSheetTracker so spelling variants of the same file (foo.css, ./foo.css, context://foo.css) deduplicate to a single <link>. - AppShellRegistry.resolveStyleSheetHref delegates to the same resolver, replacing the inline rule set. The trailing BootstrapUriResolver call continues to expand context:// using the servlet-relative path produced by getContextRootRelativePath, so AppShell-level resolution stays consistent with the UIDL path. Test fixtures updated for the canonical context://-prefixed URLs in the dependency list. UidlWriterTest also registers inline test resources at the leading-slash path (/inline.css) to match the servlet container lookup that resolveResource produces for a context:// value. Fixes #22888.
Artur-
force-pushed
the
auto-context-url-rewrite-stylesheet
branch
from
3190b0e to
7bc1d84
Compare
…ite-stylesheet # Conflicts: # flow-server/src/test/java/com/vaadin/flow/server/communication/UidlWriterTest.java
mshabarov
left a comment
mshabarov
left a comment
There was a problem hiding this comment.
Not yet tested, but I'm pretty sure that the hotswapper related functionality will be broken without changes in StyleSheetHotswapper because of the reasons listed by Claude (mainly because the hotswapper uses the old raw stylesheet urls everywhere).
|
The existing test masks the regression:
So |
UIInternals.addComponentDependencies registers stylesheet dependencies under the resolved (context://) URL, but StyleSheetHotswapper was still looking up, adding, and tracking by the raw @StyleSheet.value(). This broke hot-reload of @Stylesheet annotations: removal lookups missed the existing dependency (leaving the stale <link> in the DOM), the new dependency was added with a bare relative URL (regressing under vaadin.urlMapping), and ActiveStyleSheetTracker accumulated duplicate keys per stylesheet. Resolve URLs through FrontendDependencyUrlResolver in updateUIs and lookupUrlsForComponents so all hotswap operations key on the same canonical form as addComponentDependencies. The appShell- removal id keeps the raw value because AppShellRegistry keys it that way. Also rewrite the modify-annotation test to mirror the real hot-reload sequence (HotswapClassEvent before HotswapClassSessionEvent) so it exercises the first-time removal path, captures the original dependency id, and asserts on pendingStyleSheetRemovals — instead of relying on clearPendingSendToClient to mask the missing removal.
|
mshabarov
left a comment
mshabarov
left a comment
There was a problem hiding this comment.
Looks good to me and works as expected - checked that styles are applied on app shell and components level for no context, non-empty context and custom servlet path, hotswapping works as expected.
One small thing is that the traversal warning is reported 3 times in the log. I have a local patch for it, but can push it in a new PR.
|
This ticket/PR has been released with Vaadin 25.2.0-alpha7. |
3 participants
@Stylesheet currently produces broken elements when the Vaadin servlet is mapped at a non-root path (vaadin.urlMapping=/ui/* etc.): the browser resolves the bare relative href against (which
points to the servlet mapping path), so a file at META-INF/resources/styles.css is fetched as /ui/styles.css and 404s.
Users had to know to write @Stylesheet("context://styles.css") explicitly to step out of the servlet path.
Frame the right behavior into the framework instead:
context://foo.css) deduplicate to a single .
Test fixtures updated for the canonical context://-prefixed URLs in the dependency list. UidlWriterTest also registers inline test resources at the leading-slash path (/inline.css) to match the servlet container lookup that resolveResource produces for a context:// value.
Fixes #22888