fix(web): style editor no longer drops layout-only fields it never rendered

CodeRabbit flagged elementKeys() in style-editor.js: render() claims the
whole customization.layout child as the widget's own (removing it from the
generic fallback renderer, since posting the same offset twice is worse),
but elementKeys() only listed keys that also have their own top-level style
block. A hand-written schema can put a key under layout that never got one
-- a logo, a timeout indicator, a possession arrow with a position but no
font or colour -- and that key's only control silently disappeared: no row
in the style editor's table (elementKeys never listed it) and no fallback
section either (layout was removed wholesale).

elementKeys() now appends any layout-declared key not already covered by a
style element, so table() renders a row for it (layout columns only, no
style columns) and the wholesale layout ownership claim stays truthful.

Verified against current code before fixing. New regression test
(test/js/unit/test_style_editor_element_keys.js, following this repo's
existing eval-extraction pattern for testing widget JS without a browser)
fails against the reverted function and passes with the fix; added to
run_all.js and the suite table in test/js/README.md.

Full pytest suite: 4887 passed, 62 skipped, 2 failed -- both the
pre-existing Europe/Kiev/Asia/Calcutta tzdata-alias gap on this sandbox,
identical on origin/main, unrelated to this change.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Dpg3HLWohdCUdzz2QNHanm
This commit is contained in:
Claude
2026-09-13 23:38:37 +00:00
parent b8d344ae60
commit 75e6b2b20c
4 changed files with 124 additions and 2 deletions
+1
View File
@@ -36,6 +36,7 @@ nothing is listening, so it stays useful in a bare checkout.
|---|---|---|
| `unit/test_list_filter.js` | no | `ListFilter` search/filter/sort/count/sticky, and the installed-plugins config **extracted verbatim** from `plugins_manager.js` so the test can't drift from it |
| `unit/test_render_cards.js` | no | `renderInstalledCards` markup, both empty states, and HTML-escaping of hostile plugin metadata |
| `unit/test_style_editor_element_keys.js` | no | `elementKeys()` from `widgets/style-editor.js`: a hand-written `customization.layout` entry with no style block of its own still gets a row, since the widget claims the whole `layout` block from the generic fallback renderer |
| `dom/test_installed_dom.js` | yes | The toolbar in a real DOM: pill/search/sort interaction, the HTMX partial re-swap, and a `getComputedStyle` check that `.filter-pill[data-active]` really matches the emitted markup |
| `dom/test_store_dom.js` | yes | Store pagination, per-page, category, tri-state Installed button, and persistence across a re-boot, against the live registry |
| `dom/test_no_double_fetch.js` | yes | Loads the **whole** `plugins_manager.js` and counts requests: typing in the store search must filter the cached list, not refetch `/api/v3/plugins/store/list` |
+1 -1
View File
@@ -15,7 +15,7 @@ const fs = require('fs');
const BASE = process.env.BASE || 'http://localhost:5000';
const UNIT = ['unit/test_list_filter.js', 'unit/test_render_cards.js',
'unit/test_html_escaping.js'];
'unit/test_html_escaping.js', 'unit/test_style_editor_element_keys.js'];
const DOM = ['dom/test_installed_dom.js', 'dom/test_store_dom.js', 'dom/test_no_double_fetch.js',
'dom/test_tools_sections.js'];
@@ -0,0 +1,109 @@
// Regression test for style-editor.js's elementKeys(), extracted verbatim
// from the shipped widget (own/ownObj/elementKeys have no DOM dependency) so
// the test can't drift from the real implementation.
//
// render() in style-editor.js claims the whole `layout` child of a
// customization schema as the widget's own -- removing it from the generic
// fallback renderer entirely, because posting the same offset twice from
// two controls is worse than owning slightly too much. That is only safe
// when the style editor's own table actually renders a row for every key
// declared under `layout`. A hand-written schema can put a key under
// `layout` that never got its own top-level style block -- a logo, a
// timeout indicator, a possession arrow with a position but no font or
// colour -- and elementKeys() is what table() uses to decide which rows to
// draw. Miss one there and the fallback section that used to show it
// disappears with nothing replacing it.
const fs = require('fs');
const path = require('path');
const V3 = path.resolve(__dirname, '../../../web_interface/static/v3');
const src = fs.readFileSync(path.join(V3, 'js/widgets/style-editor.js'), 'utf8');
const start = src.indexOf('function own(obj, key)');
const endMark = 'function titleOf(schema, key)';
const end = src.indexOf(endMark);
if (start === -1 || end === -1) {
console.error('FAIL: could not locate own()/elementKeys() in style-editor.js -- slice markers need updating');
process.exit(1);
}
eval(src.slice(start, end)); // defines own/ownObj/elementKeys
let failures = 0;
function ok(desc, cond) {
if (cond) { console.log(`ok - ${desc}`); }
else { console.log(`FAIL - ${desc}`); failures++; }
}
// A hand-written schema: score_text has a real style block (and a layout
// offset), home_logo has ONLY a layout offset + scale -- no font/colour of
// its own, the shape _element_block_from_spec never gives a plain
// offsets-only element -- and "possession" lives only under layout, with no
// sibling block in customization.properties at all (the case a hand-authored
// schema can produce that the compact x-style-elements expander cannot).
const schema = {
properties: {
score_text: {
type: 'object',
'x-style-managed': true,
properties: {
font: { type: 'string' },
text_color: { type: 'array' },
},
},
layout: {
type: 'object',
properties: {
score_text: { type: 'object', properties: { x_offset: {}, y_offset: {} } },
possession: { type: 'object', properties: { x_offset: {}, y_offset: {} } },
},
},
},
};
const keys = elementKeys(schema);
ok('the declared style element is included', keys.indexOf('score_text') !== -1);
ok('a layout-only key with no style block of its own is still included',
keys.indexOf('possession') !== -1);
ok('layout and modes containers themselves are never treated as elements',
keys.indexOf('layout') === -1 && keys.indexOf('modes') === -1);
ok('nothing is duplicated', keys.length === new Set(keys).size);
// A schema with no layout block at all must behave exactly as before --
// this is the overwhelmingly common case and must not gain a phantom row.
const noLayout = {
properties: {
score_text: {
type: 'object',
'x-style-managed': true,
properties: { font: { type: 'string' } },
},
},
};
ok('a schema without a layout block is unaffected',
elementKeys(noLayout).join(',') === 'score_text');
// Every layout key already covered by a style element must not be listed
// twice (order: declared elements first, then layout-only extras).
const covered = {
properties: {
score_text: {
type: 'object', 'x-style-managed': true,
properties: { font: { type: 'string' } },
},
home_logo: {
type: 'object', 'x-style-managed': true,
properties: {},
},
layout: {
type: 'object',
properties: {
score_text: { type: 'object', properties: { x_offset: {} } },
home_logo: { type: 'object', properties: { x_offset: {}, scale: {} } },
},
},
},
};
ok('a layout key matching an existing element is not appended again',
elementKeys(covered).join(',') === 'score_text,home_logo');
if (failures) { console.log(`\n${failures} failure(s)`); process.exit(1); }
console.log('\nall checks passed');
@@ -159,7 +159,19 @@
});
// Nothing marked means the schema never went through expansion, so
// fall back to the shape test rather than rendering an empty table.
return managed.length ? managed : shaped;
var keys = managed.length ? managed : shaped;
// A hand-written layout block can offset something that never got
// a style block of its own -- a logo, a timeout indicator, a
// possession arrow with a position but no font, colour or size to
// set. render() claims the whole `layout` child as ours regardless
// (posting every covered offset twice from two controls is worse),
// so a key that only lives under layout still needs a row here, or
// it loses its only control the moment that fallback section is
// dropped.
var layoutKeys = Object.keys(ownObj(props, 'layout').properties || {});
var extra = layoutKeys.filter(function (k) { return keys.indexOf(k) === -1; });
return keys.concat(extra);
}
function titleOf(schema, key) {