Skip to content

Commit 0288e37

Browse files
committed
fix(levelcode-ai): the group stop button was never actually hidden
Spotted in a real run: a red stop button sat beside a finished group, offering to stop a command that had already exited. The reducer was right — finalizeGroup does set `stopBtn.hidden = true`. The CSS was wrong: `.tl-group .groupstop { display: inline-flex }` outranks the UA stylesheet's `[hidden] { display: none }`, so the attribute was inert and the button had been painted since the group opened, running or not. chat.html already carried four hand-written escapes for this same trap (#workbar, .modewrap, .modemenu, .st-approvals); this was the missing fifth. No DOM test could have caught it — the fake DOM has no stylesheet, so it faithfully reported a hidden attribute the browser then ignored. So the guard is static: webviewCss.test.js collects every element that ships the `hidden` attribute and fails if a class rule forces `display` without a matching `[hidden]` escape. Verified against the pre-fix file, where it flags .groupstop and nothing else. Full gate green (21 suites).
1 parent 6d260ff commit 0288e37

2 files changed

Lines changed: 78 additions & 0 deletions

File tree

extensions/levelcode-ai/media/chat.html

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -297,6 +297,10 @@
297297
.tl-group .groupcounts .a { color: var(--vscode-gitDecoration-addedResourceForeground, #4ec97a); }
298298
.tl-group .groupcounts .d { color: var(--vscode-gitDecoration-deletedResourceForeground, #e06c75); }
299299
.tl-group .groupstop { flex: 0 0 auto; display: inline-flex; align-items: center; border: none; background: none; color: var(--muted); cursor: pointer; padding: 2px; }
300+
/* an explicit display beats the UA's [hidden] rule, so .hidden = true is inert without this —
301+
same escape as #workbar[hidden] / .modemenu[hidden] above. A stop button that outlives its
302+
command is worse than no button: it offers to stop something that already finished. */
303+
.tl-group .groupstop[hidden] { display: none; }
300304
.tl-group .groupstop:hover { color: var(--vscode-errorForeground, #f14c4c); }
301305
.tl-group .groupstop .ci { width: 12px; height: 12px; }
302306
.tl-group .groupbody { padding: 2px 0 0 4px; }
Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
/*---------------------------------------------------------------------------------------------
2+
* Static CSS invariants for the chat webview — run: node test/webviewCss.test.js
3+
*
4+
* These are the bugs no DOM test can see. groupReducer.test.js proves the reducer sets
5+
* `el.hidden = true`; it cannot prove the browser then paints nothing, because the fake DOM has
6+
* no stylesheet. That gap shipped a stop button which sat next to a finished group offering to
7+
* stop a command that had already exited — `hidden` was inert the whole time.
8+
*
9+
* The trap: the UA stylesheet's `[hidden] { display: none }` is a plain element-level rule, so
10+
* ANY class rule that sets `display` outclasses it. Toggling `.hidden` on such an element does
11+
* nothing at all. chat.html already carried four hand-written escapes for this (#workbar,
12+
* .modewrap, .modemenu, .st-approvals) — proof it is easy to hit and easy to forget.
13+
*--------------------------------------------------------------------------------------------*/
14+
// @ts-check
15+
'use strict';
16+
17+
const assert = require('assert');
18+
const fs = require('fs');
19+
const path = require('path');
20+
21+
const html = fs.readFileSync(path.join(__dirname, '..', 'media', 'chat.html'), 'utf8');
22+
const css = html.slice(html.indexOf('<style'), html.indexOf('</style>'));
23+
24+
let n = 0;
25+
function test(name, fn) { fn(); n++; console.log(' ok - ' + name); }
26+
27+
// Every class/id on a tag that ships the `hidden` attribute — i.e. every element that declares
28+
// "I get shown and hidden at runtime".
29+
function tokensDeclaredHidden() {
30+
const toks = new Set();
31+
for (const t of html.matchAll(/<\w+([^>]*?)\shidden(?=[\s>])([^>]*)>/g)) {
32+
const attrs = t[1] + ' ' + t[2];
33+
const cls = /class="([^"]+)"/.exec(attrs);
34+
if (cls) { cls[1].split(/\s+/).filter(Boolean).forEach((c) => toks.add('.' + c)); }
35+
const id = /id="([^"]+)"/.exec(attrs);
36+
if (id) { toks.add('#' + id[1]); }
37+
}
38+
return [...toks];
39+
}
40+
41+
// Rules whose selector mentions `tok`, split into the ones that set `display` unconditionally and
42+
// the `[hidden]`-qualified escape that would restore the UA behaviour.
43+
function displayRulesFor(tok) {
44+
const esc = tok.replace(/[.#]/g, '\\$&');
45+
const rules = [...css.matchAll(new RegExp('^\\s*([^{}\\n]*' + esc + '[^{}\\n]*)\\{([^}]*)\\}', 'gm'))];
46+
return {
47+
unconditional: rules.filter((r) => /display\s*:/.test(r[2]) && !/\[hidden\]/.test(r[1])).map((r) => r[1].trim()),
48+
hasEscape: rules.some((r) => /\[hidden\]/.test(r[1]) && /display\s*:\s*none/.test(r[2]))
49+
};
50+
}
51+
52+
test('the hidden attribute is never defeated by an explicit display', () => {
53+
const toks = tokensDeclaredHidden();
54+
assert.ok(toks.length >= 4, 'expected to find the runtime-toggled elements, found ' + toks.length);
55+
const broken = [];
56+
for (const tok of toks) {
57+
const { unconditional, hasEscape } = displayRulesFor(tok);
58+
if (unconditional.length && !hasEscape) { broken.push(tok + ' — set by: ' + unconditional.join(' | ')); }
59+
}
60+
assert.deepStrictEqual(broken, [],
61+
'these elements toggle `hidden` but a class rule forces `display`, so hiding them is a no-op.\n'
62+
+ 'Add a `<selector>[hidden] { display: none; }` rule, as #workbar and .modemenu already do:\n '
63+
+ broken.join('\n '));
64+
});
65+
66+
// The guard above only bites if the group's stop button is still a hidden-toggled element, so pin
67+
// the pairing directly: the reducer hides it on finalize, and the CSS must let that mean something.
68+
test('the group stop button is hideable, and the reducer hides it when the group closes', () => {
69+
assert.ok(/class="groupstop"[^>]*\shidden/.test(html), 'groupstop still ships the hidden attribute');
70+
assert.ok(/\.groupstop\[hidden\]\s*\{[^}]*display\s*:\s*none/.test(css), 'groupstop has its [hidden] escape');
71+
assert.ok(/g\.stopBtn\.hidden\s*=\s*true/.test(html), 'finalizeGroup still hides it');
72+
});
73+
74+
console.log('webviewCss: ' + n + ' tests passed');

0 commit comments

Comments
 (0)