Repository navigation
fix(html): keep an end tag whose omission lets the adoption agency restructure the tree - #21649
Conversation
…structure the tree §13.1.2.4 lets a p or li end tag go when nothing follows it in its parent, but inside a formatting element the element left open is exactly the furthest block the adoption agency looks for on the parent's end tag: <b><p>x</p></b> printed as <b><p>x</b> re-parses to a cloned <b> under the <p>. Keep the tag when the parent is a formatting element and the element is special; div and ul are unaffected.
|
| Name | Type |
|---|---|
| webpack | Patch |
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
WalkthroughThe HTML minimizer now preserves end tags for special elements nested directly inside formatting elements. This prevents adoption-agency tree restructuring from changing serialized output. A regression fixture covers paragraph, list-item, preserved block, and inline cases. The test configuration builds and minimizes the HTML, then snapshots the emitted page. The changeset documents the correction. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f6e232c5-bbb8-4f58-a899-138b1d259379
⛔ Files ignored due to path filters (2)
test/configCases/html/minimize-end-tag-in-formatting/__snapshots__/ConfigCacheTest.snapis excluded by!**/*.snaptest/configCases/html/minimize-end-tag-in-formatting/__snapshots__/ConfigTest.snapis excluded by!**/*.snap
📒 Files selected for processing (6)
.changeset/157-html-minify-text-fixes.mdlib/html/syntax.jstest/configCases/html/minimize-end-tag-in-formatting/index.jstest/configCases/html/minimize-end-tag-in-formatting/page.htmltest/configCases/html/minimize-end-tag-in-formatting/test.config.jstest/configCases/html/minimize-end-tag-in-formatting/webpack.config.js
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
webpack/schema-utils(auto-detected)webpack/tapable(auto-detected)
|
Please fix reviews |
|
@alexander-akait done shortened the comment to two lines please take a view thanks :)) |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #21649 +/- ##
==========================================
+ Coverage 94.23% 94.39% +0.16%
==========================================
Files 628 628
Lines 79952 80279 +327
Branches 23676 23737 +61
==========================================
+ Hits 75344 75783 +439
+ Misses 4608 4496 -112
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Merging this PR will improve performance by 33.43%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ⚡ | Memory | benchmark "asset-modules-inline", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' |
397.6 KB | 274.8 KB | +44.67% |
| ⚡ | Memory | benchmark "future-defaults", scenario '{"name":"mode-production","mode":"production"}' |
10.5 MB | 8.5 MB | +23.07% |
Tip
Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.
Comparing aryanraj45:fix/html-end-tag-in-formatting-parent (49a2be2) with main (6412396)
|
This PR is packaged and the instant preview is available (57f2f2e). Install it locally:
npm i -D webpack@https://pkg.pr.new/webpack@57f2f2e
yarn add -D webpack@https://pkg.pr.new/webpack@57f2f2e
pnpm add -D webpack@https://pkg.pr.new/webpack@57f2f2e |
Summary
§13.1.2.4 lets a paragraph or list-item end tag be omitted when nothing follows it in its parent, and the minifier took that permission at face value. Inside a formatting element it does not hold: the element left open is exactly the furthest block the adoption agency looks for when the parent's end tag arrives, so the page re-parses to a different tree —
<b><p>x</p></b>printed as<b><p>x</b>re-parses to<b></b><p><b>x</b>. Anemwrapping anlibreaks the same way. Found by the real-browser DOM equivalence check while adding parser coverage cases.The end tag is now kept when the parent is an HTML formatting element and the element itself is special. A
divorulparent is unaffected, and no existing snapshot in the suite changed.What kind of change does this PR introduce?
fix
Did you add tests for your changes?
Yes — test/configCases/html/minimize-end-tag-in-formatting, covering both broken shapes plus two controls that must keep omitting. The fixture is also picked up by the browser equivalence unit test, which failed on it before the fix (11 vs 13 elements) and passes after.
Does this PR introduce a breaking change?
No. Output only grows where it was previously wrong.
If relevant, what needs to be documented once your changes are merged or what have you already documented?
n/a
Use of AI
AI was used. I found the divergence through the browser equivalence check, then used AI to reduce it to a minimal reproduction, locate the rule in the printer, write the regression case and run the suites. I reviewed the change and verified the round-trip and the unaffected cases myself
Summary by CodeRabbit
Bug Fixes
Tests