diff --git a/pkg/service/soundtouchweb/browser_compatibility_test.go b/pkg/service/soundtouchweb/browser_compatibility_test.go index b7cf0e39..58e75779 100644 --- a/pkg/service/soundtouchweb/browser_compatibility_test.go +++ b/pkg/service/soundtouchweb/browser_compatibility_test.go @@ -636,8 +636,7 @@ func TestProviderSourceWithoutRecentsNavigatesInstead(t *testing.T) { t.Errorf("navigated = %v, want [radiobrowser]", navigated) } - // LOCAL_INTERNET_RADIO has its own browser: Play URL is what emits that - // source, so that is where a click with nothing to resume belongs. + // LOCAL_INTERNET_RADIO never resumes, so it goes straight to Play URL. var localRadioNav []string if err := chromedp.Run(ctx, chromedp.Evaluate(`window.navigated = []`, nil), @@ -767,6 +766,67 @@ func TestPlayingSourceWithoutLocationStillConfirms(t *testing.T) { } } +// TestLocalInternetRadioNeverResumes: AfterTouch plays its own TTS and the +// notification ding through LOCAL_INTERNET_RADIO, so that source's Recents mix +// one-shot audio with stations. Observed on real hardware: resuming its newest +// entry played the "AfterTouch ding". It must open Play URL instead, even when +// a perfectly resumable entry exists. +func TestLocalInternetRadioNeverResumes(t *testing.T) { + var mu sync.Mutex + recentsFetches := 0 + writes := 0 + server := newPlayerFixtureServer(t, providerFixtureScript, func(r chi.Router) { + r.Get("/api/control/devices/speaker/recents", func(w http.ResponseWriter, _ *http.Request) { + mu.Lock() + recentsFetches++ + mu.Unlock() + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"success":true,"data":{"Items":[ + {"ID":1,"ContentItem":{"Source":"LOCAL_INTERNET_RADIO","Type":"stationurl", + "Location":"https://host/custom/v1/playback/abc?name=AfterTouch+ding", + "ItemName":"AfterTouch ding","IsPresetable":true}} + ]}}`)) + }) + r.Post("/api/control/devices/speaker/play", func(w http.ResponseWriter, _ *http.Request) { + mu.Lock() + writes++ + mu.Unlock() + }) + r.Post("/api/control/devices/speaker/action/source", func(w http.ResponseWriter, _ *http.Request) { + mu.Lock() + writes++ + mu.Unlock() + }) + }) + + ctx := newHeadlessChromeContext(t) + var navigated []string + if err := chromedp.Run(ctx, + chromedp.Navigate(server.URL+"/fixture"), + chromedp.WaitVisible(`.source-btn`, chromedp.ByQuery), + chromedp.Click(`.source-btn:nth-child(2)`, chromedp.ByQuery), + chromedp.Poll(`window.navigated.length === 1`, nil), + chromedp.Evaluate(`window.navigated`, &navigated), + ); err != nil { + t.Fatalf("exercise LOCAL_INTERNET_RADIO with a resumable recent: %v", err) + } + + if len(navigated) != 1 || navigated[0] != "playurl" { + t.Errorf("navigated = %v, want [playurl]", navigated) + } + + mu.Lock() + defer mu.Unlock() + if writes != 0 { + t.Errorf("issued %d writes, want 0: this source never plays anything on click", writes) + } + // Not merely ignored: the lookup is skipped, so a slow /recents cannot + // delay opening the page. + if recentsFetches != 0 { + t.Errorf("fetched recents %d times, want 0", recentsFetches) + } +} + func TestSourceSelectionStopsReadbacksOnceTheEventStreamConfirms(t *testing.T) { var mu sync.Mutex reads := 0 diff --git a/pkg/service/soundtouchweb/static/js/components/Sources.js b/pkg/service/soundtouchweb/static/js/components/Sources.js index d8ca701d..7a52f028 100644 --- a/pkg/service/soundtouchweb/static/js/components/Sources.js +++ b/pkg/service/soundtouchweb/static/js/components/Sources.js @@ -30,14 +30,23 @@ const SOURCE_READBACK_DELAYS_MS = [2000, 5000, 10000]; // Location). ALEXA is also advertised READY but is NOT listed: whether a bare // select resumes anything for it is unverified, so it keeps today's behaviour. // -// `page` is the browser to fall back to when there is nothing to resume: the -// page in this app that produces content for that source. LOCAL_INTERNET_RADIO -// maps to Play URL because that is what HandlePlayURL emits, a ContentItem -// with Source "LOCAL_INTERNET_RADIO". +// `page` is the browser to open for the source: the page in this app that +// produces content for it. `resume` says whether clicking may first replay +// that source's newest Recents entry. +// +// LOCAL_INTERNET_RADIO does not resume. AfterTouch plays its own one-shot +// audio through that source -- TTS and the notification ding go out over +// /custom/v1/playback/ -- so its Recents mix notifications with stations, and +// the newest entry is as likely to be a ding as anything worth replaying. +// Observed on real hardware: the only LOCAL_INTERNET_RADIO recent was +// "AfterTouch ding", so resuming it played the notification. Opening Play URL, +// which is what HandlePlayURL emits content for, is predictable instead of +// guessing. RADIO_BROWSER and TUNEIN have no such problem: nothing writes +// one-shot audio to them, and their Recents hold real stations. const PROVIDER_SOURCES = { - RADIO_BROWSER: { page: 'radiobrowser' }, - TUNEIN: { page: 'tunein' }, - LOCAL_INTERNET_RADIO: { page: 'playurl' }, + RADIO_BROWSER: { page: 'radiobrowser', resume: true }, + TUNEIN: { page: 'tunein', resume: true }, + LOCAL_INTERNET_RADIO: { page: 'playurl', resume: false }, }; function isErrorSource(source) { @@ -187,10 +196,12 @@ export function Sources({ } let item = null; - try { - item = await mostRecentPlayableFor(src); - } catch (_) { - item = null; + if (provider.resume) { + try { + item = await mostRecentPlayableFor(src); + } catch (_) { + item = null; + } } // Nothing to resume, and a bare select would strand the speaker on a