diff --git a/server/player/index.html b/server/player/index.html
index 88c0f0d..35b61fc 100644
--- a/server/player/index.html
+++ b/server/player/index.html
@@ -1676,11 +1676,12 @@
if (BS && BS.reboot()) console.log('[bs] reboot requested via host');
else console.log('reboot: not supported on this player');
}
- // Media volume, 0-100 from the dashboard. Applies to whatever is playing now and is
- // remembered for items mounted later.
+ // Media volume. Applies to whatever is playing now and is remembered for items mounted
+ // later (see setMediaVolume). The wire parsing is volumeLevelFromCommand().
if (data.type === 'set_volume') {
- const pct = Number(data.payload?.value ?? data.value);
- if (isFinite(pct)) setMediaVolume(Math.max(0, Math.min(100, pct)) / 100);
+ const v = volumeLevelFromCommand(data);
+ if (v === null) console.warn('[volume] set_volume with no usable level/value:', JSON.stringify(data));
+ else setMediaVolume(v);
}
});
@@ -4032,6 +4033,42 @@
try { return BS.displayPower(on); } catch (e) { return false; }
}
+ /*
+ * The 0..1 volume a set_volume command is asking for, or null when it does not carry one.
+ *
+ * The wire form is `payload.level`, a FRACTION. That is what the dashboard sends
+ * (device-detail.js: `{ level: slider / 100 }`), what a group command relays unchanged, and what
+ * the Android player reads (`optDouble("level")`) — so the fraction is canonical and this player
+ * now matches it instead of inventing a third convention.
+ *
+ * What was here read `payload.value` and treated it as a percentage, which matched nothing the
+ * product sends: the slider was a no-op on every browser panel. Correcting only the KEY would
+ * have been worse than leaving it broken — 0.5 clamped as a percentage is 0.5%, inaudible, and
+ * it would have looked fixed to anyone who checked only that the handler ran.
+ *
+ * `value` is still accepted as a 0..100 percentage for any caller written against the old
+ * handler. The scale is chosen by WHICH KEY ARRIVED, never by the magnitude of the number: 1 is
+ * legal in both scales (1% and full volume), so a magnitude test is guaranteed to be wrong for
+ * somebody, silently, in whichever direction hurts more.
+ *
+ * Extracted from the socket handler so the parsing can be asserted without a socket — the bug
+ * it replaces was invisible precisely because nothing ever ran it against a real payload.
+ */
+ function volumeLevelFromCommand(data) {
+ if (!data || typeof data !== 'object') return null;
+ const p = (data.payload && typeof data.payload === 'object') ? data.payload : {};
+ const pick = (k) => {
+ const raw = (p[k] !== undefined && p[k] !== null) ? p[k] : data[k];
+ if (raw === undefined || raw === null || raw === '' || typeof raw === 'boolean') return null;
+ const n = Number(raw);
+ return isFinite(n) ? n : null;
+ };
+ const level = pick('level'); // canonical: 0..1
+ const pct = level === null ? pick('value') : null; // legacy: 0..100
+ const v = level !== null ? level : (pct !== null ? pct / 100 : null);
+ return v === null ? null : Math.max(0, Math.min(1, v));
+ }
+
// Volume that survives the next item. Media elements are created per item, so remembering the
// level is what makes a volume command stick rather than lasting until the playlist advances.
let mediaVolume = null;
@@ -4040,8 +4077,17 @@
// Wall followers stay silent by design; don't override that.
try { if (typeof isWallFollower === 'function' && isWallFollower()) return; } catch (e) { /* not a wall */ }
document.querySelectorAll('video, audio').forEach((el) => {
- try { el.volume = v; el.muted = v === 0; } catch (e) { /* element torn down mid-call */ }
+ try { el.volume = v; } catch (e) { /* element torn down mid-call */ }
});
+ // Volume is a LEVEL. Mute is a separate decision with four inputs and a fixed order
+ // (lib/media-mute.js), already resolved when the element was mounted, and this function does
+ // not get to overrule it. It used to write `el.muted = (v === 0)`, which un-muted whatever was
+ // playing on any non-zero volume: an item an operator had deliberately silenced started making
+ // noise the moment somebody touched the volume slider — reproduced live (item flagged muted,
+ // one set_volume, muted went false). The same write contradicts the resolver's autoplay rule,
+ // where unmuting without a user gesture costs the VIDEO rather than winning the audio.
+ //
+ // Nothing is lost by leaving mute alone: volume 0 is silence on every media element we run on.
}
// Media elements are created per item across several code paths — fullscreen, zones, the
@@ -4059,7 +4105,10 @@
}
if (mediaVolume == null) return;
try { if (typeof isWallFollower === 'function' && isWallFollower()) return; } catch (err) { /* not a wall */ }
- try { el.volume = mediaVolume; el.muted = mediaVolume === 0; } catch (err) { /* gone */ }
+ // Level only — see setMediaVolume: the mount path has already resolved this element's mute
+ // from media-mute.js, and re-deciding it here from the volume number alone would undo a
+ // per-item mute (and, without a user gesture, the playback itself).
+ try { el.volume = mediaVolume; } catch (err) { /* gone */ }
}, true);
// Screen-off state, tracked because the playlist keeps advancing while the screen is "off"
diff --git a/server/player/sw.js b/server/player/sw.js
index c83c303..8e6b04f 100644
--- a/server/player/sw.js
+++ b/server/player/sw.js
@@ -1,6 +1,6 @@
// v24: an empty playlist payload no longer prunes. `assignments: []` is what the server sends for a
-// device between playlists AND for a snapshot that failed to parse, and treating it as "keep nothing"
-// wiped the panel's entire offline library on a message that means nothing of the sort.
+// device between playlists AND for a snapshot that failed to parse, and treating it as "keep
+// nothing" wiped the panel's entire offline library on a message that means nothing of the sort.
// v23: offline.cache is claimed only when a worker is actually IN CONTROL — a real BrightSign
// widget exposes navigator.serviceWorker, refuses to register one, and was advertising the
// capability to the fleet regardless.
@@ -54,11 +54,26 @@ self.addEventListener('fetch', (event) => {
const url = new URL(event.request.url);
// Widget renders pinned to a revision: cache-FIRST, because those exact bytes cannot change
- // without the rev changing. This is what lets a widget keep rendering when the network is gone —
- // previously the server sent no-store for every render, so widgets were the one thing the
- // player's offline cache could never hold, and a display that lost its uplink lost them.
- // ignoreSearch is deliberately NOT used here: the query string carries the rev, and ignoring it
- // would match a different revision's entry, which is the staleness we are trying to remove.
+ // without the rev changing. ignoreSearch is deliberately NOT used here: the query string carries
+ // the rev, and ignoring it would match a different revision's entry, which is the staleness we
+ // are trying to remove.
+ //
+ // MEASURED LIMIT, do not read more into this branch than it delivers. The player mounts widgets in
+ // an iframe sandboxed to `allow-scripts` with NO allow-same-origin (index.html,
+ // renderWidgetBuffered), so that frame is an OPAQUE-origin client — and a service worker does not
+ // control opaque-origin clients. Its navigation request never reaches this handler. Driven in
+ // Chrome against a live server: a clock widget mounted five times over 25s and the shell cache
+ // held zero widget entries, while a plain fetch() of the identical URL from the controlled page
+ // was intercepted and stored. So this branch serves anything that reaches it — a same-origin
+ // fetch, a future non-sandboxed mount — and today the player's own widgets are not that.
+ //
+ // What actually keeps widgets rendering offline right now is the HTTP cache plus the server's
+ // `max-age=31536000, immutable` on a rev-pinned render (routes/widgets.js). That is sound in a
+ // desktop browser and is NOT a documented-persistent store on BrightSign, which guarantees
+ // survival across reboots for IndexedDB, localStorage and SQLite only — the same gap that made
+ // content caching necessary. Closing it properly means routing the render through a same-origin
+ // fetch and mounting it as srcdoc; granting the frame allow-same-origin instead would hand widget
+ // scripts the player's origin, which is not a trade worth making for an offline nicety.
if (url.pathname.startsWith('/api/widgets/') && url.pathname.endsWith('/render') && url.searchParams.has('rev')) {
event.respondWith(
caches.match(event.request).then(cached => {
@@ -146,14 +161,16 @@ self.addEventListener('message', (event) => {
// writes a new randomly-named file, so the old copy lives at a different PATH and nothing keyed
// on the asset path can find it. Without this the cache only grows, and on a panel with a 1GB
// widget quota a handful of replaced videos is the entire budget.
+ //
// An EMPTY list is never a prune instruction, and that distinction is the whole guard. "This
- // display needs nothing" and "the payload did not arrive intact" are the same message on the wire,
- // and the second is not rare: buildPlaylistPayload() yields `assignments: []` for a device between
- // playlists, for a playlist never published, AND inside the catch when a published_snapshot fails
- // to JSON.parse. Honouring it deleted every byte of media the panel held — three cached assets,
- // one empty payload, cache emptied — which is only survivable while the uplink is up, i.e. exactly
- // when this cache does not matter. A cache kept too long costs disk the quota reclaims anyway; one
- // dropped at the wrong moment is a dark screen with no way back.
+ // display needs nothing" and "the payload did not arrive intact" are the same message on the
+ // wire, and the second one is not rare: buildPlaylistPayload() yields `assignments: []` for a
+ // device between playlists, for a playlist that has never been published, AND for a
+ // published_snapshot that fails to JSON.parse. Honouring it deleted every byte of media the panel
+ // held — reproduced here: three cached assets, one empty payload, cache emptied — which is only
+ // survivable while the uplink is up, i.e. exactly when this cache does not matter. A cache that is
+ // kept too long costs disk the quota reclaims anyway; one dropped at the wrong moment is a dark
+ // screen with no way back.
if (data.prune && data.urls.length > 0) {
prefetchChain = prefetchChain.then(() => pruneToPlaylist(data.urls)).catch(() => {});
}
diff --git a/server/test/player-sw-scope.test.js b/server/test/player-sw-scope.test.js
index aaa7f77..7dee238 100644
--- a/server/test/player-sw-scope.test.js
+++ b/server/test/player-sw-scope.test.js
@@ -70,12 +70,12 @@ test('the worker prunes to the set the player declares', () => {
// a panel with a 1GB widget quota, a few replaced videos is the whole budget.
const sw = fs.readFileSync(path.join(__dirname, '..', 'player', 'sw.js'), 'utf8');
assert.match(sw, /function pruneToPlaylist/);
- // The guard is `data.prune && data.urls.length > 0`, not a bare `data.prune`: an EMPTY list is
- // never a prune instruction. `assignments: []` is what the server sends for a device between
- // playlists and from the catch when a snapshot fails to parse, and honouring it as "keep nothing"
- // wiped a panel's whole offline library (fixed in 1.9.30).
+ // Driven by the player declaring a complete set — and an EMPTY set is not a declaration. See
+ // test/sw-prune-guard.test.js, which runs the handler: `assignments: []` is what the server sends
+ // for a device between playlists and for a snapshot that failed to parse, and honouring it as
+ // "keep nothing" wiped the panel's whole offline library.
assert.match(sw, /if \(data\.prune && data\.urls\.length > 0\)/,
- 'the prune must require a NON-EMPTY declared set');
+ 'the prune must be driven by the player declaring a NON-EMPTY complete set');
const html = fs.readFileSync(path.join(__dirname, '..', 'player', 'index.html'), 'utf8');
assert.match(html, /prune:\s*true/);
diff --git a/server/test/player-volume-command.test.js b/server/test/player-volume-command.test.js
new file mode 100644
index 0000000..00b683c
--- /dev/null
+++ b/server/test/player-volume-command.test.js
@@ -0,0 +1,182 @@
+'use strict';
+
+// The web player's set_volume handler, and the mute state it is not allowed to touch.
+//
+// Two bugs lived here at once, and the second is why fixing the first alone would have been worse
+// than leaving it broken:
+//
+// 1. THE KEY. The dashboard sends `{ level: <0..1> }` (device-detail.js: slider/100) and the
+// Android player reads exactly that (`optDouble("level")`). The web player read
+// `payload.value`. Nothing in the product sends `value`, so the slider was a silent no-op on
+// every browser panel — and a no-op is invisible: the command is delivered, acked, and does
+// nothing.
+// 2. THE SCALE. The local was named `pct` and divided by 100. Had only the key been corrected,
+// `level: 0.5` would have become 0.5% — near-silence that LOOKS like the fix worked, because
+// the handler now runs and the element's volume genuinely changes.
+//
+// And separately: setMediaVolume() wrote `el.muted = (v === 0)`, so any non-zero volume command
+// un-muted whatever was playing — defeating a per-item mute an operator had set on purpose
+// (reproduced in a browser: item flagged muted, one set_volume, muted went false), and defying the
+// autoplay rule in lib/media-mute.js, where unmuting without a gesture costs the video.
+//
+// The real functions are extracted from index.html and run against fake globals, so this asserts
+// what the shipped player does rather than a paraphrase of it.
+
+const { test } = require('node:test');
+const assert = require('node:assert/strict');
+const fs = require('node:fs');
+const path = require('node:path');
+const vm = require('node:vm');
+
+const HTML = fs.readFileSync(path.join(__dirname, '..', 'player', 'index.html'), 'utf8');
+
+/** Extract a top-level `function name(...) { ... }` from the player by brace matching. */
+function extract(name) {
+ const start = HTML.indexOf(` function ${name}(`);
+ assert.notEqual(start, -1, `${name}() must exist in the player`);
+ let depth = 0;
+ for (let i = HTML.indexOf('{', start); i < HTML.length; i++) {
+ if (HTML[i] === '{') depth++;
+ else if (HTML[i] === '}' && --depth === 0) return HTML.slice(start, i + 1);
+ }
+ throw new Error(`unterminated ${name}()`);
+}
+
+function loadVolumeParser() {
+ const sandbox = { console: { log() {}, warn() {} } };
+ vm.createContext(sandbox);
+ vm.runInContext(extract('volumeLevelFromCommand'), sandbox);
+ return (data) => vm.runInContext('volumeLevelFromCommand', sandbox)(data);
+}
+
+/**
+ * setMediaVolume() against fake media elements. Returns the elements so the test can see exactly
+ * which properties were written.
+ */
+function loadSetVolume({ wallFollower = false, elements } = {}) {
+ const els = elements || [
+ { tagName: 'VIDEO', volume: 1, muted: false },
+ { tagName: 'VIDEO', volume: 1, muted: true },
+ ];
+ const sandbox = {
+ console: { log() {}, warn() {} },
+ document: { querySelectorAll: () => els },
+ isWallFollower: () => wallFollower,
+ mediaVolume: null,
+ };
+ sandbox.window = sandbox;
+ vm.createContext(sandbox);
+ vm.runInContext('let mediaVolume = null;\n' + extract('setMediaVolume'), sandbox);
+ return { els, call: (v) => vm.runInContext('setMediaVolume', sandbox)(v), sandbox };
+}
+
+// ---------------------------------------------------------------------------------------------
+// THE WIRE FORM
+
+test('THE BUG: the key the dashboard actually sends is honoured', () => {
+ const parse = loadVolumeParser();
+ assert.equal(parse({ type: 'set_volume', payload: { level: 0.5 } }), 0.5);
+});
+
+test('THE OTHER HALF: a fraction is not divided by 100', () => {
+ // The trap. 0.5 -> 0.005 is near-mute and would read as "fixed" to anyone checking that the
+ // handler fires at all.
+ const parse = loadVolumeParser();
+ assert.equal(parse({ type: 'set_volume', payload: { level: 0.5 } }), 0.5);
+ assert.equal(parse({ type: 'set_volume', payload: { level: 1 } }), 1);
+ assert.equal(parse({ type: 'set_volume', payload: { level: 0 } }), 0);
+});
+
+test('the legacy percentage key is still understood, as a percentage', () => {
+ const parse = loadVolumeParser();
+ assert.equal(parse({ type: 'set_volume', payload: { value: 80 } }), 0.8);
+ assert.equal(parse({ type: 'set_volume', value: 25 }), 0.25); // top-level, as the old code read it
+});
+
+test('the scale comes from the KEY, never from the magnitude', () => {
+ // 1 is legal in both conventions — full volume as a fraction, 1% as a percentage. A magnitude
+ // heuristic has to be wrong for one of them, so there is no heuristic.
+ const parse = loadVolumeParser();
+ assert.equal(parse({ payload: { level: 1 } }), 1, 'level:1 is FULL volume');
+ assert.equal(parse({ payload: { value: 1 } }), 0.01, 'value:1 is one percent');
+});
+
+test('level wins when a caller sends both', () => {
+ const parse = loadVolumeParser();
+ assert.equal(parse({ payload: { level: 0.3, value: 90 } }), 0.3);
+});
+
+test('out-of-range input is clamped, not wrapped', () => {
+ const parse = loadVolumeParser();
+ assert.equal(parse({ payload: { level: 4 } }), 1);
+ assert.equal(parse({ payload: { level: -2 } }), 0);
+ assert.equal(parse({ payload: { value: 5000 } }), 1);
+});
+
+test('a command carrying no level is refused rather than turned into 0', () => {
+ // Returning 0 for "no value" would silence a display on a malformed command — a failure that
+ // looks exactly like someone dragging the slider down.
+ const parse = loadVolumeParser();
+ for (const bad of [{}, { payload: {} }, { payload: { level: null } }, { payload: { level: '' } },
+ { payload: { level: 'loud' } }, { payload: { level: true } }, null, undefined]) {
+ assert.equal(parse(bad), null, `expected null for ${JSON.stringify(bad)}`);
+ }
+});
+
+test('numeric strings are accepted — an integrator posting JSON as text still works', () => {
+ const parse = loadVolumeParser();
+ assert.equal(parse({ payload: { level: '0.4' } }), 0.4);
+ assert.equal(parse({ payload: { value: '70' } }), 0.7);
+});
+
+// ---------------------------------------------------------------------------------------------
+// WHAT IT IS NOT ALLOWED TO TOUCH
+
+test('THE BUG: a volume command does not un-mute a deliberately muted item', () => {
+ const { els, call } = loadSetVolume();
+ els[1].muted = true; // an operator silenced this item
+ call(0.6);
+ assert.equal(els[1].volume, 0.6, 'the level still applies');
+ assert.equal(els[1].muted, true, 'a muted element must stay muted');
+ assert.equal(els[0].muted, false, 'and an unmuted one must stay unmuted');
+});
+
+test('volume 0 does not need to set muted — the level alone is silence', () => {
+ const { els, call } = loadSetVolume();
+ call(0);
+ assert.equal(els[0].volume, 0);
+ assert.equal(els[0].muted, false, 'mute is not this function\'s decision to make');
+});
+
+test('a wall follower is left silent — volume never reaches its elements', () => {
+ // One audio source per wall. A follower that honoured the slider would give the room the same
+ // track from every panel, a few milliseconds apart.
+ const { els, call } = loadSetVolume({ wallFollower: true });
+ call(0.9);
+ assert.equal(els[0].volume, 1, 'untouched');
+});
+
+test('the level is remembered so it survives the next item', () => {
+ const { call, sandbox } = loadSetVolume();
+ call(0.35);
+ assert.equal(vm.runInContext('mediaVolume', sandbox), 0.35);
+});
+
+test('an element torn down mid-call cannot break the loop', () => {
+ const boom = { tagName: 'VIDEO', set volume(v) { throw new Error('detached'); }, muted: false };
+ const good = { tagName: 'VIDEO', volume: 1, muted: false };
+ const { call } = loadSetVolume({ elements: [boom, good] });
+ call(0.5);
+ assert.equal(good.volume, 0.5, 'the surviving element still gets the level');
+});
+
+// ---------------------------------------------------------------------------------------------
+// THE HANDLER WIRING — the parser is useless if the socket handler does not call it
+
+test('the set_volume handler routes through the parser and setMediaVolume', () => {
+ const handler = HTML.slice(HTML.indexOf("if (data.type === 'set_volume')"));
+ const block = handler.slice(0, handler.indexOf('\n }') + 10);
+ assert.match(block, /volumeLevelFromCommand\(data\)/, 'the handler must use the shared parser');
+ assert.match(block, /setMediaVolume\(/, 'and apply it');
+ assert.ok(!/\/\s*100/.test(block), 'the handler must not re-scale — the parser already returns 0..1');
+});
diff --git a/server/test/player-widget-frame-origin.test.js b/server/test/player-widget-frame-origin.test.js
new file mode 100644
index 0000000..fd71dac
--- /dev/null
+++ b/server/test/player-widget-frame-origin.test.js
@@ -0,0 +1,56 @@
+'use strict';
+
+// The widget iframe's origin, and what it costs.
+//
+// The player mounts a widget in an iframe sandboxed to `allow-scripts` with NO allow-same-origin,
+// which gives it an OPAQUE origin: widget scripts cannot read the player's window, its localStorage,
+// or its device token. That is deliberate and worth keeping — widget HTML is operator-authored and
+//, for the webpage widget, third-party.
+//
+// It has a consequence that is easy to forget and was in fact forgotten: a service worker does not
+// control opaque-origin clients, so the widget frame's navigation NEVER reaches sw.js. The
+// cache-first widget branch there is real code that this player's own widgets do not use — measured
+// in Chrome, a clock widget mounted five times over 25 seconds while the shell cache held zero
+// widget entries, and a plain fetch() of the same URL from the controlled page was cached
+// immediately. Widgets survive an outage today on the HTTP cache and the server's immutable
+// Cache-Control, not on the worker.
+//
+// So this test pins the security property, and pins the fact that the offline story for widgets
+// rests on the HTTP header. Someone who "fixes" offline widgets by adding allow-same-origin trades
+// the isolation for a cache — the wrong direction, and the reason this is asserted rather than left
+// as a comment.
+
+const { test } = require('node:test');
+const assert = require('node:assert/strict');
+const fs = require('node:fs');
+const path = require('node:path');
+
+const HTML = fs.readFileSync(path.join(__dirname, '..', 'player', 'index.html'), 'utf8');
+const SW = fs.readFileSync(path.join(__dirname, '..', 'player', 'sw.js'), 'utf8');
+const WIDGETS = fs.readFileSync(path.join(__dirname, '..', 'routes', 'widgets.js'), 'utf8');
+
+test('the widget iframe is sandboxed into an opaque origin', () => {
+ const sandboxes = [...HTML.matchAll(/setAttribute\('sandbox',\s*'([^']*)'\)/g)].map((m) => m[1]);
+ assert.ok(sandboxes.length > 0, 'the player must sandbox its widget frames');
+ for (const s of sandboxes) {
+ assert.match(s, /allow-scripts/, 'a widget needs scripts to be a widget');
+ assert.doesNotMatch(s, /allow-same-origin/,
+ 'allow-same-origin would give widget scripts the player origin — its storage, its device token');
+ }
+});
+
+test('the offline guarantee for widgets is the HTTP header, and the server still sets it', () => {
+ // If this regresses to no-store, widgets stop surviving an outage everywhere — and the service
+ // worker will NOT quietly cover for it, because it never sees the request.
+ assert.match(WIDGETS, /max-age=31536000, immutable/,
+ 'a rev-pinned render must stay hard-cacheable: it is the only thing holding widgets offline');
+ assert.match(WIDGETS, /no-store/, 'a render with no rev must stay uncacheable — nothing distinguishes one from the next');
+});
+
+test('the worker does not claim to be what keeps widgets offline', () => {
+ // The comment above that branch used to say it was. A worker cannot control an opaque-origin
+ // client, so the claim was false in exactly the deployment it was written for.
+ const branch = SW.slice(0, SW.indexOf("url.pathname.startsWith('/api/widgets/')"));
+ assert.match(branch, /opaque-origin|opaque origin/i,
+ 'sw.js must record that the widget frame is opaque-origin and bypasses it');
+});