Skip to content

HTMLRewriter: close a void element that tagName renames to a non-void name - #41968

Draft
robobun wants to merge 1 commit into
mainfrom
robobun/8283e706/htmlrewriter-void-rename
Draft

robobun wants to merge 1 commit into
mainfrom
robobun/8283e706/htmlrewriter-void-rename

Conversation

@robobun

@robobun robobun commented Sep 8, 2026 •

Copy link
Copy Markdown
Collaborator

Draft until upstream reviews cloudflare/lol-html#337 (fork port: oven-sh/lol-html#4). After #43266 (lol-html v3.0.1) merges, the pin moves to that port.

Problem

  • el.tagName = "picture" on a void element (img, br) rewrites only the start tag. Nothing closes the new name: <img><p>next</p> becomes <picture><p>next</p>, and the <p> re-parses as a child of <picture>.
  • The cause is Element::set_tag_name in lol-html (vendor/lolhtml/src/rewritable_units/element.rs:181). It renames the end tag only when can_have_content is true, and a void element has no end tag.

Fix

  • Pin oven-sh/lol-html at 5816ca4, the port for the v2.7.2 lineage that main pins. For an HTML void element renamed to a non-void name, lol-html now writes </name> right after the start tag, before any after() content.
  • Unchanged: void-to-void renames, /> elements in foreign content, remove(), replace(). canHaveContent stays false.
  • Verified: test/js/workerd/html-rewriter.test.js (new test, fails on bun 1.4.3). Also bundler_html.

Background

  • HTMLRewriter binds lol-html, built from the oven-sh/lol-html fork at the commit in scripts/build/deps/lolhtml.ts.
  • lol-html has no tree builder. It decides per start tag whether the element can have content (canHaveContent). Void elements cannot.
  • after() content of a void element lives on its start tag. The end tag is part of that tag's serialization, so it stays ahead of every after() call. A userland el.after("</picture>", { html: true }) cannot.

Downsides

  • Output differs from Cloudflare Workers for void-to-non-void renames until upstream merges the fix.
  • Code that already closes the renamed element by hand now emits two end tags. A browser ignores the second, except </p> (an empty paragraph). A GitHub code search found none.
Notes

Repro on bun 1.4.3:

const r = (sel, name, html) => new HTMLRewriter().on(sel, { element(e) { e.tagName = name; } }).transform(html);
r("img", "picture", '<img src=a.png><p>next</p><p>last</p>'); // <picture src=a.png><p>next</p><p>last</p>
r("br", "textarea", '<br><p>next</p></body></html>');         // <textarea><p>next</p></body></html>
r("input", "a", '<input value=v><b>sib</b>');                   // <a value=v><b>sib</b>

Fuzz numbers: 8 of 12 void-to-non-void renames diverged (the 4 that agreed had no following siblings), 157/157 other renames and 51/51 invalid-name throws agreed with the DOM.

Fork lineages. Main pins v2.7.2 plus handler suspension. The fork's bun branch moved to v3.0.1 on 2026-09-17 for #43266. 5816ca4 (fork branch robobun/void-rename-end-tag-v2.7.2) is the port for the lineage main pins. It also includes ea6125b (descendant matching linear in depth), because that lineage is linear; #43266 brings the same change through v3.0.1. oven-sh/lol-html#4 (55edbfd) is the port on the v3.0.1 bun branch.

Why void -> void emits no end tag: </br> parses as a second <br>. Why foreign /> elements are unchanged: /> closes them whatever the name. remove()/replace() drop the start tag, so no end tag is written.

Other bun suites run: htmlrewriter-additional-bugs, html-rewriter-end-error, test/js/web/html/, bundler_html.

lol-html checks on the pinned commit (v2.7.2 lineage): cargo test --features integration_test 174 + 3 + 32 + 2 pass (nine upstream expectations that used an <img> rename as a match marker now include the end tag), cargo clippy --all-targets no new warnings. The v3.0.1 port: cargo test --features _integration_test 210 + 3 + 38 + 2 pass, including a new test that renames a void element before and during a handler suspension. The same patch on upstream main: 184 lib tests pass.

Doubled end tag check (parse5): <picture></picture></picture>tail, <span></span></span>tail and <amp-img></amp-img></amp-img>tail all parse to one empty element plus the text. <p></p></p>tail parses to two <p> elements.


no test proof · iteration 2 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/workerd/html-rewriter.test.js

@robobun

robobun commented Sep 8, 2026 •

Copy link
Copy Markdown
Collaborator Author

Status: reproduced on bun 1.4.3 (<img src=a.png><p>next</p> renamed to picture comes out unclosed, both <p> become its children). The fix is in lol-html and is submitted upstream first as cloudflare/lol-html#337. test/js/workerd/html-rewriter.test.js fails on stock bun at the first assertion and passes with the pin. 185/185 in that file on a debug build.

Draft. Upstream has not reviewed cloudflare/lol-html#337 (opened 2026-09-08, pinged once on 2026-09-15).

Fork state: the fork's bun branch moved to lol-html v3.0.1 on 2026-09-17 for #43266. This PR pins 5816ca4, the port for the v2.7.2 lineage that main pins today (fork branch robobun/void-rename-end-tag-v2.7.2). oven-sh/lol-html#4 holds the port for the v3.0.1 lineage (55edbfd, 210 + 3 + 38 + 2 lol-html tests pass). That port also applies with no conflict on 0e6a4b1, the fork commit that #43266 pins (216 + 3 + 38 + 2 tests pass there).

After #43266 merges, this PR needs a rebase, and its pin must move to the v3.0.1 port. To request that, comment on this PR.

To ship before upstream answers, a maintainer merges oven-sh/lol-html#4 after #43266 lands, and this PR pins the result. That makes Bun's output differ from Cloudflare Workers for void-to-non-void renames until upstream merges.

@robobun

robobun commented Sep 8, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 5:45 AM PT - Sep 15th, 2026

✅ @robobun, your commit fd65a8b040c5daa8a2e6c1f763f3447c64058b5b passed in Build #115943! 🎉


🧪   To try this PR locally:

bunx bun-pr 41968

That installs a local version of the PR into your bun-41968 executable, so you can run:

bun-41968 --bun

… name

Pin oven-sh/lol-html at 5816ca4 (oven-sh/lol-html#4, the bun-branch
port of cloudflare/lol-html#337). `el.tagName = "picture"` on `<img>`
rewrote only the start tag, so the following siblings re-parsed as
children of the unclosed `<picture>`. lol-html now writes `</picture>`
right after the start tag unless the new name is void too.

The pin also moves past ea6125b (descendant-selector matching linear
in depth), which #41479 pins on its own.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants