Skip to content

Prevent inline qwikloader emission inside SVG/MathML foreign content in v2 SSR#8645

Merged
wmertens merged 7 commits into
build/v2from
copilot/fix-svg-foreign-content-issue
May 20, 2026
Merged

Prevent inline qwikloader emission inside SVG/MathML foreign content in v2 SSR#8645
wmertens merged 7 commits into
build/v2from
copilot/fix-svg-foreign-content-issue

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 19, 2026

Fixes SSR inline qwikloader emission guards so scripts are not injected while serializing unsupported parser contexts, with focus on SVG/MathML foreign content and other blocked parser-state elements.

Changes made

  • Replaced ad-hoc guard checks with a shared blocked-elements guard in SSR container emission logic.
  • Added/updated SSR container regression coverage for:
    • foreign content (svg, math)
    • blocked parser-state elements tracked by the no-script refcounter
  • Updated spec loops to comply with qwik-local/loop-style.
  • Removed the deprecated plaintext-specific loader behavior and its related test based on review feedback.

Validation

  • Reproduced the original lint and vitest failures.
  • Re-ran targeted lint and SSR container tests successfully.
  • Ran final validation (code review + CodeQL) with no blocking issues.

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 19, 2026

⚠️ No Changeset found

Latest commit: 9812a60

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copilot AI changed the title [WIP] Fix qwikloader script emission inside open SVG elements Prevent inline qwikloader emission inside SVG/MathML foreign content in v2 SSR May 19, 2026
Copilot AI requested a review from wmertens May 19, 2026 09:24
Comment on lines +443 to +452
if (
elementName === 'plaintext' &&
!isQwikStyle &&
this.qlInclude === QwikLoaderInclude.Inline &&
this.$noScriptHere$ === 0
) {
// <plaintext> switches the tokenizer to plain text until EOF, so any later script would be
// swallowed even after a serialized closing tag.
this.emitQwikLoaderInline();
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is deprecated would completely break Qwik anyway, so this case can be left out

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the plaintext-specific block from openElement and dropped the plaintext regression test in commit e0a85e5.

Removed handling for 'plaintext' element in Qwik SSR. It is deprecated and would break Qwik anyway
@wmertens wmertens marked this pull request as ready for review May 19, 2026 09:52
@wmertens wmertens requested a review from a team as a code owner May 19, 2026 09:52
Copilot AI review requested due to automatic review settings May 19, 2026 09:52
@wmertens wmertens enabled auto-merge May 19, 2026 09:52
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Extends the SSR v2 inline qwikloader emission guard so that it does not inject <script> while serializing inside elements whose tokenizer/parser state would corrupt the script (raw-text, escapable raw-text, legacy raw-text, template, and SVG/MathML foreign content). The previous guard only covered noscript, template, and script, leaving svg, math, style, textarea, title, iframe, noframes, xmp as cases where an inline loader could be emitted in an unsafe context.

Changes:

  • Replaces the inline element-name comparison in openElement/_closeElement with a shared NO_SCRIPT_HERE_ELEMENTS Set covering all unsafe parser-state and foreign-content roots.
  • Adds focused regression tests for foreign-content (svg, math), all guarded raw-text elements (via $noScriptHere$ refcount), and confirms plaintext-emission ordering, refactoring shared setup into createTestContainer.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
packages/qwik/src/server/ssr-container.ts Introduces NO_SCRIPT_HERE_ELEMENTS set and uses it to gate the $noScriptHere$ counter in openElement/_closeElement.
packages/qwik/src/server/ssr-container.spec.ts Adds regression tests for foreign content, guarded raw-text elements, and plaintext; extracts shared createTestContainer helper.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented May 19, 2026

Open in StackBlitz

@qwik.dev/core

npm i https://pkg.pr.new/QwikDev/qwik/@qwik.dev/core@8645

@qwik.dev/router

npm i https://pkg.pr.new/QwikDev/qwik/@qwik.dev/router@8645

eslint-plugin-qwik

npm i https://pkg.pr.new/QwikDev/qwik/eslint-plugin-qwik@8645

create-qwik

npm i https://pkg.pr.new/QwikDev/qwik/create-qwik@8645

@qwik.dev/optimizer

npm i https://pkg.pr.new/QwikDev/qwik/@qwik.dev/optimizer@8645

commit: e0a85e5

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 19, 2026

built with Refined Cloudflare Pages Action

⚡ Cloudflare Pages Deployment

Name Status Preview Last Commit
qwik-docs ✅ Ready (View Log) Visit Preview 61a0988

auto-merge was automatically disabled May 19, 2026 16:32

Head branch was pushed to by a user without write access

Copilot AI requested a review from wmertens May 19, 2026 16:34
Comment on lines +442 to +451
if (
elementName === 'plaintext' &&
!isQwikStyle &&
this.qlInclude === QwikLoaderInclude.Inline &&
this.$noScriptHere$ === 0
) {
// <plaintext> switches the tokenizer to plain text until EOF, so any later script would be
// swallowed even after a serialized closing tag.
this.emitQwikLoaderInline();
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot plaintext is deprecated and it would break Qwik anyway, so this if block can go

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in e0a85e5: the plaintext if block was removed from packages/qwik/src/server/ssr-container.ts, and the related deprecated plaintext test was removed.

Copilot AI requested a review from wmertens May 20, 2026 20:15
@wmertens wmertens merged commit a8eb1b6 into build/v2 May 20, 2026
31 checks passed
@wmertens wmertens deleted the copilot/fix-svg-foreign-content-issue branch May 20, 2026 20:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[🐞] v2: Inline qwikloader is emitted inside an open <svg> (foreign content), so the script never executes and q:container stays "paused"

3 participants