fix(playground): warn when malformed HTML swallows the runner script - #1705
fix(playground): warn when malformed HTML swallows the runner script#1705caugner wants to merge 1 commit into
Conversation
|
00e7b3c was deployed to: https://fred-pr1705.review.mdn.allizom.net/ |
24b04be to
9623022
Compare
caugner
changed the title
9623022 to
4f7eb53
Compare
4f7eb53 to
72cb4a7
Compare
caugner
changed the title
fe7f962 to
dd9f0e1
Compare
Add `id="mdn-play-js"` and `id="mdn-play-js-end"` markers around the generated script tags, and check from the head after `DOMContentLoaded` that nothing but whitespace sits between them. If the markers are missing or content leaked in between, log a console warning. Unclosed or malformed tags in the HTML input can absorb the runner script, so the JavaScript never executes and the playground silently does nothing.
dd9f0e1 to
c64793c
Compare
caugner
changed the title
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| let leaked = false; | ||
| for ( | ||
| let node = start?.nextSibling; | ||
| node && node !== end; | ||
| node = node.nextSibling | ||
| ) { | ||
| if (node.nodeType !== Node.TEXT_NODE || node.data.trim()) { | ||
| leaked = true; | ||
| break; | ||
| } | ||
| } | ||
| if (!(start instanceof HTMLScriptElement) || !end || leaked) { |
There was a problem hiding this comment.
The leaked scan produces false warnings for working examples. The runner script at id="mdn-play-js" is a classic (non-module) script in every mode except ix-wat, so it executes synchronously during parsing — before script#mdn-play-js-end has been parsed. Any node the user's JS appends to document.body (or writes via document.write) at that point lands as the last child of <body>, i.e. between the two markers, and the parser only appends the end marker afterwards.
Failure scenario: js: 'document.body.appendChild(document.createElement("p"));' with valid HTML. The <p> sits between #mdn-play-js and #mdn-play-js-end, leaked becomes true, and the playground console shows "Could not verify that the JavaScript ran" even though it ran correctly. document.body.appendChild(...) is used by a large share of MDN live samples.
Drop the between-nodes scan — the <o-style swallowing this PR targets removes script#mdn-play-js from the DOM entirely (it is parsed as attributes of the open tag), so the marker-existence check alone detects it:
| let leaked = false; | |
| for ( | |
| let node = start?.nextSibling; | |
| node && node !== end; | |
| node = node.nextSibling | |
| ) { | |
| if (node.nodeType !== Node.TEXT_NODE || node.data.trim()) { | |
| leaked = true; | |
| break; | |
| } | |
| } | |
| if (!(start instanceof HTMLScriptElement) || !end || leaked) { | |
| if (!(start instanceof HTMLScriptElement) || !end) { |
Note that leaked and the for loop above become unused and should be removed with it.
AI-generated review by Claude
3 participants
(⚠️ Mirrored to mdn/dex#435. ⚠️ )
Description
Fix the Playground runner content assembly (
vendor/yari/libs/play/index.js) to warn when the user's JS script is swallowed by malformed HTML:id="mdn-play-js"andid="mdn-play-js-end".<head>that runs onDOMContentLoadedand warns (viaconsole.warn) if the runner script did not parse as anHTMLScriptElement, if the end marker is missing, or if anything other than whitespace ended up between the two.Adds a unit test (
test/unit/play/runner.test.js) covering the markers and the placement of the check.Motivation
Malformed HTML like
<ois parsed so the following<script>becomes attributes of the open tag, and the user's JS renders as visible text instead of running, with no hint why (#1440).Additional details
The check lives in the
<head>rather than next to the runner script, because a malformed tag can swallow the trailing script too, taking the check down with it. The warning is worded as "Could not verify that the JavaScript ran", because the check observes the symptom (the runner script not sitting where it is expected in the DOM) rather than the cause, and the surrounding text points at an unclosed or malformed tag as the usual explanation.This file is mirrored in
mdn/dex(cloud-function/src/internal/play/index.js), which serves the runner in deployed/review environments, so the fix is only observable outside local dev once mdn/dex#435 lands.Related issues and pull requests
Fixes #1440.