Branch review fixes: CSS comment reader and site-links-published, red then green open
On branch fiesta-1-1
Two Important findings from Astra's whole-branch review, fixed in two commits. Hub CI passed on ef382e6: all steps, all 48 suite shards. releases/, css/, js/, i18n/ and icons/ are unchanged, so the gzip sizes stay at 23,823 / 23,104 / 23,217 bytes.
1. 6222049: find CSS comments the way a browser does
Finding. stripCSSComments ignored strings. A rule between content: "/*" and content: "*/" dropped out of both compatibility gates, but browsers still applied it.
Fix. One scanner, cssSpan in check/markuphelp_test.go, now reads:
- strings in either quote;
- a backslash plus the character it escapes, inside or outside a string;
- comments.
Where matching a browser would mean guessing, it refuses the stylesheet:
- a string cut off by a newline or by the end of the file;
- a comment that is never closed;
- a comment with no space on either side (
1/**/2px); - an unquoted
url(; - a function name spelt with an escape (
u\72l().
No 1.0 or tree stylesheet contains any of these. The gate's structure readers (cssItems, matchingBrace, cssDeclarations) also skip an escaped character now.
Other readers in check/:
- The selector reader,
parseCSSRules, now uses the same scanner. It was already string-aware, but\/*and\"hid a rule from it. - Seven regular-expression strippers are gone. They served the token, dialog, separator, reduced-motion, print-shape, scheme-number and :has() checks.
leafRulesno longer strips comments itself. It used to strip the size gate's already-stripped CSS a second time with a string-unaware regex.
Red, on the old reader:
- TestTheNewRuleGateHasNoKnownHoles: passes Astra's mutation and four variants: single quotes, an escaped quote inside the string,
\/*, and\"outside a string. - TestTheReaderRefusesWhatItCannotReadAsABrowserDoes: fails.
- TestTheRuleReaderSeesEveryRuleABrowserApplies: misses
[ rst-PAGE]behind\/*and\".
Mutations of the new code, each failing one of those tests, then reverted:
| | Mutation | |---|---| | M1 | quotes open no string | | M2 | no escape handling inside a string | | M3 | no escape handling outside a string | | M4 | cssItems skips no escape | | M5 | a newline does not cut a string off | | M6 | an unquoted url( is not refused | | M7 | parseCSSRules keeps its old comment reader | | M8 | matchingBrace skips no escape | | M9 | cssDeclarations skips no escape | | M10 | a comment between two tokens is not refused |
M4, M5 and M9 survived the first draft of the tests, so I tightened the cases until they failed.
Green:
- the check package;
- the three browser tests that read CSS through the changed code: TestTheConsoleDegradesTheWayItSaysItDoesWithoutHas, TestTheFloorChangesNothingButSizeOnADesktop and TestBothSpellingsComputeTheSameStyles;
- the role test, gofmt and staticcheck;
- the hub.
2. ef382e6: fail site-links-published when the export fails
Finding. The recipe kept its status in a variable set by an && chain. When git archive or tar failed, the test was skipped and the target still exited 0.
Fix.
- Each step exits with its own failure and a message naming the step.
- Cleanup runs in an EXIT trap.
- The archive is written to a file, then extracted, so a failing git archive is no longer hidden behind tar's exit status.
| Run | Before | After | |---|---|---| | SITE_REF=no-such-ref | exit 0 | exit 2: "git archive could not export no-such-ref" | | stub tar that fails | exit 0 | exit 2: "could not extract HEAD's tree" | | SITE_REF=v1.0.0 | n/a | exit 2: "has no published-only mode" | | HEAD | n/a | runs TestEveryLinkInTheSiteResolves in published mode, PASS |
- The suite's fallback, with FIESTALINKSPUBLISHED unset, also passes.
- .amadan/ci.d does not name this target.