fix #3325: list-style-type and reserved idents

evanw · Sep 12, 2023 · 56e25c1 · 56e25c1
1 parent cc74cd0
commit 56e25c1
Show file tree

Hide file tree

Showing 4 changed files with 131 additions and 26 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,28 @@
 # Changelog
 
+## Unreleased
+
+* Fix `list-style-type` with the `local-css` loader ([#3325](https://github.com/evanw/esbuild/issues/3325))
+
+    The `local-css` loader incorrectly treated all identifiers provided to `list-style-type` as a custom local identifier. That included identifiers such as `none` which have special meaning in CSS, and which should not be treated as custom local identifiers. This release fixes this bug:
+
+    ```css
+    /* Original code */
+    ul { list-style-type: none }
+
+    /* Old output (with --loader=local-css) */
+    ul {
+      list-style-type: stdin_none;
+    }
+
+    /* New output (with --loader=local-css) */
+    ul {
+      list-style-type: none;
+    }
+    ```
+
+    Note that this bug only affected code using the `local-css` loader. It did not affect code using the `css` loader.
+
 ## 0.19.2
 
 * Update how CSS nesting is parsed again

diff --git a/internal/bundler_tests/bundler_css_test.go b/internal/bundler_tests/bundler_css_test.go
@@ -519,13 +519,16 @@ func TestImportCSSFromJSLocalAtCounterStyle(t *testing.T) {
 				div :local { list-style-type: local }
 
 				/* Must not accept invalid type values */
+				div :local { list-style-type: none }
 				div :local { list-style-type: INITIAL }
 				div :local { list-style-type: decimal }
 				div :local { list-style-type: disc }
 				div :local { list-style-type: SQUARE }
 				div :local { list-style-type: circle }
 				div :local { list-style-type: disclosure-OPEN }
 				div :local { list-style-type: DISCLOSURE-closed }
+				div :local { list-style-type: LAO }
+				div :local { list-style-type: "\1F44D" }
 			`,
 
 			"/list_style.css": `
@@ -564,6 +567,7 @@ func TestImportCSSFromJSLocalAtCounterStyle(t *testing.T) {
 				div :local { list-style: circle }
 				div :local { list-style: disclosure-OPEN }
 				div :local { list-style: DISCLOSURE-closed }
+				div :local { list-style: LAO }
 			`,
 		},
 		entryPaths: []string{"/entry.js"},

diff --git a/internal/bundler_tests/snapshots/snapshots_css.txt b/internal/bundler_tests/snapshots/snapshots_css.txt
@@ -2482,14 +2482,7 @@ TestImportCSSFromJSLocalAtCounterStyle
 ---------- /out/entry.js ----------
 // list_style_type.css
 var list_style_type_default = {
-  local: "list_style_type_local",
-  INITIAL: "list_style_type_INITIAL",
-  decimal: "list_style_type_decimal",
-  disc: "list_style_type_disc",
-  SQUARE: "list_style_type_SQUARE",
-  circle: "list_style_type_circle",
-  "disclosure-OPEN": "list_style_type_disclosure-OPEN",
-  "DISCLOSURE-closed": "list_style_type_DISCLOSURE-closed"
+  local: "list_style_type_local"
 };
 
 // list_style.css
@@ -2514,25 +2507,34 @@ div {
   list-style-type: list_style_type_local;
 }
 div {
-  list-style-type: list_style_type_INITIAL;
+  list-style-type: none;
 }
 div {
-  list-style-type: list_style_type_decimal;
+  list-style-type: INITIAL;
 }
 div {
-  list-style-type: list_style_type_disc;
+  list-style-type: decimal;
 }
 div {
-  list-style-type: list_style_type_SQUARE;
+  list-style-type: disc;
 }
 div {
-  list-style-type: list_style_type_circle;
+  list-style-type: SQUARE;
 }
 div {
-  list-style-type: list_style_type_disclosure-OPEN;
+  list-style-type: circle;
 }
 div {
-  list-style-type: list_style_type_DISCLOSURE-closed;
+  list-style-type: disclosure-OPEN;
+}
+div {
+  list-style-type: DISCLOSURE-closed;
+}
+div {
+  list-style-type: LAO;
+}
+div {
+  list-style-type: "👍";
 }
 
 /* list_style.css */
@@ -2614,6 +2616,9 @@ div {
 div {
   list-style: DISCLOSURE-closed;
 }
+div {
+  list-style: LAO;
+}
 
 ================================================================================
 TestImportCSSFromJSLocalAtKeyframes

diff --git a/internal/css_parser/css_decls_list_style.go b/internal/css_parser/css_decls_list_style.go
@@ -65,12 +65,7 @@ func (p *parser) processListStyleShorthand(tokens []css_ast.Token) {
 			}
 
 			if typeIndex == -1 {
-				switch lower {
-				case "decimal", "disc", "square", "circle", "disclosure-open", "disclosure-closed":
-					// "list-style-type" is definitely not a <custom-ident>
-					return
-				}
-				if cssWideAndReservedKeywords[lower] {
+				if cssWideAndReservedKeywords[lower] || predefinedCounterStyles[lower] {
 					// "list-style-type" is definitely not a <custom-ident>
 					return
 				}
@@ -94,13 +89,91 @@ func (p *parser) processListStyleShorthand(tokens []css_ast.Token) {
 			return
 		}
 
-		p.processListStyleType(&tokens[typeIndex])
+		if t := &tokens[typeIndex]; t.Kind == css_lexer.TIdent {
+			t.Kind = css_lexer.TSymbol
+			t.PayloadIndex = p.symbolForName(t.Loc, t.Text).Ref.InnerIndex
+		}
 	}
 }
 
-func (p *parser) processListStyleType(token *css_ast.Token) {
-	if token.Kind == css_lexer.TIdent {
-		token.Kind = css_lexer.TSymbol
-		token.PayloadIndex = p.symbolForName(token.Loc, token.Text).Ref.InnerIndex
+func (p *parser) processListStyleType(t *css_ast.Token) {
+	if t.Kind == css_lexer.TIdent {
+		if lower := strings.ToLower(t.Text); lower != "none" && !cssWideAndReservedKeywords[lower] && !predefinedCounterStyles[lower] {
+			t.Kind = css_lexer.TSymbol
+			t.PayloadIndex = p.symbolForName(t.Loc, t.Text).Ref.InnerIndex
+		}
 	}
 }
+
+// https://drafts.csswg.org/css-counter-styles-3/#predefined-counters
+var predefinedCounterStyles = map[string]bool{
+	// 6.1. Numeric:
+	"arabic-indic":         true,
+	"armenian":             true,
+	"bengali":              true,
+	"cambodian":            true,
+	"cjk-decimal":          true,
+	"decimal-leading-zero": true,
+	"decimal":              true,
+	"devanagari":           true,
+	"georgian":             true,
+	"gujarati":             true,
+	"gurmukhi":             true,
+	"hebrew":               true,
+	"kannada":              true,
+	"khmer":                true,
+	"lao":                  true,
+	"lower-armenian":       true,
+	"lower-roman":          true,
+	"malayalam":            true,
+	"mongolian":            true,
+	"myanmar":              true,
+	"oriya":                true,
+	"persian":              true,
+	"tamil":                true,
+	"telugu":               true,
+	"thai":                 true,
+	"tibetan":              true,
+	"upper-armenian":       true,
+	"upper-roman":          true,
+
+	// 6.2. Alphabetic:
+	"hiragana-iroha": true,
+	"hiragana":       true,
+	"katakana-iroha": true,
+	"katakana":       true,
+	"lower-alpha":    true,
+	"lower-greek":    true,
+	"lower-latin":    true,
+	"upper-alpha":    true,
+	"upper-latin":    true,
+
+	// 6.3. Symbolic:
+	"circle":            true,
+	"disc":              true,
+	"disclosure-closed": true,
+	"disclosure-open":   true,
+	"square":            true,
+
+	// 6.4. Fixed:
+	"cjk-earthly-branch": true,
+	"cjk-heavenly-stem":  true,
+
+	// 7.1.1. Japanese:
+	"japanese-formal":   true,
+	"japanese-informal": true,
+
+	// 7.1.2. Korean:
+	"korean-hangul-formal":  true,
+	"korean-hanja-formal":   true,
+	"korean-hanja-informal": true,
+
+	// 7.1.3. Chinese:
+	"simp-chinese-formal":   true,
+	"simp-chinese-informal": true,
+	"trad-chinese-formal":   true,
+	"trad-chinese-informal": true,
+
+	// 7.2. Ethiopic Numeric Counter Style:
+	"ethiopic-numeric": true,
+}