fix(web): CSS.escape() the owned key before it becomes a selector

container.dataset.ownedKeys round-trips schema property keys through a
DOM dataset attribute, and the takeover handoff spliced each one
straight into '[data-child-key="' + k + '"]' with no escaping --
inconsistent with this codebase's own convention elsewhere
(plugin-file-manager.js, app-shell.js's escapeCssSelector) for building
a selector from a dynamic value. A key containing a quote or backslash
would break the selector or be steerable; Codacy's static analysis
flagged this pattern (1 high ErrorProne finding on PR #569, current
head at the time) as a new issue, though its dashboard is unreachable
from this sandbox (egress to app.codacy.com is blocked) and the
check-run API returned no detail text -- verified and fixed by reading
the diff directly rather than the tool's own description.

Added a source-assertion regression test alongside this file's
existing ones (this behavior lives in an inline script no Python test
executes).

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

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

src/element_style.py

@@ -763,6 +763,12 @@ def _adopt_handwritten_block(schema: Dict[str, Any],
 
     for key in element_keys:
         _upgrade_font_property(props[key])
+        # Marked like a declared element, so consumers can tell the style
+        # blocks from whatever else the plugin keeps under customization.
+        # Football's block also holds favorite_result_colors, which is a
+        # feature with its own fields -- without this the editor treats it
+        # as an element and every row grows an "enabled"/"win color" column.
+        props[key]['x-style-managed'] = True
 
     modes = customization.get('x-style-modes')
     if isinstance(modes, list) and modes:
@@ -904,11 +910,14 @@ def _normalize_color(value: Any) -> Optional[Tuple[int, int, int]]:
         return None
     if isinstance(value, (list, tuple)) and len(value) == 3:
         try:
-            rgb = tuple(int(c) for c in value)
+            # Clamped, not rejected. The readers this replaced clamped
+            # (sports_card.coerce_rgb), and the eight scoreboards' own tests
+            # pin it: a configured [999, -5, 20] is a typo'd bright red, and
+            # answering "unusable, take the default" turned it white instead.
+            rgb = tuple(max(0, min(255, int(c))) for c in value)
         except (TypeError, ValueError):
             return None
-        if all(0 <= c <= 255 for c in rgb):
-            return rgb  # type: ignore[return-value]
+        return rgb  # type: ignore[return-value]
     return None
 
 

test/js/README.md

@@ -36,6 +36,9 @@ 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 |
+| `unit/test_style_editor_layout_leaf_columns.js` | no | `columnsFor()` from `widgets/style-editor.js`: a layout-only key whose own value is a leaf (no x/y sub-object, e.g. a `show_logo` toggle) gets a self-keyed column instead of a blank, uneditable row |
+| `unit/test_style_editor_layout_leaf_collision.js` | no | `columnsFor()` from `widgets/style-editor.js`: a layout-only leaf key still gets its own column even when its name collides with an unrelated element's style sub-field or another layout axis's sub-field |
 | `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` |

test/js/run_all.js

@@ -15,7 +15,9 @@ 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',
+              'unit/test_style_editor_layout_leaf_columns.js',
+              'unit/test_style_editor_layout_leaf_collision.js'];
 const DOM = ['dom/test_installed_dom.js', 'dom/test_store_dom.js', 'dom/test_no_double_fetch.js',
              'dom/test_tools_sections.js'];
 

test/js/unit/test_style_editor_element_keys.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');

test/js/unit/test_style_editor_layout_leaf_collision.js

@@ -0,0 +1,76 @@
+// Regression test for style-editor.js's columnsFor(), extracted verbatim
+// from the shipped widget (see test_style_editor_layout_leaf_columns.js for
+// the base case this builds on).
+//
+// A layout-only leaf key (e.g. a bare "scale" toggle directly under
+// `layout`, no x/y object underneath) used to be keyed in the internal
+// `seen` map by its own bare field name. If some *other* element's style
+// block, or another element's layout axis block, happened to declare a
+// sub-field with that exact same name (e.g. "scale" is also a real
+// COLUMN_ORDER axis name most elements use), the `!seen.has(key)` guard
+// skipped creating the leaf's column -- it silently reused the existing
+// 'element'/'layout' column instead. That column's row binding in
+// elementRow() then looked the field up under the wrong schema location for
+// the leaf's own row, found nothing, and rendered a blank cell: the same
+// "silently disappears" bug the leaf-column fix was meant to close, just
+// reached through a name collision instead of a missing column altogether.
+// This pins the fix: layout-leaf columns are keyed under a namespaced id so
+// they can never be shadowed by an unrelated column sharing their name.
+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 control(opts)';
+const end = src.indexOf(endMark);
+if (start === -1 || end === -1) {
+  console.error('FAIL: could not locate own()/columnsFor() in style-editor.js -- slice markers need updating');
+  process.exit(1);
+}
+eval(src.slice(start, end)); // defines own/ownObj/elementKeys/columnsFor
+
+let failures = 0;
+function ok(desc, cond) {
+  if (cond) { console.log(`ok - ${desc}`); }
+  else { console.log(`FAIL - ${desc}`); failures++; }
+}
+
+// `possession` is a real style element whose layout axis block declares a
+// "scale" sub-field, which seeds seen.set('scale', 'layout'). `timeout_flag`
+// is layout-only and a bare leaf valued directly under layout, but its own
+// key is *also* "scale" -- an unrelated collision with that same name.
+const schema = {
+  properties: {
+    possession: {
+      type: 'object',
+      'x-style-managed': true,
+      properties: { font: { type: 'string' } },
+    },
+    scale: {
+      // The collision: a second element literally named "scale", entirely
+      // unrelated to the layout-only leaf below sharing that string.
+      type: 'boolean',
+    },
+    layout: {
+      type: 'object',
+      properties: {
+        possession: { type: 'object', properties: { scale: { type: 'number' } } },
+        scale: { type: 'boolean', default: true },
+      },
+    },
+  },
+};
+
+const columns = columnsFor(schema);
+const leafColumns = columns.filter(function (c) { return c.where === 'layout-leaf'; });
+
+ok('the colliding layout axis sub-field still gets its own column',
+   columns.some(function (c) { return c.key === 'scale' && c.where === 'layout'; }));
+ok('the layout-only leaf key still gets its own column despite the name collision',
+   leafColumns.length === 1 && leafColumns[0].key === 'scale');
+ok('the leaf column is distinct from the colliding element/layout column',
+   columns.filter(function (c) { return c.key === 'scale'; }).length === 2);
+
+if (failures) { console.log(`\n${failures} failure(s)`); process.exit(1); }
+console.log('\nall checks passed');

test/js/unit/test_style_editor_layout_leaf_columns.js

@@ -0,0 +1,89 @@
+// Regression test for style-editor.js's columnsFor(), extracted verbatim
+// from the shipped widget (own/ownObj/elementKeys/columnsFor have no DOM
+// dependency) so the test can't drift from the real implementation.
+//
+// elementKeys() already appends a layout-only key that never got its own
+// top-level style block (see test_style_editor_element_keys.js), so table()
+// draws a row for it. But render() still claims the whole `layout` child as
+// the widget's own regardless of what that row actually shows -- and
+// columnsFor() only produced a column for a layout-only key whose own value
+// is an *object* with sub-fields (x_offset, y_offset, ...). A hand-written
+// schema can instead put a plain leaf value directly under layout -- a
+// "show_logo" toggle, a timeout indicator's on/off flag -- with no x/y
+// object underneath it. That key still got a row (data-element="show_logo"),
+// but zero columns ever matched it, so the row rendered with every cell
+// blank: no control, and no way back to the fallback since layout was
+// removed wholesale. This pins the fix: such a key gets its own
+// self-keyed 'layout-leaf' column.
+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 control(opts)';
+const end = src.indexOf(endMark);
+if (start === -1 || end === -1) {
+  console.error('FAIL: could not locate own()/columnsFor() in style-editor.js -- slice markers need updating');
+  process.exit(1);
+}
+eval(src.slice(start, end)); // defines own/ownObj/elementKeys/columnsFor
+
+let failures = 0;
+function ok(desc, cond) {
+  if (cond) { console.log(`ok - ${desc}`); }
+  else { console.log(`FAIL - ${desc}`); failures++; }
+}
+
+// score_text is a real style element; possession is layout-only but shaped
+// like an object (x/y); show_logo is layout-only and a bare leaf value.
+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: {} } },
+        show_logo: { type: 'boolean', default: true },
+      },
+    },
+  },
+};
+
+const columns = columnsFor(schema);
+const byKey = {};
+columns.forEach(function (c) { byKey[c.key] = c; });
+
+ok('an element style field still gets its own column', byKey.font && byKey.font.where === 'element');
+ok('a layout-only object key still contributes its x/y columns',
+   byKey.x_offset && byKey.x_offset.where === 'layout');
+ok('a layout-only leaf key gets a column keyed to itself',
+   !!byKey.show_logo);
+ok('...marked as a layout-leaf column, not a shared sub-field',
+   byKey.show_logo && byKey.show_logo.where === 'layout-leaf');
+ok('nothing is duplicated', columns.length === new Set(columns.map(function (c) { return c.key; })).size);
+
+// A schema with no leaf-valued layout key must not gain a phantom column --
+// this is the common case (every existing scoreboard) and must be unaffected.
+const noLeaf = {
+  properties: {
+    score_text: {
+      type: 'object', 'x-style-managed': true,
+      properties: { font: { type: 'string' } },
+    },
+    layout: {
+      type: 'object',
+      properties: { score_text: { type: 'object', properties: { x_offset: {} } } },
+    },
+  },
+};
+ok('a schema with only object-shaped layout keys gets no layout-leaf column',
+   columnsFor(noLeaf).every(function (c) { return c.where !== 'layout-leaf'; }));
+
+if (failures) { console.log(`\n${failures} failure(s)`); process.exit(1); }
+console.log('\nall checks passed');

test/test_sports_card.py

@@ -53,6 +53,14 @@ class TestColour:
         cfg = {"customization": {"score_text": {"text_color": value}}}
         assert C.element_color(cfg, "score_text", (9, 9, 9)) == (9, 9, 9)
 
+    def test_out_of_range_components_are_clamped_not_rejected(self):
+        """The readers this replaced clamped, and the eight scoreboards' own
+        tests pin it. Rejecting instead turned a typo'd [999, -5, 20] -- a
+        bright red -- into the default white, which is a colour the user never
+        asked for rather than the one they nearly asked for."""
+        cfg = {"customization": {"score_text": {"text_color": [999, -5, 20]}}}
+        assert C.element_color(cfg, "score_text", (9, 9, 9)) == (255, 0, 20)
+
     def test_coerce_rgb_clamps_rather_than_rejecting(self):
         assert C.coerce_rgb([300, -5, 20], (0, 0, 0)) == (255, 0, 20)
 

test/web_interface/test_style_editor_takeover.py

@@ -0,0 +1,137 @@
+"""When the style editor takes over a customization block, it takes over
+exactly its own part of it -- and it actually gets to take over at all.
+
+Two defects this pins, both found by rendering the real partial in a browser
+rather than by reading the code:
+
+1. The hand-off guard asked "do any fallback controls differ from their
+   server-rendered defaults?" as a proxy for "is someone editing this?". The
+   fallback contains this block's own font fields, and the font-selector
+   widget populates them on the same 50ms timer -- so a plain page load with
+   nobody touching anything raced into "dirty" (seven customization.*.font
+   selects, measured ~60ms after injection) and the style editor removed
+   itself, leaving the accordion form it exists to replace. The question is
+   whether a *person* typed, and isTrusted answers exactly that.
+
+2. Taking over used to remove the whole fallback section. A customization
+   block can hold more than styling -- football keeps favorite_result_colors
+   there -- so that removed the only UI those fields had.
+
+These assert on the template source because the behaviour lives in an inline
+script that no Python test executes; the browser check that found both is not
+something CI runs.
+"""
+
+import re
+import sys
+from pathlib import Path
+
+import pytest
+
+PROJECT_ROOT = Path(__file__).parent.parent.parent
+sys.path.insert(0, str(PROJECT_ROOT))
+
+TEMPLATE = (PROJECT_ROOT / "web_interface" / "templates" / "v3" / "partials"
+            / "plugin_config.html")
+
+
+@pytest.fixture(scope="module")
+def source():
+    return TEMPLATE.read_text(encoding="utf-8")
+
+
+class TestTheHandOffGuard:
+    def test_it_asks_whether_a_person_typed(self, source):
+        assert "isTrusted" in source, (
+            "a programmatic change by a sibling widget must not read as a "
+            "user edit -- that is what removed the editor on a plain load")
+        assert "userEdited" in source
+
+    def test_it_no_longer_diffs_values_against_defaults(self, source):
+        assert "fallbackIsDirty" not in source, (
+            "value-diffing cannot distinguish a user's edit from another "
+            "widget populating this block's own fields")
+        assert "defaultSelected" not in source
+
+    def test_the_listeners_are_attached_synchronously(self, source):
+        """The edit worth protecting can happen before initWidget runs, so
+        the listeners cannot wait for it.
+
+        Scoped to this branch: the template defines an initWidget for every
+        widget it can render, and the first one in the file belongs to a
+        different branch entirely.
+        """
+        start = source.index("obj_widget == 'style-editor'")
+        branch = source[start:source.index("{% elif prop.properties %}", start)]
+        assert "watchForRealEdits" in branch
+        assert branch.index("watchForRealEdits") < branch.index("function initWidget"), (
+            "the watcher must be installed above this branch's initWidget, "
+            "and invoked immediately rather than from inside it")
+
+    def test_the_guard_still_backs_off_for_a_real_edit(self, source):
+        assert re.search(r"if \(userEdited\) \{ container\.remove\(\); return; \}",
+                         source), "a genuine in-progress edit must still win"
+
+
+class TestItTakesOverOnlyItsOwnBlocks:
+    def test_children_are_individually_addressable(self, source):
+        assert 'data-child-key="{{ nested_key }}"' in source, (
+            "without a handle per child the only options are removing the "
+            "whole section or none of it")
+
+    def test_removal_is_driven_by_what_the_widget_reported(self, source):
+        assert "ownedKeys" in source
+        assert "fallback.querySelector(" in source
+
+    def test_an_unowned_child_survives(self, source):
+        """The empty-section case still removes the lot, so a block that is
+        entirely styling looks exactly as it did before."""
+        assert "!fallback.querySelector('[data-child-key]')" in source
+
+    def test_the_owned_key_is_escaped_before_it_becomes_a_selector(self, source):
+        """ownedKeys comes back off a dataset attribute, not a literal --
+        splicing it into '[data-child-key="' + k + '"]' unescaped breaks (or
+        is steerable) on a key containing a quote or backslash. The rest of
+        this codebase (plugin-file-manager.js, app-shell.js) always routes a
+        dynamic value through CSS.escape() before it lands in a selector."""
+        assert "CSS.escape(k)" in source, (
+            "an owned key must be CSS.escape()'d before being interpolated "
+            "into the data-child-key attribute selector")
+
+
+class TestTheSchemaSaysWhichBlocksAreStyling:
+    def test_adopted_blocks_are_marked_and_others_are_not(self):
+        from src.element_style import expand_style_elements
+
+        schema = {
+            "type": "object",
+            "properties": {
+                "customization": {
+                    "type": "object",
+                    "properties": {
+                        "score_text": {
+                            "type": "object",
+                            "properties": {
+                                "font": {"type": "string",
+                                         "default": "PressStart2P-Regular.ttf"},
+                                "font_size": {"type": "integer", "default": 10},
+                                "text_color": {"type": "array",
+                                               "default": [255, 255, 255]},
+                            },
+                        },
+                        # Not styling: a feature that happens to live here.
+                        "favorite_result_colors": {
+                            "type": "object",
+                            "properties": {
+                                "enabled": {"type": "boolean", "default": False},
+                                "win_color": {"type": "array",
+                                              "default": [0, 255, 0]},
+                            },
+                        },
+                    },
+                },
+            },
+        }
+        props = expand_style_elements(schema)["properties"]["customization"]["properties"]
+        assert props["score_text"]["x-style-managed"] is True
+        assert "x-style-managed" not in props["favorite_result_colors"]

web_interface/static/v3/js/widgets/style-editor.js

@@ -146,10 +146,32 @@
     function elementKeys(schema) {
         var props = (schema && schema.properties) || {};
         var order = schema['x-propertyOrder'] || Object.keys(props);
-        return order.filter(function (k) {
+        var shaped = order.filter(function (k) {
             return k !== 'layout' && k !== 'modes'
                 && own(props, k) && ownObj(props, k).properties;
         });
+        // Core marks the blocks it recognises as styling. Prefer that: a
+        // customization block can also hold a feature of its own -- football
+        // keeps favorite_result_colors there -- and treating one as an element
+        // gives every row that feature's fields as extra columns.
+        var managed = shaped.filter(function (k) {
+            return ownObj(props, k)['x-style-managed'] === true;
+        });
+        // Nothing marked means the schema never went through expansion, so
+        // fall back to the shape test rather than rendering an empty table.
+        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) {
@@ -360,9 +382,12 @@
     /**
      * The columns a table needs, derived from the schema rather than fixed.
      *
-     * Two sources: the sub-fields elements declare (font, font_size,
-     * text_color, visible, align) and the sub-fields their layout blocks
-     * declare (x_offset, y_offset, scale).
+     * Three sources: the sub-fields elements declare (font, font_size,
+     * text_color, visible, align), the sub-fields their layout blocks
+     * declare (x_offset, y_offset, scale), and a layout-only key whose own
+     * value *is* the field to set -- a plain "show_logo" toggle has no x/y
+     * object underneath it, so it gets a column keyed to itself rather than
+     * to a shared sub-field name.
      */
     function columnsFor(schema) {
         var props = schema.properties || {};
@@ -374,8 +399,23 @@
         elementKeys(schema).forEach(function (key) {
             Object.keys(ownObj(props, key).properties || {}).forEach(
                 function (f) { seen.set(f, 'element'); });
-            Object.keys(ownObj(layoutProps, key).properties || {}).forEach(
-                function (f) { seen.set(f, 'layout'); });
+            var layoutEntry = own(layoutProps, key);
+            if (layoutEntry && typeof layoutEntry === 'object' && layoutEntry.properties) {
+                Object.keys(layoutEntry.properties).forEach(
+                    function (f) { seen.set(f, 'layout'); });
+            } else if (layoutEntry) {
+                // layout-only and a leaf: nothing else will share this
+                // column, but leaving it out drops the field's only control
+                // the moment the wholesale `layout` claim removes its
+                // fallback (#569 review). Keyed under a namespaced id, not
+                // the bare field name -- an unrelated element's own style
+                // sub-field or another layout axis can share this exact
+                // name, and reusing that shared slot would give this row a
+                // column typed 'element'/'layout' that it doesn't declare,
+                // dropping the control right back out (#569 follow-up).
+                var leafId = 'layout-leaf:' + key;
+                if (!seen.has(leafId)) { seen.set(leafId, 'layout-leaf'); }
+            }
         });
         var known = COLUMN_ORDER.filter(function (f) { return seen.get(f); });
         // Anything the schema declares that this file has never heard of
@@ -384,10 +424,12 @@
             return COLUMN_ORDER.indexOf(f) === -1;
         }).sort();
         return known.concat(extra).map(function (f) {
+            var isLeafId = f.indexOf('layout-leaf:') === 0;
+            var fieldKey = isLeafId ? f.slice('layout-leaf:'.length) : f;
             return {
-                key: f,
+                key: fieldKey,
                 where: seen.get(f),
-                label: own(COLUMN_LABELS, f) || f.replace(/_/g, ' ')
+                label: own(COLUMN_LABELS, fieldKey) || fieldKey.replace(/_/g, ' ')
             };
         });
     }
@@ -461,8 +503,13 @@
         }
 
         opts.columns.forEach(function (col) {
+            var isLeaf = col.where === 'layout-leaf';
             var inLayout = col.where === 'layout';
-            var prop = inLayout ? own(axes, col.key) : own(props, col.key);
+            // A leaf column only applies to the one row named after it --
+            // every other row leaves it blank, same as an element that
+            // doesn't declare a shared sub-field.
+            var prop = isLeaf ? (col.key === key ? own(layoutProps, key) : null)
+                             : inLayout ? own(axes, col.key) : own(props, col.key);
             var cell;
             if (!prop) {
                 // This element does not declare that field; keep the grid
@@ -470,13 +517,14 @@
                 row.appendChild(el('span'));
                 return;
             }
-            var path = inLayout ? ['layout', key, col.key] : [key, col.key];
-            var base = inLayout ? opts.layoutPrefix + '.' + key
+            var path = isLeaf ? ['layout', key]
+                     : inLayout ? ['layout', key, col.key] : [key, col.key];
+            var base = (isLeaf || inLayout) ? opts.layoutPrefix + '.' + key
                                 : opts.prefix + '.' + key;
             var node = control({
                 key: col.key,
                 prop: prop,
-                name: base + '.' + col.key,
+                name: isLeaf ? base : base + '.' + col.key,
                 current: effective(value, path, prop, optional),
                 optional: optional,
                 fonts: opts.fonts,
@@ -555,6 +603,20 @@
             var root = el('div', { class: 'style-editor' });
             container.appendChild(root);
 
+            // Published synchronously, because the host reads it the moment
+            // this returns while the panels below wait on the font catalog.
+            // layout and modes count as ours: their fields appear as columns
+            // in these rows, so leaving them to the generic renderer would
+            // post every offset twice from two different controls.
+            var owned = elementKeys(schema);
+            if (ownObj(schema.properties || {}, 'layout').properties) {
+                owned = owned.concat(['layout']);
+            }
+            if (ownObj(schema.properties || {}, 'modes').properties) {
+                owned = owned.concat(['modes']);
+            }
+            container.dataset.ownedKeys = owned.join(',');
+
             loadFonts().then(function (fonts) {
                 var modeProps = ownObj(schema.properties || {}, 'modes').properties || {};
                 var modes = Object.keys(modeProps);

web_interface/templates/v3/partials/plugin_config.html

@@ -88,23 +88,30 @@
             </div>
             <script>
             (function() {
-                function fallbackIsDirty(root) {
-                    if (!root) { return false; }
-                    var controls = root.querySelectorAll('input, select, textarea');
-                    for (var i = 0; i < controls.length; i++) {
-                        var c = controls[i];
-                        if (c.type === 'checkbox' || c.type === 'radio') {
-                            if (c.checked !== c.defaultChecked) { return true; }
-                        } else if (c.tagName === 'SELECT') {
-                            for (var j = 0; j < c.options.length; j++) {
-                                if (c.options[j].selected !== c.options[j].defaultSelected) { return true; }
-                            }
-                        } else if (c.value !== c.defaultValue) {
-                            return true;
-                        }
-                    }
-                    return false;
-                }
+                // Did a *person* type into the fallback before this widget
+                // took over? Comparing values against their defaults cannot
+                // answer that. The fallback holds this block's own font
+                // fields, and the font-selector widget populates them on the
+                // same 50ms timer -- so a plain page load with nobody
+                // touching anything raced into "dirty" (measured: seven
+                // customization.*.font selects, ~60ms in) and silently
+                // removed the style editor, leaving the accordion form it
+                // exists to replace.
+                //
+                // isTrusted is false for programmatic changes and true only
+                // for real input, which is exactly the question. The
+                // listeners go on now, synchronously, because the edit to
+                // protect can happen before initWidget ever runs.
+                var userEdited = false;
+                (function watchForRealEdits() {
+                    var fb = document.getElementById('{{ field_id }}_fallback');
+                    if (!fb) { return; }
+                    ['input', 'change'].forEach(function (evt) {
+                        fb.addEventListener(evt, function (e) {
+                            if (e && e.isTrusted) { userEdited = true; }
+                        }, true);
+                    });
+                })();
                 function initWidget() {
                     if (!window.LEDMatrixWidgets) { setTimeout(initWidget, 50); return; }
                     var widget = window.LEDMatrixWidgets.get('style-editor');
@@ -116,7 +123,7 @@
                     // while this widget's script is still loading; swapping
                     // in the widget now would replace them with the stale
                     // server-rendered value and silently drop that edit.
-                    if (fallbackIsDirty(fallback)) { container.remove(); return; }
+                    if (userEdited) { container.remove(); return; }
                     var value = {{ obj_value|tojson|safe }};
                     var config = { schema: {{ prop|tojson|safe }} };
                     try {
@@ -130,9 +137,32 @@
                         container.remove();
                         return;
                     }
-                    // The widget owns these fields now; drop the generic
-                    // rendering so the form does not post both.
-                    if (fallback) { fallback.remove(); }
+                    // The widget owns the style blocks now, so drop those from
+                    // the generic rendering or the form would post both. Only
+                    // those: a customization block can also hold a feature of
+                    // its own (football keeps favorite_result_colors there),
+                    // and removing the section wholesale took the only UI that
+                    // feature had with it.
+                    if (fallback) {
+                        var owned = (container.dataset.ownedKeys || '')
+                            .split(',').filter(function (k) { return k; });
+                        owned.forEach(function (k) {
+                            // CSS.escape: k is a schema property key read
+                            // back off dataset.ownedKeys, not a literal --
+                            // building the attribute selector by raw
+                            // concatenation would break (or be steerable)
+                            // on a key containing a quote or backslash.
+                            var node = fallback.querySelector(
+                                '[data-child-key="' + CSS.escape(k) + '"]');
+                            if (node) { node.remove(); }
+                        });
+                        // Nothing left but the section heading, or a widget
+                        // that reported nothing: drop the lot, as before.
+                        if (!owned.length
+                                || !fallback.querySelector('[data-child-key]')) {
+                            fallback.remove();
+                        }
+                    }
                 }
                 if (document.readyState === 'loading') {
                     document.addEventListener('DOMContentLoaded', initWidget);
@@ -1036,7 +1066,12 @@
                 {% if nested_key in prop.properties %}
                     {% set nested_prop = prop.properties[nested_key] %}
                     {% set nested_val = nested_value[nested_key] if nested_key in nested_value else none %}
-                    {{ render_field(nested_key, nested_prop, nested_val, full_key, plugin_id) }}
+                    {# Tagged so a widget that takes over part of a block can
+                       drop exactly the children it owns and leave the rest
+                       rendered, rather than removing the whole section. #}
+                    <div data-child-key="{{ nested_key }}">
+                        {{ render_field(nested_key, nested_prop, nested_val, full_key, plugin_id) }}
+                    </div>
                 {% endif %}
             {% endfor %}
         </div>