amadan
Find CSS comments the way a browser does in every check that reads them
The compatibility gates took every /* for the start of a comment. A
rule between two new snippet rules, one with content: "/*" and one
with content: "*/", dropped out of the reading, so both gates saw only
a valid snippet addition, while a browser applied the rule between.
Inside a 700-750px media query, [rst-page] { padding-inline: 3rem }
changed every 1.0 page at widths no photograph covers.
The gates, the selector reader and the other checks now share one
scanner. It reads strings of either quote, a backslash with what it
escapes inside or outside a string, and comments, as a browser's
tokenizer does. The structure readers behind the gate also skip an
escaped character, since --a: \"; opens no string and reading one
there hides every rule up to the next quote.
Where agreeing with a browser would mean guessing, the scanner refuses
the stylesheet instead: a string cut off by a newline or the end of
the file, a comment never closed, a comment with no space either side
(1/**/2px is neither 12px nor 1 2px), an unquoted url(), which holds
neither a string nor a comment so url(/*) opens none, and a function
name spelt with an escape, since u\72l( is url(. No stylesheet, 1.0's
or the tree's, writes any of these. Reading url() as a browser does
was the alternative to refusing it; quoting the address costs nothing.
The regular-expression strippers in the token, dialog, separator,
reduced-motion, print-shape, scheme-number and :has() checks go too.
They had the same flaw, and leafRules re-stripped its input with one,
so the size gate's stripped CSS lost its strings' markers a second
time.
Red: Astra's mutation and four variants (single quotes, an escaped
quote inside the string, \/*, an escaped quote outside a string) pass
TestTheNewRuleGateHasNoKnownHoles on the old reader;
TestTheReaderRefusesWhatItCannotReadAsABrowserDoes and
TestTheRuleReaderSeesEveryRuleABrowserApplies fail. Ten mutations of
the new scanner each fail one of the three. Green: the check package,
plain and the browser tests that read CSS.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
12 files changed,
+306
−103
check/baseline_test.go+83 −9check/console_shell_browser_test.go+5 −4check/coverage_test.go+11 −8check/css_test.go+21 −23check/markup_v3_browser_test.go+5 −4check/markuphelp_test.go+112 −10check/rig_browser_test.go+1 −1check/selectors_test.go+1 −1check/sizing_browser_test.go+1 −1check/sizing_test.go+9 −8check/spelling_fixture_test.go+1 −1check/spelling_test.go+56 −33
Intent
diff --git a/check/baseline_test.go b/check/baseline_test.go| index 7cc661e..ed6a3a5 100644 |
| --- a/check/baseline_test.go |
| +++ b/check/baseline_test.go |
| @@ -227,7 +227,11 @@ func declarationsOf(file, css string) ([]declaration, error) { |
| // could record, and still replaces 1.0's spinner on every page. |
| func readStylesheet(file, css string) ([]declaration, []string, error) { |
| r := cssReader{file: file} |
| - if err := r.rules(stripCSSComments(css), nil); err != nil { |
| + bare, err := cssWithoutComments(css) |
| + if err == nil { |
| + err = r.rules(bare, nil) |
| + } |
| + if err != nil { |
| return nil, nil, fmt.Errorf("%s: %w", file, err) |
| } |
| return r.out, r.atRules, nil |
| @@ -322,12 +326,17 @@ func atRuleName(prelude string) string { |
| // cssItems splits a rule list into its statements, each up to its ; |
| // or through its {} block, with strings and parentheses respected so a |
| -// brace inside content: "{" or a url() is not taken for structure. |
| +// brace inside content: "{" or a url() is not taken for structure. A |
| +// backslash takes the next character with it here and in the two |
| +// readers below, as it does for a browser: --a: \"; opens no string, |
| +// and reading one there would hide every rule up to the next quote. |
| func cssItems(src string) ([]cssItem, error) { |
| var items []cssItem |
| start, depth := 0, 0 |
| for i := 0; i < len(src); i++ { |
| switch c := src[i]; c { |
| + case '\\': |
| + i++ |
| case '"', '\'': |
| i = skipString(src, i) |
| case '(': |
| @@ -378,6 +387,8 @@ func matchingBrace(src string, open int) (int, bool) { |
| depth := 0 |
| for i := open; i < len(src); i++ { |
| switch src[i] { |
| + case '\\': |
| + i++ |
| case '"', '\'': |
| i = skipString(src, i) |
| case '{': |
| @@ -417,6 +428,8 @@ func cssDeclarations(block string) ([][3]string, error) { |
| } |
| for i := 0; i < len(block); i++ { |
| switch block[i] { |
| + case '\\': |
| + i++ |
| case '"', '\'': |
| i = skipString(block, i) |
| case '(': |
| @@ -624,9 +637,9 @@ func vocabularyOfBaseline(t *testing.T, files []sideBySide) map[string]bool { |
| for _, f := range files { |
| css = append(css, f.wasBytes) |
| } |
| - names := vocabularyOf(css...) |
| + names := vocabularyOf(t, css...) |
| for _, f := range files { |
| - for class := range styledClasses(stripCSSComments(f.wasBytes)) { |
| + for class := range styledClasses(stripCSSComments(t, f.wasBytes)) { |
| names[class] = true |
| } |
| } |
| @@ -737,7 +750,7 @@ func refusedAdditions(t *testing.T, files []sideBySide) []string { |
| // would reset 1.0's spacing), so only a name 1.0 never wrote is new. |
| oldProps, oldAtRules := map[string]bool{}, map[string]int{} |
| for _, f := range files { |
| - for _, p := range customProperty.FindAllString(stripCSSComments(f.wasBytes), -1) { |
| + for _, p := range customProperty.FindAllString(stripCSSComments(t, f.wasBytes), -1) { |
| oldProps[p] = true |
| } |
| for _, at := range f.wasAt { |
| @@ -809,10 +822,12 @@ func refusedAdditions(t *testing.T, files []sideBySide) []string { |
| // tokens.css that changes 1.0 pages where no photograph looks: a value |
| // selector that matches 1.0's bare attribute, a registration that |
| // changes a property 1.0 already uses, an empty redefinition of a 1.0 |
| -// animation (which still() hides from every photograph), and a new |
| -// name that only an ancestor 1.0 markup already carries supplies. The |
| -// last four are additions the gate must let through, so the test |
| -// cannot pass by refusing everything. |
| +// animation (which still() hides from every photograph), a new name |
| +// that only an ancestor 1.0 markup already carries supplies, and a |
| +// rule hidden from the reading between two comment markers a browser |
| +// does not read as a comment. The cases marked false are additions the |
| +// gate must let through, so the test cannot pass by refusing |
| +// everything. |
| func TestTheNewRuleGateHasNoKnownHoles(t *testing.T) { |
| // The spelling rules that refuse escaped, spaced and capitalised |
| // names must not refuse 1.0.0's own CSS, or the grammar would be |
| @@ -835,6 +850,22 @@ func TestTheNewRuleGateHasNoKnownHoles(t *testing.T) { |
| {`@media (width: 731px) { @property --rst-\73p-4 { syntax: "<length>"; inherits: false; initial-value: 48px; } }`, `--rst-\73p-4`, true}, |
| {`@media (width: 731px) { @keyframes rst-\73pin {} }`, `rst-\73pin`, true}, |
| {`[rst-page-foot] [rst-dropdown] { margin: 0; }`, "[rst-page-foot] [rst-dropdown]", false}, |
| + // A comment marker the reader takes for one and a browser does |
| + // not, either side of a rule: inside a string of either quote, |
| + // after an escaped quote inside the string, or after a |
| + // backslash. The rule between still applies at those widths, |
| + // where no photograph looks. |
| + {`@media (min-width: 700px) and (max-width: 750px) { [rst-snippet]::before { content: "/*"; } [rst-page] { padding-inline: 3rem; } [rst-snippet]::after { content: "*/"; } }`, "[rst-page] { padding-inline: 3rem", true}, |
| + {`@media (min-width: 700px) and (max-width: 750px) { [rst-snippet]::before { content: '/*'; } [rst-page] { padding-inline: 3rem; } [rst-snippet]::after { content: '*/'; } }`, "[rst-page] { padding-inline: 3rem", true}, |
| + {`@media (min-width: 700px) and (max-width: 750px) { [rst-snippet]::before { content: "\"/*"; } [rst-page] { padding-inline: 3rem; } [rst-snippet]::after { content: "*/\""; } }`, "[rst-page] { padding-inline: 3rem", true}, |
| + {`@media (min-width: 700px) and (max-width: 750px) { [rst-snippet] { --rst-gate-a: \/*; } [rst-page] { padding-inline: 3rem; } [rst-snippet] { --rst-gate-b: */; } }`, "[rst-page] { padding-inline: 3rem", true}, |
| + // An escaped quote opens no string, so it cannot hide a rule |
| + // inside one either. |
| + {`@media (min-width: 700px) and (max-width: 750px) { [rst-snippet] { --rst-gate-a: \"; } [rst-page] { padding-inline: 3rem; } [rst-snippet] { --rst-gate-b: "\""; } }`, "[rst-page] { padding-inline: 3rem", true}, |
| + // A snippet whose content is a comment marker adds nothing to a |
| + // 1.0 page, so the reading that refuses the rules above must |
| + // still let it through. |
| + {`[rst-snippet]::before { content: "/* */"; }`, "[rst-snippet]::before", false}, |
| } { |
| files := everyStylesheet(t) |
| for i := range files { |
| @@ -936,6 +967,49 @@ func TestDeclarationsAreReadInTheirContext(t *testing.T) { |
| sameDeclarations(t, got, want) |
| } |
| +// The reader takes comments out and reads a backslash as a browser |
| +// does, and refuses what it cannot read the same way: a string a |
| +// newline or the end of the file cuts off, a comment never closed or |
| +// wedged between two tokens, and an unquoted url(), whose contents a |
| +// browser reads as neither a string nor a comment, so url(/*) opens |
| +// none. A function name spelt |
| +// with an escape is refused with it, since u\72l( is url( to a |
| +// browser. |
| +func TestTheReaderRefusesWhatItCannotReadAsABrowserDoes(t *testing.T) { |
| + for _, c := range []struct{ css, refusal string }{ |
| + {`[rst-snippet] { --rst-gate-a: url(/*); } [rst-page] { padding-inline: 3rem; } [rst-snippet] { --rst-gate-b: url(*/); }`, "url("}, |
| + {`a { b: URL(c) }`, "url("}, |
| + {`a { b: u\72l(c) }`, "escape"}, |
| + // Two strings cut off by their lines pair up into one if the |
| + // newline is not read as cutting the first one off. |
| + {"a { content: \"b\n; } c { content: \"d\n; }", "string"}, |
| + {`a { content: 'b`, "string"}, |
| + {`a { content: "b\`, "string"}, |
| + {`a { b: c } /* d`, "comment"}, |
| + {`a { padding: 1/**/2px }`, "between two tokens"}, |
| + } { |
| + if _, err := declarationsOf("t.css", c.css); err == nil || !strings.Contains(err.Error(), c.refusal) { |
| + t.Errorf("declarationsOf(%q): %v, want an error naming the %s", c.css, err, c.refusal) |
| + } |
| + } |
| + const css = "a { content: \"/*\"; b: url(\"*/\"); c: url( '/*' ) } d { content: \"e\\\nf\"; g: x-url( /**/ h) } i\\\"j { k: l; m: \\\"; n: o }" |
| + got, err := declarationsOf("t.css", css) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + want := []declaration{ |
| + {File: "t.css", Selector: "a", Property: "content", Value: `"/*"`}, |
| + {File: "t.css", Selector: "a", Property: "b", Value: `url("*/")`}, |
| + {File: "t.css", Selector: "a", Property: "c", Value: `url( '/*' )`}, |
| + {File: "t.css", Selector: "d", Property: "content", Value: "\"e\\ f\""}, |
| + {File: "t.css", Selector: "d", Property: "g", Value: "x-url( h)"}, |
| + {File: "t.css", Selector: `i\"j`, Property: "k", Value: "l"}, |
| + {File: "t.css", Selector: `i\"j`, Property: "m", Value: `\"`}, |
| + {File: "t.css", Selector: `i\"j`, Property: "n", Value: "o"}, |
| + } |
| + sameDeclarations(t, got, want) |
| +} |
| + |
| func TestAtRulesAreReadAsRecords(t *testing.T) { |
| const css = `@property --p { syntax: "<number>"; inherits: false; initial-value: 0; } @media (m) { @view-transition { navigation: auto; } } @keyframes k { from { opacity: 0 } to { opacity: 1 } }` |
| got, err := declarationsOf("t.css", css) |
diff --git a/check/console_shell_browser_test.go b/check/console_shell_browser_test.go| index 4908006..28ed480 100644 |
| --- a/check/console_shell_browser_test.go |
| +++ b/check/console_shell_browser_test.go |
| @@ -622,9 +622,10 @@ var hasRulePattern = regexp.MustCompile(`(?s)[^{}]*\{[^{}]*\}`) |
| // having about `display`, and the rail landed in the right cell only |
| // because grid auto-placement happened to agree with the declared area. |
| // It looked correct. It was luck. |
| -func stripHas(css string) (string, int) { |
| +func stripHas(t testing.TB, css string) (string, int) { |
| + t.Helper() |
| dropped := 0 |
| - // Comments come out FIRST, using css_test.go's own cssComment. A |
| + // Comments come out FIRST, with the comment reader the gates use. A |
| // browser tokenises them away before it ever looks at a selector, |
| // and the first version of this function did not: the wide |
| // layout's comment explains why the :has() spelling is repeated |
| @@ -633,7 +634,7 @@ func stripHas(css string) (string, int) { |
| // rule. The wide legs of the drive below caught it, which is the |
| // useful thing to say about them — they are not only a gate on the |
| // stylesheet, they are the control on this function. |
| - css = cssComment.ReplaceAllString(css, "") |
| + css = stripCSSComments(t, css) |
| out := hasRulePattern.ReplaceAllStringFunc(css, func(rule string) string { |
| prelude, _, ok := strings.Cut(rule, "{") |
| if !ok || !strings.Contains(prelude, ":has(") { |
| @@ -677,7 +678,7 @@ func stripHas(css string) (string, int) { |
| // fixture's and not the degradation's. |
| func TestTheConsoleDegradesTheWayItSaysItDoesWithoutHas(t *testing.T) { |
| real := string(fiesta.TokensCSS()) |
| - legacy, dropped := stripHas(real) |
| + legacy, dropped := stripHas(t, real) |
| // Control on the instrument before anything is measured with it. |
| if dropped == 0 { |
diff --git a/check/coverage_test.go b/check/coverage_test.go| index a91087c..72ac44c 100644 |
| --- a/check/coverage_test.go |
| +++ b/check/coverage_test.go |
| @@ -156,8 +156,9 @@ func tokensStyle(css, name string) bool { |
| // styledCSS is tokens.css and shell.css together, comments out, so a |
| // name mentioned in prose does not count as styled. |
| -func styledCSS() string { |
| - return stripCSSComments(string(fiesta.TokensCSS())) + "\n" + stripCSSComments(string(fiesta.ShellCSS())) |
| +func styledCSS(t testing.TB) string { |
| + t.Helper() |
| + return stripCSSComments(t, string(fiesta.TokensCSS())) + "\n" + stripCSSComments(t, string(fiesta.ShellCSS())) |
| } |
| // styledClasses is every rst- class the selectors of css name, whole: |
| @@ -181,16 +182,18 @@ var attrInSelector = regexp.MustCompile(`\[(rst-[a-z0-9-]+)(?:~="([^"]+)")?\]`) |
| // The other classes are not entries of their own: every one is the |
| // twin of an attribute selector in the same rule (spelling_test.go), |
| // so the attribute's example is its example. |
| -func styledVocabulary() map[string]bool { |
| - return vocabularyOf(string(fiesta.TokensCSS()), string(fiesta.ShellCSS())) |
| +func styledVocabulary(t testing.TB) map[string]bool { |
| + t.Helper() |
| + return vocabularyOf(t, string(fiesta.TokensCSS()), string(fiesta.ShellCSS())) |
| } |
| // vocabularyOf is styledVocabulary over any stylesheets: the baseline's |
| // five as well as the tree's two. |
| -func vocabularyOf(sheets ...string) map[string]bool { |
| +func vocabularyOf(t testing.TB, sheets ...string) map[string]bool { |
| + t.Helper() |
| out := map[string]bool{} |
| for _, css := range sheets { |
| - for _, rule := range parseCSSRules(css) { |
| + for _, rule := range cssRules(t, css) { |
| for _, sel := range rule.selectors { |
| for _, m := range attrInSelector.FindAllStringSubmatch(sel, -1) { |
| out[m[1]] = true |
| @@ -218,7 +221,7 @@ func vocabularyOf(sheets ...string) map[string]bool { |
| // plain element and look, in the gallery, like the design system's |
| // idea of that component. |
| func TestEveryExampleAttributeAndClassIsStyled(t *testing.T) { |
| - css := styledCSS() |
| + css := styledCSS(t) |
| // The vocabulary below is read off the literal [rst-…] and .rst-…; |
| // a spelling a browser matches and that read misses is refused first. |
| assertPlainSpelling(t, "tokens.css and shell.css", css) |
| @@ -332,7 +335,7 @@ func TestEverySelectorAppearsInAnExample(t *testing.T) { |
| // The two stylesheets name some 230 attributes, variants and |
| // utilities between them; far fewer is a reader that stopped |
| // reading, not a design system that shrank. |
| - styled := styledVocabulary() |
| + styled := styledVocabulary(t) |
| if len(styled) < 200 { |
| t.Fatalf("read %d styled names from the stylesheets; the reader is broken, not the files", len(styled)) |
| } |
diff --git a/check/css_test.go b/check/css_test.go| index 69555e0..3494a20 100644 |
| --- a/check/css_test.go |
| +++ b/check/css_test.go |
| @@ -59,6 +59,10 @@ func TestTokensCSSIsEmbedded(t *testing.T) { |
| // else is searched everywhere, the print and forced-colours blocks |
| // included: a radius there would override a theme's geometry. |
| func shapeLeaks(css string) ([]string, error) { |
| + css, err := cssWithoutComments(css) |
| + if err != nil { |
| + return nil, err |
| + } |
| const flatten = "--rst-shadow-knob: 0 0 0 transparent; --rst-shadow-knob-hover: 0 0 0 transparent;" |
| if n := strings.Count(css, flatten); n != 1 { |
| return nil, fmt.Errorf("tokens.css carries print's knob-shadow flatten %d times, want once, at html:root inside @media print", n) |
| @@ -357,29 +361,23 @@ func reducedMotionBlocks(css string) (blocks []string, rest string) { |
| // declaration body. |
| type leafRule struct{ selector, body string } |
| -// leafRulePattern's selector group ([^{}]+) is greedy over everything |
| -// since the previous rule's closing brace, which includes any comment |
| -// sitting between the two rules — for the very first rule in the file, |
| -// that is tokens.css's entire, ~100-line file-header comment. blockCommentPattern |
| -// strips /* ... */ spans (across lines: (?s) makes '.' match '\n') out of |
| -// a raw selector capture before it is trimmed and used as a map key or |
| -// error-message value, so neither is unusably long or noisy. |
| -var blockCommentPattern = regexp.MustCompile(`(?s)/\*.*?\*/`) |
| - |
| // leafRulePattern extracts every innermost selector{...} rule in css — |
| // this naturally includes rules nested inside an @media block (matched |
| // before the wrapping @media's own, still-open brace), since none of |
| // them nest further themselves. |
| var leafRulePattern = regexp.MustCompile(`([^{}]+)\{([^{}]*)\}`) |
| +// leafRules is every innermost rule in css, which must have had its |
| +// comments taken out first, with stripCSSComments: a comment between |
| +// two rules would otherwise be read as part of the next one's |
| +// selector, and a comment reader of its own here would be one more |
| +// that a /* inside a string could fool. |
| func leafRules(css string) []leafRule { |
| var out []leafRule |
| for _, m := range leafRulePattern.FindAllStringSubmatch(css, -1) { |
| - sel := blockCommentPattern.ReplaceAllString(m[1], "") |
| - // Fields+Join collapses any remaining internal whitespace/newlines |
| - // left by a stripped multi-line comment down to single spaces, on |
| - // top of trimming the ends. |
| - sel = strings.Join(strings.Fields(sel), " ") |
| + // Fields+Join collapses the newlines and indentation between |
| + // rules down to single spaces, on top of trimming the ends. |
| + sel := strings.Join(strings.Fields(m[1]), " ") |
| out = append(out, leafRule{selector: sel, body: m[2]}) |
| } |
| return out |
| @@ -473,6 +471,7 @@ func TestShellCSSReducedMotionDisablesEveryAnimation(t *testing.T) { |
| func assertReducedMotion(t *testing.T, name, css string) { |
| t.Helper() |
| + css = stripCSSComments(t, css) |
| reduceBlocks, rest := reducedMotionBlocks(css) |
| if len(reduceBlocks) == 0 { |
| t.Fatal(name + " declares no @media (prefers-reduced-motion: reduce) block at all") |
| @@ -544,8 +543,7 @@ func TestEveryTokenReferenceIsDeclared(t *testing.T) { |
| ref := regexp.MustCompile(`var\((--rst-[a-z0-9-]+)\s*\)`) |
| // Comments out first: a "--rst-x:" in prose would otherwise count as |
| // a declaration, and these files are mostly prose. |
| - comment := regexp.MustCompile(`(?s)/\*.*?\*/`) |
| - strip := func(b []byte) string { return comment.ReplaceAllString(string(b), "") } |
| + strip := func(b []byte) string { return stripCSSComments(t, string(b)) } |
| tokens := strip(fiesta.TokensCSS()) |
| declared := map[string]bool{} |
| for _, m := range decl.FindAllStringSubmatch(tokens, -1) { |
| @@ -644,10 +642,6 @@ func TestDropdownExclusivityIsNative(t *testing.T) { |
| } |
| } |
| -// cssComment matches a whole /* … */ block, so a check can read |
| -// tokens.css's selectors without its prose about them. |
| -var cssComment = regexp.MustCompile(`(?s)/\*.*?\*/`) |
| - |
| // detailsNamePattern reads the name attribute off a <details> open tag, |
| // whichever order the attributes are written in. |
| var detailsNamePattern = regexp.MustCompile(`<details [^>]*name="([^"]*)"`) |
| @@ -707,7 +701,7 @@ func TestModalPanelIsARenderedOpenDialog(t *testing.T) { |
| // Comments stripped first: this file explains ::backdrop and names |
| // the reset rule in prose, and a check that cannot tell a selector |
| // from a sentence about one is not a check. |
| - css := cssComment.ReplaceAllString(string(fiesta.TokensCSS()), "") |
| + css := stripCSSComments(t, string(fiesta.TokensCSS())) |
| i := strings.Index(css, "dialog[rst-modal-panel]") |
| if i < 0 { |
| t.Fatal("tokens.css has no scoped dialog reset; the UA dialog block would position the panel absolutely") |
| @@ -778,7 +772,7 @@ func TestSrOnlyUtilityComesAfterTheRulesItMustBeat(t *testing.T) { |
| // are checked, and the width is checked for being non-zero. |
| func TestEveryMenuSurfaceStylesItsSeparator(t *testing.T) { |
| styled := map[string]bool{} |
| - for _, rule := range leafRules(string(fiesta.TokensCSS())) { |
| + for _, rule := range leafRules(stripCSSComments(t, string(fiesta.TokensCSS()))) { |
| decls := declarations(rule.body) |
| if !resetsBorder(decls["border"]) || !drawsARule(decls["border-top"]) { |
| continue |
| @@ -951,7 +945,11 @@ func schemeNumberBlocks(t *testing.T, name, css string) []string { |
| func schemeNumberProblems(name, css string) (blocks []string, problems []string) { |
| where := "themes/" + name + ".css" |
| bad := func(format string, args ...any) { problems = append(problems, where+": "+fmt.Sprintf(format, args...)) } |
| - stripped := regexp.MustCompile(`(?s)/\*.*?\*/`).ReplaceAllString(css, "") |
| + stripped, err := cssWithoutComments(css) |
| + if err != nil { |
| + bad("%v", err) |
| + return nil, problems |
| + } |
| const ( |
| toggleLight = `:root[data-theme="light"] { color-scheme: light; }` |
| toggleDark = `:root[data-theme="dark"] { color-scheme: dark; }` |
diff --git a/check/markup_v3_browser_test.go b/check/markup_v3_browser_test.go| index f02f943..3cb6e8e 100644 |
| --- a/check/markup_v3_browser_test.go |
| +++ b/check/markup_v3_browser_test.go |
| @@ -106,8 +106,9 @@ func bareRSTMarkup(markup string) string { |
| // declared set is read from the file rather than typed out, so a rule |
| // that starts painting something new is compared on it without anybody |
| // remembering to add it here. |
| -func styleProperties() []string { |
| - css := stripCSSComments(string(fiesta.TokensCSS())) |
| +func styleProperties(t testing.TB) []string { |
| + t.Helper() |
| + css := stripCSSComments(t, string(fiesta.TokensCSS())) |
| seen := map[string]bool{} |
| for _, m := range regexp.MustCompile(`[;{]\s*([a-z-]+)\s*:`).FindAllStringSubmatch(css, -1) { |
| if name := m[1]; !strings.HasPrefix(name, "--") { |
| @@ -243,7 +244,7 @@ func TestBothSpellingsComputeTheSameStyles(t *testing.T) { |
| ctx, cancel := context.WithTimeout(rig.Context(), 180*time.Second) |
| defer cancel() |
| - props := styleProperties() |
| + props := styleProperties(t) |
| propsJSON, err := json.Marshal(props) |
| if err != nil { |
| t.Fatalf("marshalling the property list: %v", err) |
| @@ -389,7 +390,7 @@ func TestBothSpellingsComputeTheSameStyles(t *testing.T) { |
| func assertFixtureCoversTokensCSS(t *testing.T, fixture string) { |
| t.Helper() |
| styled := map[string]bool{} |
| - for _, m := range classInSelector.FindAllStringSubmatch(stripCSSComments(string(fiesta.TokensCSS())), -1) { |
| + for _, m := range classInSelector.FindAllStringSubmatch(stripCSSComments(t, string(fiesta.TokensCSS())), -1) { |
| styled[m[1]] = true |
| } |
| used := map[string]bool{} |
diff --git a/check/markuphelp_test.go b/check/markuphelp_test.go| index 859d54a..f1999ea 100644 |
| --- a/check/markuphelp_test.go |
| +++ b/check/markuphelp_test.go |
| @@ -1,6 +1,7 @@ |
| package check |
| import ( |
| + "fmt" |
| "regexp" |
| "strings" |
| "testing" |
| @@ -95,19 +96,120 @@ var ( |
| rstAttrInFixture = regexp.MustCompile(`(?:^|\s)(rst-[a-z0-9-]+)(?:="([^"]*)")?`) |
| ) |
| -func stripCSSComments(s string) string { |
| +// stripCSSComments is cssWithoutComments failing the test on a |
| +// stylesheet it refuses. |
| +func stripCSSComments(t testing.TB, css string) string { |
| + t.Helper() |
| + out, err := cssWithoutComments(css) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + return out |
| +} |
| + |
| +// cssWithoutComments is css with its comments taken out, found the way |
| +// a browser finds them. A reader that takes every /* for a comment can |
| +// be made to skip a rule a browser applies: between content: "/*" and |
| +// content: "*/" a whole rule drops out of the reading and still styles |
| +// every page. See cssSpan for what it refuses. |
| +func cssWithoutComments(css string) (string, error) { |
| var b strings.Builder |
| - for { |
| - k := strings.Index(s, "/*") |
| + for i := 0; i < len(css); { |
| + end, comment, err := cssSpan(css, i) |
| + if err != nil { |
| + return "", err |
| + } |
| + if !comment { |
| + b.WriteString(css[i:end]) |
| + } |
| + i = end |
| + } |
| + return b.String(), nil |
| +} |
| + |
| +// cssSpan reads the piece of css that starts at i, as a browser's |
| +// tokenizer would, for the readers that need to know what is a comment: |
| +// a comment, a whole string, a backslash with the character it |
| +// escapes, a run of name characters, or else one byte. It returns |
| +// where the piece ends and whether it is a comment. A /* inside a |
| +// string or after a backslash opens no comment, and a quote inside a |
| +// comment or after a backslash opens no string. |
| +// |
| +// What a browser reads in a way these readers would have to guess at |
| +// is refused instead. A string cut off by a newline or the end of the |
| +// file is dropped by a browser along with its declaration; a reader |
| +// would have to guess where it ends. A comment never closed swallows |
| +// the rest of the file, and one with no space either side has no |
| +// spelling without it that means the same (see below). An unquoted url() holds neither a string nor a |
| +// comment, so url(/*) opens none; reading that would mean reading |
| +// url() as a browser does, and quoting the address costs nothing. A |
| +// function name spelt with an escape is refused with it, since u\72l( |
| +// is url( to a browser. |
| +func cssSpan(css string, i int) (end int, comment bool, err error) { |
| + line := func() int { return 1 + strings.Count(css[:i], "\n") } |
| + switch c := css[i]; { |
| + case strings.HasPrefix(css[i:], "/*"): |
| + k := strings.Index(css[i+2:], "*/") |
| if k < 0 { |
| - b.WriteString(s) |
| - return b.String() |
| + return 0, false, fmt.Errorf("line %d: a comment is never closed", line()) |
| + } |
| + end := i + 2 + k + 2 |
| + // A comment keeps the tokens either side apart without being |
| + // whitespace, so dropping it joins them and putting a space in |
| + // its place adds a combinator: 1/**/2px is not 12px, and |
| + // .a/**/.b is not .a .b. |
| + if i > 0 && end < len(css) && !isCSSSpace(css[i-1]) && !isCSSSpace(css[end]) { |
| + return 0, false, fmt.Errorf("line %d: a comment sits between two tokens with no space beside it; add one", line()) |
| + } |
| + return end, true, nil |
| + case c == '"' || c == '\'': |
| + for j := i + 1; j < len(css); j++ { |
| + switch css[j] { |
| + case c: |
| + return j + 1, false, nil |
| + case '\\': |
| + // An escaped newline continues the string; \r\n is |
| + // one newline. |
| + if strings.HasPrefix(css[j+1:], "\r\n") { |
| + j++ |
| + } |
| + j++ |
| + case '\n', '\r', '\f': |
| + return 0, false, fmt.Errorf("line %d: a string is cut off by the end of its line", line()) |
| + } |
| + } |
| + return 0, false, fmt.Errorf("line %d: a string is cut off by the end of the file", line()) |
| + case c == '\\': |
| + return min(i+2, len(css)), false, nil |
| + case isCSSNameByte(c): |
| + j, escaped := i, false |
| + for j < len(css) && (isCSSNameByte(css[j]) || css[j] == '\\') { |
| + if css[j] == '\\' { |
| + j, escaped = j+1, true |
| + } |
| + j++ |
| } |
| - b.WriteString(s[:k]) |
| - j := strings.Index(s[k+2:], "*/") |
| - if j < 0 { |
| - return b.String() |
| + j = min(j, len(css)) |
| + if j == len(css) || css[j] != '(' { |
| + return j, false, nil |
| } |
| - s = s[k+2+j+2:] |
| + if escaped { |
| + return 0, false, fmt.Errorf("line %d: the function name %s( is spelt with an escape; write the name it spells", line(), css[i:j]) |
| + } |
| + if strings.EqualFold(css[i:j], "url") { |
| + if rest := strings.TrimLeft(css[j+1:], " \t\n\r\f"); rest == "" || (rest[0] != '"' && rest[0] != '\'') { |
| + return 0, false, fmt.Errorf("line %d: an unquoted url( is not read here; quote its address", line()) |
| + } |
| + } |
| + return j, false, nil |
| } |
| + return i + 1, false, nil |
| +} |
| + |
| +func isCSSSpace(c byte) bool { return strings.IndexByte(" \t\n\r\f", c) >= 0 } |
| + |
| +// isCSSNameByte is a byte that can continue a CSS name: a letter, a |
| +// digit, - or _, or any byte of a non-ASCII character. |
| +func isCSSNameByte(c byte) bool { |
| + return c == '-' || c == '_' || c >= '0' && c <= '9' || c >= 'a' && c <= 'z' || c >= 'A' && c <= 'Z' || c >= 0x80 |
| } |
diff --git a/check/rig_browser_test.go b/check/rig_browser_test.go| index 8cb327a..10cc329 100644 |
| --- a/check/rig_browser_test.go |
| +++ b/check/rig_browser_test.go |
| @@ -511,7 +511,7 @@ func classSpelling(t *testing.T, html string) string { |
| t.Helper() |
| byAttr := map[string]string{} |
| byVariant := map[string]string{} |
| - for _, m := range classInSelector.FindAllStringSubmatch(stripCSSComments(string(fiesta.TokensCSS())), -1) { |
| + for _, m := range classInSelector.FindAllStringSubmatch(stripCSSComments(t, string(fiesta.TokensCSS())), -1) { |
| class := m[1] |
| name, variant, ok := attributeFor(class) |
| if !ok { |
diff --git a/check/selectors_test.go b/check/selectors_test.go| index f6160cb..21066e4 100644 |
| --- a/check/selectors_test.go |
| +++ b/check/selectors_test.go |
| @@ -87,7 +87,7 @@ func plainSpelling(sel string) error { |
| // spelling and coverage gates as well as in the grammar. |
| func assertPlainSpelling(t *testing.T, name, css string) { |
| t.Helper() |
| - for _, rule := range parseCSSRules(css) { |
| + for _, rule := range cssRules(t, css) { |
| for _, sel := range rule.selectors { |
| if strings.HasPrefix(sel, "@") { |
| if strings.ContainsRune(sel, '\\') { |
diff --git a/check/sizing_browser_test.go b/check/sizing_browser_test.go| index b0fc454..05eae2d 100644 |
| --- a/check/sizing_browser_test.go |
| +++ b/check/sizing_browser_test.go |
| @@ -1025,7 +1025,7 @@ var mayShrink = map[string]string{ |
| func TestTheFloorChangesNothingButSizeOnADesktop(t *testing.T) { |
| without, block := floorBlock(t) |
| var sels []string |
| - for _, r := range leafRules(stripCSSComments(block)) { |
| + for _, r := range leafRules(stripCSSComments(t, block)) { |
| for _, s := range splitSelectorList(r.selector) { |
| if !strings.HasPrefix(s, ":where(") { |
| sels = append(sels, s) |
diff --git a/check/sizing_test.go b/check/sizing_test.go| index 9263bc3..7e3252c 100644 |
| --- a/check/sizing_test.go |
| +++ b/check/sizing_test.go |
| @@ -40,7 +40,7 @@ const bareZoomFloor = ":where(input:not([type=checkbox]):not([type=radio]):not([ |
| // this rewrite exists to remove). Moving any floor one rule up or down |
| // fails here. |
| func TestTheZoomFloorsSitWhereTheyCannotBeResetOrWin(t *testing.T) { |
| - css := stripCSSComments(string(fiesta.TokensCSS())) |
| + css := stripCSSComments(t, string(fiesta.TokensCSS())) |
| floors := map[string][2]int{} |
| for _, m := range zoomFloorRule.FindAllStringSubmatchIndex(css, -1) { |
| sel := collapseSpace(css[m[2]:m[3]]) |
| @@ -100,7 +100,7 @@ func TestTheZoomFloorsSitWhereTheyCannotBeResetOrWin(t *testing.T) { |
| // The type step itself (§1.2), read as text so a desktop-only edit |
| // cannot move it: the four tokens, in the touch query, at :root. |
| func TestTheTypeScaleMovesUpOneStepOnSmallOrTouchScreens(t *testing.T) { |
| - css := stripCSSComments(string(fiesta.TokensCSS())) |
| + css := stripCSSComments(t, string(fiesta.TokensCSS())) |
| re := regexp.MustCompile(regexp.QuoteMeta(touchQuery) + `\s*:root\s*\{([^{}]*)\}`) |
| m := re.FindStringSubmatch(css) |
| if m == nil { |
| @@ -135,7 +135,7 @@ func TestTheTypeScaleMovesUpOneStepOnSmallOrTouchScreens(t *testing.T) { |
| // --rst-tap directly would be 44px on a desktop too, or, moved out of |
| // the touch query, the 32px desktop floor would never reach it. |
| func TestTheTargetFloorIsOneToken(t *testing.T) { |
| - css := stripCSSComments(string(fiesta.TokensCSS())) |
| + css := stripCSSComments(t, string(fiesta.TokensCSS())) |
| root := regexp.MustCompile(`(?m)^:root \{([^{}]*)\}`).FindStringSubmatch(css) |
| if root == nil || !regexp.MustCompile(`--rst-target:\s*2rem;`).MatchString(root[1]) { |
| t.Error("the top-level :root does not declare --rst-target: 2rem, the desktop floor") |
| @@ -235,7 +235,7 @@ var floorDeclarations = map[string]struct { |
| // either here with its reason, or it goes in the touch query. |
| func TestTheFloorBlockCarriesOnlySizes(t *testing.T) { |
| _, block := floorBlock(t) |
| - rules := leafRules(stripCSSComments(block)) |
| + rules := leafRules(stripCSSComments(t, block)) |
| if len(rules) < 30 { |
| t.Fatalf("read %d rules in the floor block; the parse no longer finds them", len(rules)) |
| } |
| @@ -402,7 +402,7 @@ var iconFree = map[string]string{ |
| // rule exists to stop. |
| func TestEveryControlSizesTheIconInIt(t *testing.T) { |
| var sel string |
| - for _, r := range leafRules(stripCSSComments(string(fiesta.TokensCSS()))) { |
| + for _, r := range leafRules(stripCSSComments(t, string(fiesta.TokensCSS()))) { |
| if strings.HasPrefix(r.selector, ":where(") && strings.Contains(r.selector, ") .icon") && strings.Contains(r.body, "block-size: 1rem") { |
| sel = collapseSpace(r.selector) |
| } |
| @@ -450,9 +450,10 @@ func endsOnControlElement(sel string) bool { |
| // interactiveCSSRules is every innermost rule in css that carries cursor: |
| // pointer or styles an a, button, summary or label. |
| -func interactiveCSSRules(css string) []leafRule { |
| +func interactiveCSSRules(t testing.TB, css string) []leafRule { |
| + t.Helper() |
| var out []leafRule |
| - for _, r := range leafRules(stripCSSComments(css)) { |
| + for _, r := range leafRules(stripCSSComments(t, css)) { |
| hit := strings.Contains(r.body, "cursor: pointer") |
| for _, s := range splitSelectorList(r.selector) { |
| hit = hit || endsOnControlElement(s) |
| @@ -470,7 +471,7 @@ func interactiveCSSRules(css string) []leafRule { |
| // table above (or an exemption with a reason), and every idiom in the |
| // table has to be on the page the browser drive measures. |
| func TestEveryInteractiveRuleIsInTheTapInventory(t *testing.T) { |
| - rules := interactiveCSSRules(string(fiesta.TokensCSS())) |
| + rules := interactiveCSSRules(t, string(fiesta.TokensCSS())) |
| if len(rules) < 40 { |
| t.Fatalf("found only %d interactive rules in tokens.css; the reader is broken, not the file", len(rules)) |
| } |
diff --git a/check/spelling_fixture_test.go b/check/spelling_fixture_test.go| index b0886dd..ba69652 100644 |
| --- a/check/spelling_fixture_test.go |
| +++ b/check/spelling_fixture_test.go |
| @@ -21,7 +21,7 @@ import ( |
| func mappedAttributes(t *testing.T) map[string]bool { |
| t.Helper() |
| out := map[string]bool{} |
| - for _, m := range classInSelector.FindAllStringSubmatch(stripCSSComments(string(fiesta.TokensCSS())), -1) { |
| + for _, m := range classInSelector.FindAllStringSubmatch(stripCSSComments(t, string(fiesta.TokensCSS())), -1) { |
| if name, _, ok := markup.Attribute(m[1]); ok { |
| out[name] = true |
| } |
diff --git a/check/spelling_test.go b/check/spelling_test.go| index 2bdeb20..f23631c 100644 |
| --- a/check/spelling_test.go |
| +++ b/check/spelling_test.go |
| @@ -38,6 +38,7 @@ package check |
| // match, property by property, pseudo-elements included. |
| import ( |
| + "slices" |
| "sort" |
| "strings" |
| "testing" |
| @@ -74,42 +75,44 @@ type cssRule struct { |
| // tokens.css is a hand-written file with no preprocessor, and a |
| // full CSS parser would be a dependency this file has no need of. The |
| // preludes it returns include at-rules (@media, @supports); those carry |
| -// no rst- selector and fall out of every check below on their own. |
| -func parseCSSRules(css string) []cssRule { |
| +// no rst- selector and fall out of every check below on their own. It |
| +// reads comments, strings and escapes with cssSpan, the comment |
| +// reader the declaration gates use, so a /* a browser does not take |
| +// for a comment hides no rule from the spelling gates either, and it |
| +// refuses what cssSpan refuses. |
| +func parseCSSRules(css string) ([]cssRule, error) { |
| var rules []cssRule |
| - start, i, n := 0, 0, len(css) |
| - for i < n { |
| - if strings.HasPrefix(css[i:], "/*") { |
| - if j := strings.Index(css[i+2:], "*/"); j >= 0 { |
| - i += 2 + j + 2 |
| - } else { |
| - i = n |
| - } |
| - continue |
| + var prelude strings.Builder |
| + for i := 0; i < len(css); { |
| + end, comment, err := cssSpan(css, i) |
| + if err != nil { |
| + return nil, err |
| } |
| - switch c := css[i]; c { |
| - case '"', '\'': |
| - i++ |
| - for i < n && css[i] != c { |
| - if css[i] == '\\' { |
| - i++ |
| - } |
| - i++ |
| - } |
| - i++ |
| - case '{': |
| + switch { |
| + case comment: |
| + case css[i] == '{': |
| rules = append(rules, cssRule{ |
| - selectors: splitSelectorList(stripCSSComments(css[start:i])), |
| + selectors: splitSelectorList(prelude.String()), |
| line: 1 + strings.Count(css[:i], "\n"), |
| }) |
| - i++ |
| - start = i |
| - case '}': |
| - i++ |
| - start = i |
| + prelude.Reset() |
| + case css[i] == '}': |
| + prelude.Reset() |
| default: |
| - i++ |
| + prelude.WriteString(css[i:end]) |
| } |
| + i = end |
| + } |
| + return rules, nil |
| +} |
| + |
| +// cssRules is parseCSSRules failing the test on a stylesheet it |
| +// refuses. |
| +func cssRules(t testing.TB, css string) []cssRule { |
| + t.Helper() |
| + rules, err := parseCSSRules(css) |
| + if err != nil { |
| + t.Fatal(err) |
| } |
| return rules |
| } |
| @@ -300,7 +303,7 @@ func TestShellCSSPairsBothSpellings(t *testing.T) { |
| // leaving nothing to check. |
| func assertTwins(t *testing.T, name, css string, floor int) { |
| t.Helper() |
| - rules := parseCSSRules(css) |
| + rules := cssRules(t, css) |
| if len(rules) == 0 { |
| t.Fatal(name + " parsed to no rules at all — the reader is broken, not the file") |
| } |
| @@ -342,7 +345,7 @@ func assertNoOrphans(t *testing.T, name, css string) { |
| // A selector spelt to slip past the literal "[rst-" read below |
| // ([ rst-page], [rst-PAGE], an escape) is refused by name first. |
| assertPlainSpelling(t, name, css) |
| - for _, rule := range parseCSSRules(css) { |
| + for _, rule := range cssRules(t, css) { |
| twins := make(map[string]bool, len(rule.selectors)) |
| for _, sel := range rule.selectors { |
| if twin := attributeTwin(sel); twin != sel { |
| @@ -368,7 +371,7 @@ func assertNoOrphans(t *testing.T, name, css string) { |
| // no entry may have quietly grown an attribute form (so "this stays a |
| // class" stays true). |
| func TestClassesThatKeepTheirSpellingAreStillReal(t *testing.T) { |
| - css := stripCSSComments(string(fiesta.TokensCSS())) |
| + css := stripCSSComments(t, string(fiesta.TokensCSS())) |
| for class, why := range classKeepsItsSpelling { |
| if !strings.Contains(css, "."+class) { |
| t.Errorf("markup.Utilities names %q (%s) and tokens.css has no such class: delete the entry", class, why) |
| @@ -404,7 +407,7 @@ func TestClassesThatKeepTheirSpellingAreStillReal(t *testing.T) { |
| // Every new name has to be checked against this, not just the ones that |
| // existed on the day. |
| func TestNoTwoClassesWantTheSameAttribute(t *testing.T) { |
| - css := stripCSSComments(string(fiesta.TokensCSS())) |
| + css := stripCSSComments(t, string(fiesta.TokensCSS())) |
| seen := map[string][]string{} |
| for _, m := range classInSelector.FindAllStringSubmatch(css, -1) { |
| class := m[1] |
| @@ -432,6 +435,26 @@ func TestNoTwoClassesWantTheSameAttribute(t *testing.T) { |
| } |
| } |
| +// The selector reader finds every rule a browser applies. A comment |
| +// marker a browser does not take for one, beside a rule, must not hide |
| +// that rule from the spelling gates: [ rst-PAGE] is [rst-page] on |
| +// every 1.0 page. |
| +func TestTheRuleReaderSeesEveryRuleABrowserApplies(t *testing.T) { |
| + for _, css := range []string{ |
| + `[rst-snippet]::before { content: "/*"; } [ rst-PAGE] { padding: 0; } [rst-snippet]::after { content: "*/"; }`, |
| + `[rst-snippet] { --rst-gate-a: \/*; } [ rst-PAGE] { padding: 0; } [rst-snippet] { --rst-gate-b: */; }`, |
| + `[rst-snippet] { --rst-gate-a: \"; } [ rst-PAGE] { padding: 0; } [rst-snippet] { --rst-gate-b: "\""; }`, |
| + } { |
| + seen := false |
| + for _, rule := range cssRules(t, css) { |
| + seen = seen || slices.Contains(rule.selectors, "[ rst-PAGE]") |
| + } |
| + if !seen { |
| + t.Errorf("parseCSSRules(%q) does not read the rule [ rst-PAGE], which a browser applies", css) |
| + } |
| + } |
| +} |
| + |
| // TestSpecificityReadsSelectorsTheWayTheSpecDoes is the gate's own |
| // gate. The pairing check leans on specificity() being right about |
| // :where() (weightless), :not()/:has() (their heaviest argument) and |