rastrillo / fiesta Public

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.
  • leafRules no 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.