Visitar URL original
fix(html): minify an attribute by what it says, not how it was spelle… · webpack/webpack@4c80c99 · GitHub
Skip to content

Commit 4c80c99

Browse files
aryanraj45aryanraj45
authored
fix(html): minify an attribute by what it says, not how it was spelled (#22095)
* fix(html): minify an attribute by what it says, not how it was spelled The value rewrites sat behind the source having quoted the value, so an unquoted one was echoed as written: no enumerated folding (`method=GET`), no boolean collapse (`checked=checked`), no reference decoding. A rebuilt tag — a renamed `<image>`, merged shell attributes — always quoted, and the bare name came off the raw value, not the folded one, so `class=" "` took two passes. 43 of 2408 values newly reach it. * test(html): pin the new case to LF endings Its `.vtt` is content-hashed into the page, and git rewrites LF to CRLF on checkout for a text file, so Windows built a different name than the snapshot holds.
1 parent f6fd097 commit 4c80c99

14 files changed

Lines changed: 194 additions & 25 deletions

File tree

‎.changeset/026-html-attribute-minify-by-value.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"webpack": patch
3+
---
4+
5+
Minify an attribute by what its value says, not by how the source spelled it.

‎.gitattributes‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ test/configCases/html/webmanifest-icons/** eol=lf
2626
test/configCases/html/webmanifest-build-http/** eol=lf
2727
test/configCases/html/webmanifest-parsing/** eol=lf
2828
test/configCases/html/minimize-attributes/** eol=lf
29+
test/configCases/html/minimize-attributes-by-value/** eol=lf
2930
test/configCases/html/minimize-integrity/** eol=lf
3031
test/configCases/html/minimize-transforms/** eol=lf
3132
test/configCases/html/minimize-round-trip/** eol=lf

‎lib/html/syntax.js‎

Lines changed: 46 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -9150,16 +9150,36 @@ const _escapeTextContent = (s, minify) => {
91509150
* @returns {string} serialized opening tag
91519151
*/
91529152
const _synthesizeOpenTag = (path) => {
9153-
let out = `<${path.tagName()}`;
9153+
const tagName = path.tagName();
9154+
let out = `<${tagName}`;
9155+
// Looked up once for the tag, as the attribute loop does for a written one.
9156+
const enumeratedOn =
9157+
_minifying && path.namespace(path.node) === NS_HTML
9158+
? ENUMERATED_KEYWORDS[tagName]
9159+
: undefined;
91549160
for (const attribute of path.attributes()) {
91559161
const name =
91569162
attribute.serializedName !== undefined
91579163
? attribute.serializedName
91589164
: attribute.name;
9159-
out +=
9160-
attribute.value === ""
9161-
? ` ${name}`
9162-
: ` ${name}="${attribute.value.replace(/"/g, "&quot;")}"`;
9165+
if (attribute.value === "") {
9166+
out += ` ${name}`;
9167+
continue;
9168+
}
9169+
// Nothing was written here for this tag, so there is no source spelling to
9170+
// beat: the shortest one wins outright.
9171+
if (!_minifying || !_transforms.normalizeAttributeQuotes) {
9172+
out += ` ${name}="${attribute.value.replace(/"/g, "&quot;")}"`;
9173+
continue;
9174+
}
9175+
const value = _rewriteAttributeValue(
9176+
tagName,
9177+
name,
9178+
attribute.value,
9179+
false,
9180+
enumeratedOn
9181+
);
9182+
out += ` ${name}=${_minimalAttributeValue(decodeEntities(value, true))}`;
91639183
}
91649184
return `${out}>`;
91659185
};
@@ -9893,9 +9913,10 @@ const _minifyStyleBody = (css) =>
98939913
* @param {string} value its rewritten value
98949914
* @param {string} rawValue its value as the source spelled it
98959915
* @param {string | null} decoded the value decoded, for one carrying character references, or null
9916+
* @param {boolean} quoted whether the source spelled the value with a delimiter, which is two characters this has to beat
98969917
* @returns {{ text: string, unquotedTail: boolean } | null} what to emit, or null to leave the source alone
98979918
*/
9898-
const _writeAttribute = (attributeName, value, rawValue, decoded) => {
9919+
const _writeAttribute = (attributeName, value, rawValue, decoded, quoted) => {
98999920
const minimal =
99009921
_transforms.normalizeAttributeQuotes && decoded !== null
99019922
? _minimalAttributeValue(decoded)
@@ -9905,7 +9926,8 @@ const _writeAttribute = (attributeName, value, rawValue, decoded) => {
99059926

99069927
if (
99079928
minimal !== null &&
9908-
minimal.length < (unquotable ? value.length : rawValue.length + 2)
9929+
minimal.length <
9930+
(unquotable ? value.length : rawValue.length + (quoted ? 2 : 0))
99099931
) {
99109932
const opened = minimal.charCodeAt(0);
99119933

@@ -11178,7 +11200,9 @@ const _minifyOpenTag = (path, open) => {
1117811200
continue;
1117911201
}
1118011202
}
11181-
if (quoted) {
11203+
// Where nothing below can change the value: foreign content rewrites only
11204+
// `style`, and a bare value is already the shortest spelling of itself.
11205+
if (quoted || html || attributeName === "style" || rawValue.includes("&")) {
1118211206
const hasReference = rawValue.includes("&");
1118311207
// A character reference would decode to something these grammars read
1118411208
// differently, so only a reference-free value is rewritten.
@@ -11209,7 +11233,7 @@ const _minifyOpenTag = (path, open) => {
1120911233
// the switch that names the rest of them.
1121011234
if (
1121111235
_transforms.normalizeAttributeQuotes &&
11212-
(rawValue === "" ||
11236+
(value === "" ||
1121311237
(html &&
1121411238
_transforms.collapseBooleanAttributes &&
1121511239
(_transforms.collapseBooleanAttributes === "all" ||
@@ -11244,11 +11268,17 @@ const _minifyOpenTag = (path, open) => {
1124411268
*/
1124511269
const writeStyle = (minified) =>
1124611270
declarations !== null
11247-
? _writeAttribute(attributeName, rawValue, rawValue, minified)
11248-
: _writeAttribute(attributeName, minified, rawValue, spellable);
11271+
? _writeAttribute(attributeName, rawValue, rawValue, minified, quoted)
11272+
: _writeAttribute(
11273+
attributeName,
11274+
minified,
11275+
rawValue,
11276+
spellable,
11277+
quoted
11278+
);
1124911279
const written = isStyle
1125011280
? writeStyle(declarations !== null ? declarations : value)
11251-
: _writeAttribute(attributeName, value, rawValue, spellable);
11281+
: _writeAttribute(attributeName, value, rawValue, spellable, quoted);
1125211282
// The JavaScript an event handler holds, offered decoded — `&amp;&amp;`
1125311283
// is an operator to JavaScript rather than an entity. No built-in.
1125411284
const handler =
@@ -11262,7 +11292,7 @@ const _minifyOpenTag = (path, open) => {
1126211292
: null;
1126311293

1126411294
if (handler !== null && _asciiTrim(handler) !== "") {
11265-
const delimiter = open[valueStart - tagStart - 1];
11295+
const delimiter = quoted ? open[valueStart - tagStart - 1] : "";
1126611296
// A foreign `/>` reads back whether this value ends unquoted, which a
1126711297
// deferred answer is not there to decide: the source's quoting stands.
1126811298
const holdQuoted = tailIsRead && _deferEmbeddedSource !== undefined;
@@ -11295,7 +11325,8 @@ const _minifyOpenTag = (path, open) => {
1129511325
attributeName,
1129611326
escapeAttribute(rendered, CC_QUOTATION_MARK, true),
1129711327
rawValue,
11298-
rendered
11328+
rendered,
11329+
quoted
1129911330
);
1130011331

1130111332
return answered !== null && answered.text.length < asWritten.length
@@ -11333,7 +11364,7 @@ const _minifyOpenTag = (path, open) => {
1133311364
if (isStyle && _deferEmbeddedSource !== undefined) {
1133411365
// Declining leaves the attribute as written, delimiter and all — an
1133511366
// answer nothing beats is the renderer keeping what it was given.
11336-
const delimiter = open[valueStart - tagStart - 1];
11367+
const delimiter = quoted ? open[valueStart - tagStart - 1] : "";
1133711368
const asWritten = ` ${attributeName}=${delimiter}${rawValue}${delimiter}`;
1133811369
// What stands until an answer arrives, and the tag ending it commits
1133911370
// to: the built-in's text where it beat the source, else the source.

‎test/HtmlSyntax.unittest.js‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5440,8 +5440,9 @@ describe("SourceProcessor — attribute quote spelling", () => {
54405440
expect(minify('<a href="?a=1&amp;b=2">t</a>')).toBe(
54415441
'<a href="?a=1&amp;b=2">t</a>'
54425442
);
5443-
// A bare value ends at `>`, so there the reference has to stay.
5444-
expect(minify("<p title=a&gt;b>t</p>")).toBe("<p title=a&gt;b>t");
5443+
// A bare value ends at `>`, so the value is quoted rather than the
5444+
// reference kept — one character shorter, and the same value.
5445+
expect(minify("<p title=a&gt;b>t</p>")).toBe('<p title="a>b">t');
54455446
});
54465447
});
54475448

@@ -8115,10 +8116,10 @@ describe("SourceProcessor — minify serialization edge cases", () => {
81158116
});
81168117

81178118
it("keeps cloned attributes on the reconstructed element", () => {
8118-
// The reconstructed clone is synthesized, not sliced from source, and
8119-
// synthesized values always keep their quotes.
8119+
// The reconstructed clone is synthesized, not sliced from source, so
8120+
// there is no spelling to beat and the shortest one is written.
81208121
expect(minify('<b x="1"><p>x</b>y</p>')).toBe(
8121-
'<b x=1></b><p><b x="1">x</b>y'
8122+
"<b x=1></b><p><b x=1>x</b>y"
81228123
);
81238124
});
81248125

@@ -8134,18 +8135,18 @@ describe("SourceProcessor — minify serialization edge cases", () => {
81348135

81358136
it("escapes a quote-holding cloned attribute value safely", () => {
81368137
expect(minify("<b a='x\"y'><p>t</b>")).toBe(
8137-
'<b a=\'x"y\'></b><p><b a="x&quot;y">t</b>'
8138+
"<b a='x\"y'></b><p><b a='x\"y'>t</b>"
81388139
);
81398140
});
81408141

81418142
it("rebuilds renamed void tokens instead of dropping them", () => {
8142-
expect(minify('<image src="a&amp;b">')).toBe('<img src="a&amp;b">');
8143+
expect(minify('<image src="a&amp;b">')).toBe("<img src=a&amp;b>");
81438144
expect(minify("a</br>b")).toBe("a<br>b");
81448145
});
81458146

81468147
it("materializes an implied <body> once attributes merge onto it", () => {
81478148
expect(minify('<div>a</div><body class="x">')).toBe(
8148-
'<body class="x"><div>a</div></body>'
8149+
"<body class=x><div>a</div></body>"
81498150
);
81508151
});
81518152

‎test/configCases/html/minimize-attributes-by-value/__snapshots__/ConfigCacheTest.snap‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing
2+
3+
exports[`ConfigCacheTestCases html minimize-attributes-by-value minimize-attributes-by-value should compile 1`] = `
4+
"<!doctype html><head><title>attributes by value</title></head><body>
5+
<form method=get action=a><button type=submit formmethod=post>s</button></form>
6+
<input type=text inputmode=numeric checked disabled>
7+
<img src=51b834b7c1ef0b59ea50.png crossorigin=anonymous decoding=async>
8+
<div contenteditable=true draggable=true spellcheck=true>x</div>
9+
<input value=&>
10+
<img c=f>
11+
<div class>y</div>
12+
<track kind=subtitles src=5a6ee533be65928cb355.vtt>
13+
14+
15+
</body></html>"
16+
`;

‎test/configCases/html/minimize-attributes-by-value/__snapshots__/ConfigTest.snap‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing
2+
3+
exports[`ConfigTestCases html minimize-attributes-by-value minimize-attributes-by-value should compile 1`] = `
4+
"<!doctype html><head><title>attributes by value</title></head><body>
5+
<form method=get action=a><button type=submit formmethod=post>s</button></form>
6+
<input type=text inputmode=numeric checked disabled>
7+
<img src=51b834b7c1ef0b59ea50.png crossorigin=anonymous decoding=async>
8+
<div contenteditable=true draggable=true spellcheck=true>x</div>
9+
<input value=&>
10+
<img c=f>
11+
<div class>y</div>
12+
<track kind=subtitles src=5a6ee533be65928cb355.vtt>
13+
14+
15+
</body></html>"
16+
`;

‎test/configCases/html/minimize-attributes-by-value/a.png‎

Lines changed: 1 addition & 0 deletions
Loading

‎test/configCases/html/minimize-attributes-by-value/index.js‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
import "./page.html";
2+
3+
// The same attribute has to minify the same whether the source quoted its
4+
// value, and whether the printer echoed the tag or rebuilt it.
5+
it("should minify an attribute by what it says", () => {
6+
expect(true).toBe(true);
7+
});

‎test/configCases/html/minimize-attributes-by-value/page.html‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
<!DOCTYPE html>
2+
<html>
3+
<head><title>attributes by value</title></head>
4+
<body>
5+
<form method=GET action=a><button type=SUBMIT formmethod=POST>s</button></form>
6+
<input type=TEXT inputmode=NUMERIC checked=checked disabled=disabled>
7+
<img src=a.png crossorigin=ANONYMOUS decoding=ASYNC>
8+
<div contenteditable=TRUE draggable=TRUE spellcheck=TRUE>x</div>
9+
<input value=&amp;>
10+
<image c=f>
11+
<div class=" ">y</div>
12+
<track kind=SUBTITLES src=t.vtt>
13+
</body>
14+
</html>

‎test/configCases/html/minimize-attributes-by-value/t.vtt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
WEBVTT

0 commit comments

Comments
 (0)