Discard a bare semicolon inside a block's contents

[jhaygood86]

Rewritten. The first version of this PR fixed the top-level case and deliberately left blocks alone. Having now read CSS Syntax Module Level 3 properly, that was backwards — the spec says the opposite. Details under "Why the top level is excluded" below.

Problem

A bare ; inside a grouping rule's block invalidates the next rule and swallows the block's closing brace, taking the following sibling rule with it:

@media screen { .a { color: red; } ; }
@media print  { .b { color: blue; } }   /* silently disappears */

FillRules hands the semicolon to CreateRule → CreateStyle → SelectorConstructor, which has no recovery for an unexpected semicolon. It folds the token into the next rule's selector and invalidates it; the run-on rule then keeps consuming past the block's }.

What the spec says

§5.5.5 "Consume a block's contents" lists <semicolon-token> together with <whitespace-token>:

<whitespace-token>
<semicolon-token>
Discard a token from input.

So inside a block a stray semicolon is simply thrown away, exactly like whitespace.

Fix

Discard the token in FillRules, which is ExCSS's implementation of that algorithm (it backs @media, @supports and @container).

Why the top level is excluded

The asymmetry is intentional and is the part I originally got wrong.

§5.5.1 "Consume a stylesheet's contents" has cases for <whitespace-token>, <EOF-token>, <CDO-token>/<CDC-token> and <at-keyword-token>, then "anything else → Consume a qualified rule". There is no <semicolon-token> case, so a top-level ; falls into "anything else".

§5.5.3 "Consume a qualified rule" only treats a semicolon specially when it is the stop token, and a stop token is passed in exactly one place — from §5.5.5, "consume a qualified rule from input, with nested set to true, and <semicolon-token> as the stop token". At the top level none is passed, so the ; is consumed as a component value into the prelude, leaving an invalid selector and an invalid rule.

That is what ExCSS already does, so the top level is left untouched. A test now pins it so it doesn't get "fixed" by mistake later.

Acid2 agrees, and I had it backwards the first time round. Its parser-torture block is a run of deliberately-dropped rules, and the trailing ; is the mechanism for one of them:

.parser { m\argin: 2em; };
.parser { height: 3em; }        /* must NOT apply */
.parser { width: 200; }         /* must NOT apply - no unit */
.parser { border: 5em solid red ! error; }   /* must NOT apply */

One existing assertion changed

CssParseSheetWithAtAndCommentDoesNotTakeForever3 is exactly the swallowing case above. Its source contains two @media rules and it asserted Assert.Equal(1, sheet.Rules.Length) — the count the old behaviour produced. The spec-correct count is 2, so the assertion is updated and the second rule's type is now asserted too. The [Fact(Timeout = 1000)] guard the test exists for is untouched.

This is the only pre-existing assertion this PR (or any of my others) modifies. Flagging it explicitly since it is a behaviour change, not just an addition.

Tests

5 new tests in Sheet.cs: a semicolon between rules in a block, the swallowed-sibling case, repeated semicolons, @supports as well as @media, and the top-level case asserting the spec-correct invalidating behaviour.

Against master, the 4 block tests fail plus the updated one; the top-level test passes on master unchanged, confirming it documents existing behaviour. The other 1262 existing tests stay green, and all seven target frameworks build with no new warnings.