From 26c72d62bf16a42e5ac7357466672f0479a21f97 Mon Sep 17 00:00:00 2001
From: ScreenTinker <hello@screentinker.com>
Date: Wed, 1 Jul 2026 21:51:52 -0500
Subject: [PATCH] =?UTF-8?q?fix(#146):=20web=20player=20=E2=80=94=20no-chan?=
 =?UTF-8?q?ge=20refresh=20loses=20video=20(keeps=20audio);=20re-attach=20i?=
 =?UTF-8?q?dempotently?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

ROOT CAUSE (hypothesis A, pre-existing — NOT a beta7 regression; server/player/index.html
is untouched since v1.9.2-beta6): handlePlaylistUpdate's "Playlist unchanged" branch blindly
returned. The media re-attach (renderContent) lives ONLY in the content-changed branch, so
if the <video> surface was lost (element detached from the DOM while still decoding — video
gone, audio still playing) a no-new-content refresh never re-attached it. New-content
refreshes were fine because they re-render.

FIX (make the refresh idempotent for the media surface, no flicker on the healthy path):
- server/lib/player-media-health.js (new, UMD + unit-testable, mirrors schedule-eval.js):
  needsReattach(state) — re-attach ONLY when playback should be happening but the current
  item's surface is actually lost (video null / detached / ended / errored; non-video: no
  mounted surface). A healthy attached+live video returns false, so a routine poll stays a
  no-op (no re-render, no flicker). Served at /player/player-media-health.js from the single
  source; loaded by the player.
- index.html no-change branch: extract the current item's DOM facts and, iff
  PlayerMediaHealth.needsReattach, call playCurrentItem() to re-render the current item.
  Wrapped so the health check can never break a refresh.
- teardownCurrentMedia: also release currentVideoEl even when it was DETACHED from the
  container — a detached-but-playing <video> keeps emitting audio and the container-scoped
  querySelectorAll can't find it. This kills the "ghost audio" on re-attach.
- sw.js cache bumped v9 -> v10 so players pick up the new index.html + module.

Tests: test/player-media-health.test.js (6) exercises the branch selection — healthy video
-> no re-attach; detached/null/ended/errored -> re-attach; idle -> never; non-video by
surface presence. Inline player JS syntax-checked; module served + referenced verified on a
booted server. Suite 316/316.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 server/lib/player-media-health.js       | 48 +++++++++++++++++++++++++
 server/player/index.html                | 39 ++++++++++++++++++++
 server/player/sw.js                     |  2 +-
 server/server.js                        |  7 ++++
 server/test/player-media-health.test.js | 42 ++++++++++++++++++++++
 5 files changed, 137 insertions(+), 1 deletion(-)
 create mode 100644 server/lib/player-media-health.js
 create mode 100644 server/test/player-media-health.test.js

diff --git a/server/lib/player-media-health.js b/server/lib/player-media-health.js
new file mode 100644
index 0000000..f460780
--- /dev/null
+++ b/server/lib/player-media-health.js
@@ -0,0 +1,48 @@
+// Player media-surface health decision (#146 web-player fix).
+//
+// THE BUG (hypothesis A): a NO-NEW-CONTENT refresh in handlePlaylistUpdate returned early
+// ("Playlist unchanged") without verifying the media surface is still attached. If the
+// <video> element had been detached from the DOM while still decoding (audio keeps playing,
+// video surface gone), the re-attach — which lived ONLY in the content-changed branch —
+// never ran, so the video never came back. This module is the branch-selection decision the
+// no-change path now consults: re-attach ONLY when playback should be happening but the
+// surface is actually lost, so a healthy poll stays a no-op (no flicker every refresh).
+//
+// Pure + dependency-free so it is unit-testable without a DOM: the caller extracts the DOM
+// facts (is the <video> in the document? ended? errored?) into a plain state object.
+//
+// Dependency-free UMD: Node (require) + browser/Tizen (window.PlayerMediaHealth).
+(function (root, factory) {
+  if (typeof module === 'object' && module.exports) module.exports = factory();
+  else root.PlayerMediaHealth = factory();
+})(typeof self !== 'undefined' ? self : this, function () {
+  'use strict';
+
+  // state = {
+  //   isPlaying:      boolean  // the player believes an item is playing
+  //   hasCurrentItem: boolean  // playlist[currentIndex] exists
+  //   itemKind:       'video' | 'youtube' | 'image' | 'widget' | 'other'
+  //   videoEl:        { attached, ended, errored } | null   // for a plain <video> item
+  //   surfaceAttached:boolean  // for non-video: a rendered surface is present in the DOM
+  // }
+  // Returns true iff the no-change refresh must re-render/re-attach the current item.
+  function needsReattach(state) {
+    var s = state || {};
+    // Idle or no content: nothing to re-attach — leave the idle/waiting screen alone.
+    if (!s.isPlaying || !s.hasCurrentItem) return false;
+
+    if (s.itemKind === 'video') {
+      // The exact bug: a <video> that is gone or detached from the DOM (its element may
+      // still be emitting audio) — or one that ended/errored — must be re-attached.
+      if (!s.videoEl) return true;
+      if (!s.videoEl.attached) return true;
+      if (s.videoEl.ended || s.videoEl.errored) return true;
+      return false; // attached + live -> healthy, do NOT re-render (avoids flicker)
+    }
+
+    // Non-video surfaces (image / youtube iframe / widget): healthy iff a surface is mounted.
+    return !s.surfaceAttached;
+  }
+
+  return { needsReattach: needsReattach };
+});
diff --git a/server/player/index.html b/server/player/index.html
index de236ff..d95cd08 100644
--- a/server/player/index.html
+++ b/server/player/index.html
@@ -232,6 +232,7 @@
 
   <script src="/socket.io/socket.io.js"></script>
   <script src="/player/schedule-eval.js"></script>
+  <script src="/player/player-media-health.js"></script>
   <script>
     // ==================== i18n ====================
     // Lightweight inline i18n for the player. The player is a standalone page
@@ -1307,6 +1308,33 @@
 
       if (newFp === oldFp && playlist.length > 0 && !wallChanged) {
         console.log('Playlist unchanged');
+        // #146 fix: a no-change refresh used to blindly return — so if the <video> surface
+        // had been lost (detached from the DOM while its element kept decoding audio: video
+        // gone, audio still playing), the re-attach (which lives ONLY in the content-changed
+        // branch below) never ran. Re-render the CURRENT item, but ONLY when the surface is
+        // actually unhealthy, so a healthy poll stays a no-op (no flicker on every refresh).
+        try {
+          const item = playlist[currentIndex];
+          const kind = !item ? 'other'
+            : item.widget_id ? 'widget'
+            : item.mime_type === 'video/youtube' ? 'youtube'
+            : (typeof item.mime_type === 'string' && item.mime_type.startsWith('video/')) ? 'video'
+            : (typeof item.mime_type === 'string' && item.mime_type.startsWith('image/')) ? 'image' : 'other';
+          const container = document.getElementById('playerContainer');
+          const state = {
+            isPlaying: isPlaying,
+            hasCurrentItem: !!item,
+            itemKind: kind,
+            videoEl: currentVideoEl
+              ? { attached: document.contains(currentVideoEl), ended: !!currentVideoEl.ended, errored: !!currentVideoEl.error }
+              : null,
+            surfaceAttached: !!(container && container.querySelector('video,img,iframe,.wall-stage')),
+          };
+          if (window.PlayerMediaHealth && PlayerMediaHealth.needsReattach(state)) {
+            console.log('[refresh] media surface lost on no-change refresh — re-attaching current item');
+            playCurrentItem();
+          }
+        } catch (e) { /* never let the health check break a refresh */ }
         return;
       }
 
@@ -1607,6 +1635,17 @@
         });
         container.innerHTML = '';
       }
+      // #146 fix: also release currentVideoEl even if it was DETACHED from the container —
+      // a detached-but-playing <video> keeps emitting audio and the querySelectorAll above
+      // (scoped to the container) can't find it. This is what kills the "ghost audio".
+      if (currentVideoEl) {
+        try {
+          currentVideoEl.onended = null; currentVideoEl.onerror = null; currentVideoEl.onloadeddata = null;
+          currentVideoEl.pause();
+          currentVideoEl.removeAttribute('src');
+          currentVideoEl.load();
+        } catch (e) { /* element may already be gone */ }
+      }
       currentVideoEl = null;
     }
 
diff --git a/server/player/sw.js b/server/player/sw.js
index f4899d6..08a6af9 100644
--- a/server/player/sw.js
+++ b/server/player/sw.js
@@ -1,4 +1,4 @@
-const CACHE_NAME = 'rd-player-v9';
+const CACHE_NAME = 'rd-player-v10';
 
 // Install: skip waiting to activate immediately
 self.addEventListener('install', (event) => {
diff --git a/server/server.js b/server/server.js
index bc4e094..7eaf1c2 100644
--- a/server/server.js
+++ b/server/server.js
@@ -276,6 +276,13 @@ app.get('/player/schedule-eval.js', (req, res) => {
   res.sendFile(path.join(__dirname, 'lib', 'schedule-eval.js'));
 });
 
+// #146 web-player fix: serve the media-surface health decision from its single source
+// (server/lib/player-media-health.js) so the player and the Node test can't drift.
+app.get('/player/player-media-health.js', (req, res) => {
+  res.type('application/javascript').setHeader('Cache-Control', 'no-cache');
+  res.sendFile(path.join(__dirname, 'lib', 'player-media-health.js'));
+});
+
 // Serve web player at /player (same no-cache for JS/HTML). The index.html
 // route above intercepts the HTML requests; everything else still falls
 // through to this static handler (debug-overlay.js, sw.js, manifest, etc).
diff --git a/server/test/player-media-health.test.js b/server/test/player-media-health.test.js
new file mode 100644
index 0000000..a1fb825
--- /dev/null
+++ b/server/test/player-media-health.test.js
@@ -0,0 +1,42 @@
+'use strict';
+
+// #146 web-player fix — the no-change-refresh branch-selection decision. This is the unit
+// the refresh handler consults to decide whether to re-attach the media surface. It is the
+// smallest testable piece of the "no new content lost the video but not the audio" fix.
+
+const { test } = require('node:test');
+const assert = require('node:assert/strict');
+const { needsReattach } = require('../lib/player-media-health');
+
+const video = (o) => ({ isPlaying: true, hasCurrentItem: true, itemKind: 'video', videoEl: o, surfaceAttached: true });
+
+test('healthy attached+live video on a no-change refresh -> NO re-attach (no flicker)', () => {
+  assert.equal(needsReattach(video({ attached: true, ended: false, errored: false })), false);
+});
+
+test('THE BUG: a detached-but-playing <video> (audio persists, surface gone) -> re-attach', () => {
+  assert.equal(needsReattach(video({ attached: false, ended: false, errored: false })), true);
+});
+
+test('video element gone entirely -> re-attach', () => {
+  assert.equal(needsReattach({ isPlaying: true, hasCurrentItem: true, itemKind: 'video', videoEl: null }), true);
+});
+
+test('ended or errored video -> re-attach', () => {
+  assert.equal(needsReattach(video({ attached: true, ended: true, errored: false })), true);
+  assert.equal(needsReattach(video({ attached: true, ended: false, errored: true })), true);
+});
+
+test('idle / no current item -> never re-attach (leave the waiting screen)', () => {
+  assert.equal(needsReattach({ isPlaying: false, hasCurrentItem: true, itemKind: 'video', videoEl: null }), false);
+  assert.equal(needsReattach({ isPlaying: true, hasCurrentItem: false, itemKind: 'video', videoEl: null }), false);
+  assert.equal(needsReattach(undefined), false);
+});
+
+test('non-video surface (image/youtube/widget): re-attach only when the surface is missing', () => {
+  const base = { isPlaying: true, hasCurrentItem: true };
+  assert.equal(needsReattach({ ...base, itemKind: 'image', surfaceAttached: true }), false);
+  assert.equal(needsReattach({ ...base, itemKind: 'image', surfaceAttached: false }), true);
+  assert.equal(needsReattach({ ...base, itemKind: 'youtube', surfaceAttached: false }), true);
+  assert.equal(needsReattach({ ...base, itemKind: 'widget', surfaceAttached: true }), false);
+});