From f1b3007cc7ef7ad55b5bace1ec7ed647daecfb50 Mon Sep 17 00:00:00 2001 From: Kyle Date: Sun, 19 Jul 2026 19:03:36 -0400 Subject: [PATCH 1/6] 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. --- plugins/highway_3d/plugin.json | 2 +- plugins/highway_3d/screen.js | 41 +++++++++++-- .../tests/background_control.test.js | 59 +++++++++++++++++++ 3 files changed, 95 insertions(+), 7 deletions(-) diff --git a/plugins/highway_3d/plugin.json b/plugins/highway_3d/plugin.json index caa52fc9..755ed1be 100644 --- a/plugins/highway_3d/plugin.json +++ b/plugins/highway_3d/plugin.json @@ -1,7 +1,7 @@ { "id": "highway_3d", "name": "3D Highway", - "version": "3.34.0", + "version": "3.34.1", "type": "visualization", "bundled": true, "script": "screen.js", diff --git a/plugins/highway_3d/screen.js b/plugins/highway_3d/screen.js index c4929fe9..9783ca7c 100644 --- a/plugins/highway_3d/screen.js +++ b/plugins/highway_3d/screen.js @@ -4063,6 +4063,10 @@ image: { intensity: true, reactive: false, why: 'This background does not react to audio' }, video: { intensity: false, reactive: false, why: 'The video plays as-is - nothing to adjust here' }, butterchurn: { intensity: false, reactive: false, why: 'Butterchurn reacts to audio itself - tune it in Settings > 3D Highway, or its Visualizer panel' }, + // Not in BG_STYLE_IDS, so it never appears in the dropdown - reached + // only via the viz-picker Venue flow (h3dVenueSceneSetActive). While + // active it is the EFFECTIVE style, so both knobs drive nothing. + venue: { intensity: false, reactive: false, why: 'Venue visualization is active - pick a background from the visualization picker' }, }; let _pcRefs = 0, _pcEl = null, _pcSel = null, _pcReactive = null, _pcIntensity = null; // Non-disabled wrappers around the two greyable controls. A native-disabled @@ -4151,6 +4155,18 @@ // whenever the settings bus reports one of our keys changed, so editing // from the Settings page updates this control and vice-versa. function _pcSync() { + // The active style is the EFFECTIVE one, not the stored one: while the + // Venue scene override is on it is what's mounted, and it ignores the + // whole Background group - picking a style writes `style` but + // _bgMountStyle resolves back to venue, so the dropdown would look + // broken. So under Venue the ENTIRE group goes inert (dropdown too), + // and the user exits Venue from the visualization picker where they + // entered it. An unknown id enables everything rather than disabling + // it, so a style added without a _PC_USES row is merely unhelpful. + const venue = !!_venueSceneOverride; + const effectiveStyle = venue ? 'venue' : _bgReadSetting(null, 'style'); + const uses = _PC_USES[effectiveStyle] || { intensity: true, reactive: true }; + const why = uses.why || 'This background style ignores this setting'; if (_pcSel) { // The custom slots stay unselectable until something is uploaded - // same rule settings.html applies. @@ -4159,12 +4175,14 @@ if (img) img.disabled = !_bgReadSetting(null, 'customImageDataUrl'); if (vid) vid.disabled = !_bgReadSetting(null, 'customVideoName'); _pcSel.value = _bgReadSetting(null, 'style'); + // The dropdown still SHOWS the stored style (venue has no option), + // but it's inert while Venue owns the scene. + _pcSel.disabled = venue; + _pcSel.setAttribute('aria-disabled', venue ? 'true' : 'false'); + _pcSel.style.opacity = venue ? '.45' : '1'; + _pcSel.style.cursor = venue ? 'not-allowed' : ''; + _pcSel.title = venue ? why : ''; } - // Grey out whichever controls the ACTIVE style ignores (see _PC_USES). - // An unknown id enables both rather than disabling both, so a style - // added without a table row is merely unhelpful, never inert. - const uses = _PC_USES[_bgReadSetting(null, 'style')] || { intensity: true, reactive: true }; - const why = uses.why || 'This background style ignores this setting'; if (_pcReactive) { _pcPaint(_pcReactive, !!_bgReadSetting(null, 'reactive'), !uses.reactive, uses.reactive ? 'React to the audio' : why); @@ -4244,6 +4262,7 @@ _pcSel.appendChild(o); } _pcSel.addEventListener('change', () => { + if (_pcSel.disabled) return; // inert under the Venue override try { window.h3dBgSetStyle(_pcSel.value); } catch (e) { console.error('[3D-Hwy] bg style set failed', e); } }); @@ -4288,7 +4307,11 @@ _pcSync(); _pcListener = (key) => { if (key === 'style' || key === 'reactive' || key === 'intensity' - || key === 'customImageDataUrl' || key === 'customVideoName') { + || key === 'customImageDataUrl' || key === 'customVideoName' + || key === 'venueScene') { + // 'venueScene' has no dropdown/settings widget of its own, but + // toggling Venue changes the EFFECTIVE style, so the greying + // must re-evaluate (see _pcSync's effectiveStyle). _pcSync(); _pcSyncSettingsPanel(); } @@ -4313,6 +4336,12 @@ const tick = () => { _pcRetryTimer = 0; if (_pcRefs <= 0) return; // renderer went away mid-retry + // Re-attempt the bus subscription too, not just the mount. On a cold + // load the renderer can init before window.feedBack.on exists; the + // first _pcBindScreenHook() then no-ops and, without this, the hook + // never binds and the control goes permanently deaf to screen + // changes. Idempotent via the _pcScreenHook guard. + _pcBindScreenHook(); if (_pcMount()) return; if (++_pcRetry > 12) return; // ~3s at 250ms _pcRetryTimer = setTimeout(tick, 250); diff --git a/plugins/highway_3d/tests/background_control.test.js b/plugins/highway_3d/tests/background_control.test.js index 8d04c1bf..05272700 100644 --- a/plugins/highway_3d/tests/background_control.test.js +++ b/plugins/highway_3d/tests/background_control.test.js @@ -127,6 +127,10 @@ 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], _bgSubscribe: (fn) => listeners.add(fn), _bgUnsubscribe: (fn) => listeners.delete(fn), @@ -199,6 +203,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(); @@ -311,6 +338,38 @@ 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'); + + // 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'); + ctl.api._pcRelease(); +}); + test('an unknown style enables both controls (fails open)', () => { const { api, store, emit } = load(); api._pcAcquire(); From 75dd2a2da423460d08c5588be5fbf370d251dfea Mon Sep 17 00:00:00 2001 From: Kyle Date: Sun, 19 Jul 2026 19:43:18 -0400 Subject: [PATCH 2/6] 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. --- plugins/highway_3d/screen.js | 77 ++++++++++--- .../tests/background_control.test.js | 104 +++++++++++++++++- 2 files changed, 163 insertions(+), 18 deletions(-) diff --git a/plugins/highway_3d/screen.js b/plugins/highway_3d/screen.js index 9783ca7c..b18f22d0 100644 --- a/plugins/highway_3d/screen.js +++ b/plugins/highway_3d/screen.js @@ -2786,6 +2786,21 @@ if (globalVal !== null && globalVal !== undefined) return _bgCoerce(key, globalVal); return BG_DEFAULTS[key]; } + // Read a setting's GLOBAL value, ignoring any per-panel override. The + // player-chrome control is a single shared instance, so it must always + // read (and write) the global slot. Passing null as a panelKey to + // _bgReadSetting happened to work only because 'h3d_bg_null_' never + // exists; this states the intent directly and can't be shadowed if a + // panelKey of null is ever used deliberately. Mirrors the global half of + // _bgReadSetting exactly (mem-fallback precedence, then persisted, then + // default). + function _bgReadGlobal(key) { + let globalVal = null; + try { globalVal = localStorage.getItem('h3d_bg_' + key); } catch (_) { /* storage blocked */ } + if (key in _bgMemFallback) return _bgCoerce(key, _bgMemFallback[key]); + if (globalVal !== null && globalVal !== undefined) return _bgCoerce(key, globalVal); + return BG_DEFAULTS[key]; + } // Shared "stored string -> bool" coercion for every boolean // setting. Mirrors settings.html's coerceBool so the renderer and // the UI hydration always agree on what a corrupted/unknown value @@ -4024,8 +4039,10 @@ * add a style there and it shows up in both places automatically. * * MOUNTED ONCE, REFCOUNTED. Under splitscreen there are N renderer - * instances but these settings are global, so N copies of the control - * would be N ways to set one value. init() acquires, destroy() releases, + * instances but these settings are global — a panel may set a per-panel + * override, but this single shared control only ever reads/writes the + * global slot (via _bgReadGlobal), so N copies would be N ways to set + * one value. init() acquires, destroy() releases, * and the last release unmounts — so the control disappears when the user * switches to a non-3D renderer instead of lingering as a dead knob. * @@ -4046,9 +4063,11 @@ // // Derived by reading the BG_STYLES bodies: a style uses `intensity` if its // build() reads settings.intensity, and uses `reactive` if its update() - // dereferences the `bands` argument. 'butterchurn' is not a BG_STYLES entry - // at all - _bgMountStyle falls through to BG_STYLES.off - and it drives its - // own audio tap and opacity, so both are false for it. + // dereferences the `bands` argument. 'butterchurn' is a mode, not a + // BG_STYLES fog-scenery entry: _bcSyncMode owns its controller, which + // drives its own audio tap and canvas opacity (only the fog-scenery half + // falls through to BG_STYLES.off). So neither knob here reaches it - both + // are false, and the tooltip points at Butterchurn's own controls. // // KEEP IN STEP WITH BG_STYLES. If a style starts reading bands or intensity // and its row is not updated, the control stays greyed out and lies the @@ -4074,7 +4093,7 @@ // shows on hover — the whole "greyed out, says why on hover" affordance // would be dead. The reason lives on these wrappers instead, and the // disabled control gets pointer-events:none so the hover reaches them. - let _pcReactiveWrap = null, _pcIntensityWrap = null; + let _pcReactiveWrap = null, _pcIntensityWrap = null, _pcReason = null; let _pcListener = null, _pcRetry = 0, _pcRetryTimer = 0; // The player chrome exposes this slot once it has initialised. A host @@ -4130,6 +4149,8 @@ btn._on = !!on && !disabled; btn.disabled = !!disabled; btn.setAttribute('aria-disabled', disabled ? 'true' : 'false'); + // A toggle button must expose its state, not just its label. + btn.setAttribute('aria-pressed', btn._on ? 'true' : 'false'); // pointer-events:none lets the hover fall through to _pcReactiveWrap, // which carries the reason a disabled button's own title can't show. btn.style.pointerEvents = disabled ? 'none' : ''; @@ -4164,17 +4185,29 @@ // entered it. An unknown id enables everything rather than disabling // it, so a style added without a _PC_USES row is merely unhelpful. const venue = !!_venueSceneOverride; - const effectiveStyle = venue ? 'venue' : _bgReadSetting(null, 'style'); + const effectiveStyle = venue ? 'venue' : _bgReadGlobal('style'); const uses = _PC_USES[effectiveStyle] || { intensity: true, reactive: true }; const why = uses.why || 'This background style ignores this setting'; + if (_pcReason) _pcReason.textContent = why; + // Point a screen reader at the reason, but only while a control is + // inert - cleared otherwise so an enabled control is not described by a + // stale reason. + const _pcDescribe = (el, inert) => { + if (!el) return; + if (inert) el.setAttribute('aria-describedby', 'h3d-pc-reason'); + else el.removeAttribute('aria-describedby'); + }; + _pcDescribe(_pcSel, venue); + _pcDescribe(_pcReactive, !uses.reactive); + _pcDescribe(_pcIntensity, !uses.intensity); if (_pcSel) { // The custom slots stay unselectable until something is uploaded - // same rule settings.html applies. const img = _pcSel.querySelector('option[value="image"]'); const vid = _pcSel.querySelector('option[value="video"]'); - if (img) img.disabled = !_bgReadSetting(null, 'customImageDataUrl'); - if (vid) vid.disabled = !_bgReadSetting(null, 'customVideoName'); - _pcSel.value = _bgReadSetting(null, 'style'); + if (img) img.disabled = !_bgReadGlobal('customImageDataUrl'); + if (vid) vid.disabled = !_bgReadGlobal('customVideoName'); + _pcSel.value = _bgReadGlobal('style'); // The dropdown still SHOWS the stored style (venue has no option), // but it's inert while Venue owns the scene. _pcSel.disabled = venue; @@ -4184,7 +4217,7 @@ _pcSel.title = venue ? why : ''; } if (_pcReactive) { - _pcPaint(_pcReactive, !!_bgReadSetting(null, 'reactive'), !uses.reactive, + _pcPaint(_pcReactive, !!_bgReadGlobal('reactive'), !uses.reactive, uses.reactive ? 'React to the audio' : why); } // The reason shows via the wrapper (see _pcReactiveWrap); empty when @@ -4194,7 +4227,7 @@ _pcReactiveWrap.style.cursor = uses.reactive ? '' : 'not-allowed'; } if (_pcIntensity) { - _pcIntensity.value = String(_bgReadSetting(null, 'intensity')); + _pcIntensity.value = String(_bgReadGlobal('intensity')); _pcIntensity.disabled = !uses.intensity; _pcIntensity.setAttribute('aria-disabled', uses.intensity ? 'false' : 'true'); _pcIntensity.style.pointerEvents = uses.intensity ? '' : 'none'; @@ -4221,10 +4254,10 @@ function _pcSyncSettingsPanel() { try { const st = document.getElementById('h3d-bg-style'); - if (st) st.value = _bgReadSetting(null, 'style'); + if (st) st.value = _bgReadGlobal('style'); const re = document.getElementById('h3d-bg-reactive'); - if (re) re.checked = !!_bgReadSetting(null, 'reactive'); - const inten = _bgReadSetting(null, 'intensity'); + if (re) re.checked = !!_bgReadGlobal('reactive'); + const inten = _bgReadGlobal('intensity'); const ie = document.getElementById('h3d-bg-intensity'); if (ie) ie.value = String(inten); // The panel prints the numeric value beside the slider; keep its @@ -4244,6 +4277,16 @@ const box = document.createElement('div'); box.className = 'h3d-pc'; box.style.cssText = 'display:flex;flex-direction:column;width:100%;'; + // Visually-hidden text carrying the "why greyed out" reason to screen + // readers; disabled controls point aria-describedby here. A title alone + // is announced unreliably and never on touch. One span suffices - every + // greyed control shares the same reason (derived from the single + // effective style). + _pcReason = document.createElement('span'); + _pcReason.id = 'h3d-pc-reason'; + _pcReason.style.cssText = 'position:absolute;width:1px;height:1px;padding:0;' + + 'margin:-1px;overflow:hidden;clip:rect(0 0 0 0);white-space:nowrap;border:0;'; + box.appendChild(_pcReason); box.appendChild(_pcGroupLabel('Background')); // A dropdown, not pills: the style list is 8 entries and growing, and @@ -4252,6 +4295,7 @@ // raw