mirror of
https://github.com/got-feedBack/feedBack.git
synced 2026-07-22 21:01:40 +00:00
fix(player): keep play/pause button in sync when a JUCE reroute aborts autoplay's play() (#611)
On the first song after a fresh load on desktop, the audio engine is often still starting when the song loads, so the song begins on the HTML5 <audio> element and the engine-reroute watcher then migrates it to the JUCE backing transport. The reroute's first step is a deliberate audio.pause(), which rejects autoplay's in-flight togglePlay() audio.play() with an AbortError — even though playback continues on JUCE. togglePlay()'s catch then reset isPlaying=false and the button to "Play" while the song kept playing: the button showed Play during playback, so it took two clicks to actually pause (one to resync the flag, one to pause). The reroute already guards the <audio> 'play'/'pause' DOM listeners with window._juceRerouteInProgress; this extends the same guard to togglePlay()'s catch and the count-in catch, so a play() rejection caused by the reroute's own pause doesn't clobber the button. A genuine failure (outside a reroute) still resets correctly. Adds a regression test that drives togglePlay() through a reroute-aborted play() and asserts the button stays Pause; it fails without the guard. Claude-Session: https://claude.ai/code/session_01QbexxfTt8q2tAn436MqGWF Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
parent
8f0625e1f7
commit
6dbcc5861b
@ -6270,6 +6270,14 @@ async function togglePlay() {
|
||||
} catch (err) {
|
||||
if (sessionGen !== _audioSeekGen) return;
|
||||
if (attempt !== _playAttemptGen) return;
|
||||
// An engine reroute (HTML5 -> JUCE) deliberately pauses the <audio>
|
||||
// element mid-migration, which rejects this in-flight play() with an
|
||||
// AbortError even though playback continues on the JUCE transport.
|
||||
// The reroute owns isPlaying / the button while it runs (same guard
|
||||
// the <audio> 'play'/'pause' listeners use); resetting here would
|
||||
// leave the button showing Play while the song keeps playing — the
|
||||
// "two clicks to pause on the first song after a fresh load" bug.
|
||||
if (window._juceRerouteInProgress) return;
|
||||
console.error('[app] audio.play() rejected:', err);
|
||||
isPlaying = false;
|
||||
setPlayButtonState(false);
|
||||
@ -8998,6 +9006,10 @@ async function startCountIn(opts = {}) {
|
||||
setPlayButtonState(true);
|
||||
}).catch((err) => {
|
||||
if (gen !== _countInGen) return;
|
||||
// An engine reroute's deliberate pause aborts this play()
|
||||
// while playback continues on JUCE — don't reset the
|
||||
// button (mirrors the togglePlay guard).
|
||||
if (window._juceRerouteInProgress) return;
|
||||
// Same rationale as togglePlay: don't claim playback
|
||||
// started if the Promise rejected.
|
||||
console.error('[app] audio.play() rejected after count-in:', err);
|
||||
|
||||
69
tests/js/play_button_reroute_guard.test.js
Normal file
69
tests/js/play_button_reroute_guard.test.js
Normal file
@ -0,0 +1,69 @@
|
||||
// Regression: the play/pause button must not be reset to "Play" when an
|
||||
// in-flight togglePlay() audio.play() is rejected *because the engine reroute
|
||||
// (HTML5 -> JUCE) deliberately paused the <audio> element*. Playback continues
|
||||
// on the JUCE transport, so the button must stay "Pause" (isPlaying true).
|
||||
//
|
||||
// Bug: first song after a fresh load on desktop — the reroute's audio.pause()
|
||||
// aborts autoplay's play(); togglePlay's catch then flipped the button to Play
|
||||
// while the song kept playing, so it took two clicks to actually pause.
|
||||
//
|
||||
// Same isolation strategy as autoplay_exit.test.js: extract togglePlay() from
|
||||
// app.js by brace-matching and run it in a vm sandbox with stubbed deps.
|
||||
|
||||
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 { extractFunction } = require('./test_utils');
|
||||
|
||||
const APP_JS = path.join(__dirname, '..', '..', 'static', 'app.js');
|
||||
const SRC = fs.readFileSync(APP_JS, 'utf8');
|
||||
const TOGGLE_PLAY_SRC = extractFunction(SRC, 'async function togglePlay(');
|
||||
|
||||
// Drive togglePlay() from the not-playing state with an HTML5 audio.play() that
|
||||
// rejects, optionally with a reroute in progress. Returns the observed button
|
||||
// states and the final isPlaying flag.
|
||||
async function runTogglePlayRejecting({ rerouteInProgress }) {
|
||||
const buttonStates = [];
|
||||
const sandbox = {
|
||||
console: { log() {}, warn() {}, error() {} },
|
||||
// not-playing -> togglePlay takes the HTML5 play branch
|
||||
isPlaying: false,
|
||||
_audioSeekGen: 0,
|
||||
_playAttemptGen: 0,
|
||||
setPlayButtonState(v) { buttonStates.push(v); },
|
||||
audio: {
|
||||
// Reject like the browser does when a pending play() is interrupted
|
||||
// by a pause() (the reroute's deliberate audio.pause()).
|
||||
play: () => Promise.reject(new DOMException('aborted by pause', 'AbortError')),
|
||||
pause() {},
|
||||
},
|
||||
jucePlayer: { play: () => Promise.resolve(true), pause: () => Promise.resolve() },
|
||||
window: {
|
||||
_juceMode: false,
|
||||
_juceRerouteInProgress: rerouteInProgress ? 1 : 0,
|
||||
feedBack: { isPlaying: false, emit() {} },
|
||||
},
|
||||
};
|
||||
sandbox.globalThis = sandbox;
|
||||
vm.createContext(sandbox);
|
||||
vm.runInContext(TOGGLE_PLAY_SRC, sandbox, { filename: 'app.js#togglePlay' });
|
||||
await vm.runInContext('togglePlay()', sandbox);
|
||||
return { buttonStates, isPlaying: sandbox.isPlaying };
|
||||
}
|
||||
|
||||
test('reroute-aborted play() leaves the button on Pause (isPlaying stays true)', async () => {
|
||||
const { buttonStates, isPlaying } = await runTogglePlayRejecting({ rerouteInProgress: true });
|
||||
// Optimistic flip to Pause happened; the reroute guard must prevent the
|
||||
// catch from flipping it back to Play.
|
||||
assert.deepEqual(buttonStates, [true], 'button should only have been set to Pause, never reset to Play');
|
||||
assert.equal(isPlaying, true, 'isPlaying must stay true — the JUCE transport owns playback');
|
||||
});
|
||||
|
||||
test('a genuine play() rejection (no reroute) still resets the button to Play', async () => {
|
||||
const { buttonStates, isPlaying } = await runTogglePlayRejecting({ rerouteInProgress: false });
|
||||
assert.deepEqual(buttonStates, [true, false], 'button set to Pause then correctly reset to Play on real failure');
|
||||
assert.equal(isPlaying, false, 'isPlaying must reflect the failed start');
|
||||
});
|
||||
Loading…
Reference in New Issue
Block a user