fix(highway_3d): Venue desync, bind race, and a11y for the player background control (#1018)

* Fix 3D Highway background controls under Venue override

When the Venue visualization override is active, the entire Background control group (style dropdown and intensity/reactive knobs) is now disabled since the venue scene owns rendering. The dropdown, intensity, and reactive controls are greyed out with a tooltip explaining the state.

Also fixes a cold-load bug where the screen hook subscription could fail to bind if the event bus wasn't ready during renderer init, by re-attempting binding on each retry tick.

Includes test coverage for both the Venue override greyout behavior and the cold-load screen hook binding fix.

* Add accessibility features and explicit global reads to background control

Refactor the player chrome background control to use an explicit `_bgReadGlobal()` function instead of relying on the implicit behavior of `_bgReadSetting(null, ...)`. The control is a single shared instance across splitscreen panels and must always read/write the global slot.

Add accessibility improvements:
- aria-pressed on toggle buttons to expose state to screen readers
- aria-label on select and intensity controls
- aria-describedby pointing disabled controls to a visually-hidden reason span
- The reason span carries dynamic explanatory text for why a control is greyed out

Add comprehensive tests verifying the new `_bgReadGlobal` helper ignores per-panel overrides and that all accessibility attributes are set and updated correctly.

* Gate player control slot on v3 UI version

Add explicit check for `window.feedBack.uiVersion === 'v3'` in _pcSlot() per docs/plugin-v3-ui.md. This prevents the plugin from attempting to mount player controls on non-v3 hosts (e.g., legacy v2 shell). Complements the existing `playerControlSlot` typeof check and improves compatibility robustness.

Updated test mocks to include `uiVersion: 'v3'` and added test case verifying that mounting is skipped when uiVersion is not v3, including a guard to ensure the retry loop terminates properly.

* Clarify 3D highway style control behavior

Document that the style controls group also greyes out when the Venue scene override is active, since the controls don't apply in that mode.

* Restore style dropdown tooltip when Venue override exits

The style dropdown's tooltip was cleared whenever the Venue override was inactive, permanently discarding the "Background style" hint set at mount time. Since the sync runs on every settings change, the tooltip was lost on the first sync and never returned.

This brings the dropdown in line with the intensity slider and reactive toggle, which already restore their base tooltip when they're re-enabled.

Includes a test asserting the tooltip returns after the Venue override exits.

* fix(highway_3d): skip player-control retry loop on non-v3 shells

_pcAcquire only runs once the renderer is viable inside the v3 player
chrome, and player-chrome.js sets uiVersion synchronously as it builds
that chrome — so a missing 'v3' at acquire means v2, not a not-yet-ready
v3. Bail before scheduling the retry loop instead of spinning it out to
the ~3s budget for a slot that will never appear.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: byrongamatos <xasiklas@gmail.com>

---------

Signed-off-by: byrongamatos <xasiklas@gmail.com>
Co-authored-by: byrongamatos <xasiklas@gmail.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

plugins/highway_3d/tests/background_control.test.js

@@ -37,8 +37,9 @@ const END_LF = '    /* =========================================================
 // table, which would only assert that the table equals itself.
 //   intensity: true  => the style's build() reads settings.intensity
 //   reactive:  true  => the style's update() dereferences its `bands` argument
-// 'butterchurn' is not a BG_STYLES entry at all (mount falls through to
-// BG_STYLES.off) and drives its own audio tap, so both are false.
+// 'butterchurn' is a mode, not a BG_STYLES fog-scenery entry: _bcSyncMode
+// owns its controller and drives its own audio tap + canvas opacity (only
+// the fog-scenery half falls through to BG_STYLES.off), so both are false.
 const EXPECTED_USES = {
     off: { intensity: false, reactive: false },
     particles: { intensity: true, reactive: true },
@@ -73,6 +74,7 @@ function makeDom() {
         }
         addEventListener(t, fn) { (this.listeners[t] || (this.listeners[t] = [])).push(fn); }
         setAttribute(k, v) { this[k] = v; }
+        removeAttribute(k) { delete this[k]; }
         get isConnected() {
             let n = this;
             while (n.parentNode) n = n.parentNode;
@@ -127,7 +129,12 @@ function load({ store: initialStore } = {}) {
     const sandbox = {
         console,
         BG_STYLE_IDS,
+        // Module-scope in screen.js; the _pc* block reads it to resolve the
+        // effective style under the Venue override. Tests flip it via
+        // sandbox._venueSceneOverride and fire the 'venueScene' bus key.
+        _venueSceneOverride: false,
         _bgReadSetting: (_panelKey, key) => store[key],
+        _bgReadGlobal: (key) => store[key],
         _bgSubscribe: (fn) => listeners.add(fn),
         _bgUnsubscribe: (fn) => listeners.delete(fn),
         setTimeout: (fn) => { timers.push(fn); return timers.length; },
@@ -139,6 +146,7 @@ function load({ store: initialStore } = {}) {
         },
         window: {
             feedBack: {
+                uiVersion: 'v3',   // _pcSlot gates on this (docs/plugin-v3-ui.md)
                 ui: { playerControlSlot: () => dom.slot },
                 // The real bus is an EventTarget wrapper exposing on/off. Modelled
                 // here so the screen:changed subscription — and its removal — are
@@ -165,6 +173,7 @@ function load({ store: initialStore } = {}) {
         + '   get sel() { return _pcSel; },'
         + '   get react() { return _pcReactive; },'
         + '   get intens() { return _pcIntensity; },'
+        + '   get reason() { return _pcReason; },'
         + '   get refs() { return _pcRefs; } })',
         sandbox,
     );
@@ -173,6 +182,50 @@ function load({ store: initialStore } = {}) {
     return { api, dom, store, emit, writes, timers, sandbox, listenerCount: () => listeners.size, fireScreenChanged, screenHooks };
 }
 
+// Slice the real _bgReadSetting + _bgReadGlobal out of screen.js and run them
+// against a localStorage stub. The main suite stubs both helpers identically,
+// so it can't tell the #2 refactor from a no-op; this one proves the actual
+// helper bodies differ where they must: _bgReadGlobal ignores a per-panel
+// override that _bgReadSetting(panelKey, ...) still honours.
+test('_bgReadGlobal reads the global slot, ignoring per-panel overrides', () => {
+    const src = fs.readFileSync(SCREEN_JS, 'utf8');
+    const rgStart = src.indexOf('    function _bgReadSetting(panelKey, key) {');
+    const rgEnd = src.indexOf('    // Shared "stored string -> bool" coercion');
+    assert.ok(rgStart !== -1 && rgEnd > rgStart, 'could not slice the read helpers');
+    const block = src.slice(rgStart, rgEnd);
+
+    const storage = new Map();
+    const sandbox = {
+        localStorage: { getItem: (k) => (storage.has(k) ? storage.get(k) : null) },
+        _bgCoerce: (_key, v) => v,           // identity: we test key resolution, not coercion
+        _bgMemFallback: Object.create(null),
+        BG_DEFAULTS: { style: 'particles' },
+    };
+    sandbox.globalThis = sandbox;
+    const api = vm.runInNewContext(block + '\n({ _bgReadSetting, _bgReadGlobal, _bgMemFallback })', sandbox);
+
+    storage.set('h3d_bg_style', 'lights');            // global
+    storage.set('h3d_bg_panel3_style', 'geometric');  // a per-panel override
+
+    // The renderer, reading with a panel key, honours the per-panel override...
+    assert.equal(api._bgReadSetting('panel3', 'style'), 'geometric');
+    // ...but the shared control's global read must NOT see it - this is the
+    // whole point of #2 (previously _bgReadSetting(null, ...) relied on
+    // 'h3d_bg_null_style' never existing).
+    assert.equal(api._bgReadGlobal('style'), 'lights');
+
+    // In-memory staged value wins over the persisted global (matches
+    // _bgReadSetting's precedence).
+    api._bgMemFallback.style = 'aurora';
+    assert.equal(api._bgReadGlobal('style'), 'aurora');
+    delete api._bgMemFallback.style;
+
+    // Nothing stored -> BG_DEFAULTS.
+    assert.equal(api._bgReadGlobal('style'), 'lights');
+    storage.delete('h3d_bg_style');
+    assert.equal(api._bgReadGlobal('style'), 'particles');
+});
+
 test('mounts one control into the player-control slot', () => {
     const { api, dom } = load();
     api._pcAcquire();
@@ -199,6 +252,29 @@ test('multiple renderer instances share a single control', () => {
     assert.equal(api.el, null);
 });
 
+test('binds the screen hook on a retry when the bus was not ready at acquire', () => {
+    const ctl = load();
+    // Cold load: on a fresh page the renderer can init before the event bus is
+    // wired AND before the rail popover exists. Simulate both being absent.
+    const savedOn = ctl.sandbox.window.feedBack.on;
+    const savedUi = ctl.sandbox.window.feedBack.ui;
+    delete ctl.sandbox.window.feedBack.on;
+    ctl.sandbox.window.feedBack.ui = {};   // no playerControlSlot -> mount fails
+
+    ctl.api._pcAcquire();
+    assert.equal(ctl.screenHooks(), 0, 'nothing to bind to yet');
+    assert.equal(ctl.api.el, null, 'no slot yet, so nothing mounted');
+
+    // Bus + slot come online; the retry tick must bind the hook, not only mount.
+    ctl.sandbox.window.feedBack.on = savedOn;
+    ctl.sandbox.window.feedBack.ui = savedUi;
+    ctl.timers.shift()();   // run one retry tick
+
+    assert.equal(ctl.screenHooks(), 1, 'the retry tick failed to bind the screen hook');
+    assert.ok(ctl.api.el, 'and it should have mounted too');
+    ctl.api._pcRelease();
+});
+
 test('the last release unbinds the screen:changed hook', () => {
     const ctl = load();
     ctl.api._pcAcquire();
@@ -261,6 +337,18 @@ test('re-mounts into a fresh slot when the player chrome is rebuilt', () => {
     assert.equal(listenerCount(), 1, 'remount must not double-subscribe');
 });
 
+test('a non-v3 host mounts nothing (uiVersion gate)', () => {
+    const ctl = load();
+    ctl.sandbox.window.feedBack.uiVersion = 'v2';   // pre-v3 shell
+    ctl.api._pcAcquire();
+    assert.equal(ctl.api.el, null, 'must not mount when uiVersion is not v3');
+    assert.equal(ctl.dom.slot.children.length, 0);
+    // A non-v3 shell has no slot and never will, so no retry should be scheduled
+    // at all — the loop is for a not-yet-built v3 slot, not for polling v2.
+    assert.equal(ctl.timers.length, 0, 'a non-v3 host must not schedule the retry loop');
+    ctl.api._pcRelease();
+});
+
 test('a host with no player-control slot mounts nothing and does not throw', () => {
     const { api, dom, sandbox, timers } = load();
     sandbox.window.feedBack.ui = {};
@@ -300,6 +388,49 @@ test('the dropdown and Reactive pill drive the real setters', () => {
     assert.ok(writes.some((w) => w[0] === 'reactive'));
 });
 
+test('exposes state and reasons to assistive tech', () => {
+    const ctl = load({ store: { style: 'image', reactive: true } });   // image: reactive inert
+    ctl.api._pcAcquire();
+
+    // The reason live-region must be a REAL mounted element with the id the
+    // controls reference - not a dangling pointer. Assert resolution, not a
+    // literal (a wrong id in code would still equal the literal).
+    const reason = ctl.api.reason;
+    assert.ok(reason, 'the reason span was not created');
+    assert.equal(reason.id, 'h3d-pc-reason');
+    assert.equal(reason.parentNode, ctl.api.el, 'the reason span must be mounted in the control');
+
+    // aria-pressed: a toggle button must expose its state. image greys
+    // Reactive, so not-pressed AND disabled, and it points at the reason.
+    assert.equal(ctl.api.react['aria-pressed'], 'false', 'greyed toggle is not pressed');
+    assert.equal(ctl.api.react['aria-disabled'], 'true');
+    // Pointer must resolve to the actual span's id (kills a wrong-id mutation),
+    // and the span must carry the current reason text (kills a never-set-text
+    // mutation).
+    assert.equal(ctl.api.react['aria-describedby'], reason.id, 'inert control must reference the reason span');
+    assert.equal(reason.textContent, 'This background does not react to audio', 'reason text must match the style');
+    assert.equal(ctl.api.intens['aria-describedby'], undefined, 'an ENABLED control carries no reason');
+
+    // The intensity describe path: a style where INTENSITY is inert.
+    ctl.store.style = 'video'; ctl.emit('style');
+    assert.equal(ctl.api.intens.disabled, true, 'precondition: video greys intensity');
+    assert.equal(ctl.api.intens['aria-describedby'], reason.id, 'inert intensity must reference the reason');
+    assert.equal(reason.textContent, 'The video plays as-is - nothing to adjust here');
+
+    // Both enabled: describedby drops, aria-pressed follows the value.
+    ctl.store.style = 'particles'; ctl.store.reactive = true; ctl.emit('style');
+    assert.equal(ctl.api.react['aria-describedby'], undefined, 'enabled control drops the reason');
+    assert.equal(ctl.api.intens['aria-describedby'], undefined);
+    assert.equal(ctl.api.react['aria-pressed'], 'true', 'reactive on for particles');
+    ctl.store.reactive = false; ctl.emit('reactive');
+    assert.equal(ctl.api.react['aria-pressed'], 'false', 'aria-pressed follows the value');
+
+    // Accessible names on the non-label controls.
+    assert.equal(ctl.api.sel['aria-label'], 'Background style');
+    assert.equal(ctl.api.intens['aria-label'], 'Background intensity');
+    ctl.api._pcRelease();
+});
+
 test('greys out exactly the controls each style ignores', () => {
     const { api, store, emit } = load();
     api._pcAcquire();
@@ -311,6 +442,48 @@ test('greys out exactly the controls each style ignores', () => {
     }
 });
 
+test('the Venue override greys the whole Background group', () => {
+    const ctl = load({ store: { style: 'particles' } });   // a style that uses both
+    ctl.api._pcAcquire();
+    assert.equal(ctl.api.intens.disabled, false, 'precondition: both enabled off-venue');
+    assert.equal(ctl.api.react.disabled, false);
+
+    // Venue turns on: the effective style is now 'venue', which uses neither.
+    // The transition arrives on the settings bus as the 'venueScene' key.
+    ctl.sandbox._venueSceneOverride = true;
+    ctl.emit('venueScene');
+    assert.equal(ctl.api.intens.disabled, true, 'intensity should grey under Venue');
+    assert.equal(ctl.api.react.disabled, true, 'reactive should grey under Venue');
+    assert.equal(ctl.api.sel.disabled, true, 'the dropdown should be inert under Venue too');
+    assert.match(ctl.api.intens.title, /venue/i, 'reason should mention Venue');
+    // All three inert controls point at the reason under Venue (kills a
+    // 'describe reactive only' regression on the select/intensity paths).
+    const vReason = ctl.api.reason.id;
+    assert.equal(ctl.api.sel['aria-describedby'], vReason, 'select must reference the reason under Venue');
+    assert.equal(ctl.api.intens['aria-describedby'], vReason, 'intensity must reference the reason under Venue');
+    assert.equal(ctl.api.react['aria-describedby'], vReason, 'reactive must reference the reason under Venue');
+    assert.match(ctl.api.reason.textContent, /venue/i, 'the reason span carries the Venue text');
+
+    // The dropdown still shows the stored style (venue has no option), but
+    // selecting must not write while it's inert.
+    assert.equal(ctl.api.sel.value, 'particles');
+    const before = ctl.writes.length;
+    ctl.api.sel.value = 'lights';
+    ctl.api.sel.fire('change');
+    assert.equal(ctl.writes.length, before, 'a disabled dropdown must not write');
+
+    // Venue off: controls come back per the stored style.
+    ctl.sandbox._venueSceneOverride = false;
+    ctl.emit('venueScene');
+    assert.equal(ctl.api.intens.disabled, false, 'intensity re-enables when Venue exits');
+    assert.equal(ctl.api.react.disabled, false);
+    assert.equal(ctl.api.sel.disabled, false, 'the dropdown re-enables when Venue exits');
+    assert.equal(ctl.api.sel.title, 'Background style', 'the base tooltip must come back, not blank');
+    assert.equal(ctl.api.sel['aria-describedby'], undefined, 'select drops the reason off-Venue');
+    assert.equal(ctl.api.intens['aria-describedby'], undefined, 'intensity drops the reason off-Venue');
+    ctl.api._pcRelease();
+});
+
 test('an unknown style enables both controls (fails open)', () => {
     const { api, store, emit } = load();
     api._pcAcquire();