From 09e2e08fc1039e1583cf92abe98448dc04136e7b Mon Sep 17 00:00:00 2001 From: 4gray Date: Fri, 22 May 2026 09:40:11 +0300 Subject: [PATCH] refactor(runtime): gate settings external players by capability --- .../settings-playback-section.component.html | 6 +-- ...ettings-playback-section.component.spec.ts | 25 ++++++++--- .../settings-playback-section.component.ts | 1 + .../src/app/settings/settings.component.html | 1 + .../app/settings/settings.component.spec.ts | 45 ++++++++++++++++++- .../src/app/settings/settings.component.ts | 10 ++++- docs/architecture/pwa-self-hosted.md | 7 +-- .../lib/runtime-capabilities.service.spec.ts | 30 ++++++++----- .../src/lib/runtime-capabilities.service.ts | 6 +++ 9 files changed, 104 insertions(+), 27 deletions(-) diff --git a/apps/web/src/app/settings/settings-playback-section.component.html b/apps/web/src/app/settings/settings-playback-section.component.html index b3a26a9d7..3ba06a0ff 100644 --- a/apps/web/src/app/settings/settings-playback-section.component.html +++ b/apps/web/src/app/settings/settings-playback-section.component.html @@ -69,7 +69,7 @@ - @if (isDesktop() && isExternalPlayerSelected()) { + @if (supportsManagedExternalPlayers() && isExternalPlayerSelected()) {
} - @if (isDesktop() && form().value.player === 'mpv') { + @if (supportsManagedExternalPlayers() && form().value.player === 'mpv') {

{{ 'SETTINGS.MPV_PLAYER_PATH_LABEL' | translate }}

@@ -220,7 +220,7 @@
} - @if (isDesktop() && form().value.player === 'vlc') { + @if (supportsManagedExternalPlayers() && form().value.player === 'vlc') {

{{ 'SETTINGS.VLC_PLAYER_PATH_LABEL' | translate }}

diff --git a/apps/web/src/app/settings/settings-playback-section.component.spec.ts b/apps/web/src/app/settings/settings-playback-section.component.spec.ts index 0658fa3ec..c1f29a349 100644 --- a/apps/web/src/app/settings/settings-playback-section.component.spec.ts +++ b/apps/web/src/app/settings/settings-playback-section.component.spec.ts @@ -58,9 +58,10 @@ describe('SettingsPlaybackSectionComponent', () => { fixture.componentRef.setInput('streamFormatEnum', StreamFormat); }); - it('hides the external-player double-click option outside desktop builds', () => { + it('hides the external-player double-click option when managed external players are unsupported', () => { fixture.componentRef.setInput('form', createForm(VideoPlayer.MPV)); - fixture.componentRef.setInput('isDesktop', false); + fixture.componentRef.setInput('isDesktop', true); + fixture.componentRef.setInput('supportsManagedExternalPlayers', false); fixture.detectChanges(); expect( @@ -75,6 +76,7 @@ describe('SettingsPlaybackSectionComponent', () => { it('hides the external-player double-click option for embedded players', () => { fixture.componentRef.setInput('isDesktop', true); + fixture.componentRef.setInput('supportsManagedExternalPlayers', true); fixture.detectChanges(); expect( @@ -85,10 +87,11 @@ describe('SettingsPlaybackSectionComponent', () => { }); it.each([VideoPlayer.MPV, VideoPlayer.VLC])( - 'labels the double-click option as external-player behavior on desktop for %s', + 'labels the double-click option as external-player behavior when managed players are supported for %s', (player) => { fixture.componentRef.setInput('form', createForm(player)); fixture.componentRef.setInput('isDesktop', true); + fixture.componentRef.setInput('supportsManagedExternalPlayers', true); fixture.detectChanges(); expect( @@ -106,6 +109,7 @@ describe('SettingsPlaybackSectionComponent', () => { const form = createForm(); fixture.componentRef.setInput('form', form); fixture.componentRef.setInput('isDesktop', true); + fixture.componentRef.setInput('supportsManagedExternalPlayers', true); fixture.detectChanges(); expect( @@ -147,6 +151,7 @@ describe('SettingsPlaybackSectionComponent', () => { it('shows MPV bundle guidance and the IINA executable tip for desktop MPV playback', () => { fixture.componentRef.setInput('form', createForm(VideoPlayer.MPV)); fixture.componentRef.setInput('isDesktop', true); + fixture.componentRef.setInput('supportsManagedExternalPlayers', true); fixture.detectChanges(); expect(fixture.nativeElement.textContent).toContain( @@ -162,9 +167,10 @@ describe('SettingsPlaybackSectionComponent', () => { ); }); - it('hides MPV path guidance and the IINA executable tip outside desktop builds', () => { + it('hides MPV path guidance and the IINA executable tip when managed external players are unsupported', () => { fixture.componentRef.setInput('form', createForm(VideoPlayer.MPV)); - fixture.componentRef.setInput('isDesktop', false); + fixture.componentRef.setInput('isDesktop', true); + fixture.componentRef.setInput('supportsManagedExternalPlayers', false); fixture.detectChanges(); expect(fixture.nativeElement.textContent).not.toContain( @@ -183,6 +189,7 @@ describe('SettingsPlaybackSectionComponent', () => { it('shows VLC bundle guidance without the IINA tip for desktop VLC playback', () => { fixture.componentRef.setInput('form', createForm(VideoPlayer.VLC)); fixture.componentRef.setInput('isDesktop', true); + fixture.componentRef.setInput('supportsManagedExternalPlayers', true); fixture.detectChanges(); expect(fixture.nativeElement.textContent).toContain( @@ -198,9 +205,10 @@ describe('SettingsPlaybackSectionComponent', () => { ); }); - it('hides VLC path guidance outside desktop builds', () => { + it('hides VLC path guidance when managed external players are unsupported', () => { fixture.componentRef.setInput('form', createForm(VideoPlayer.VLC)); - fixture.componentRef.setInput('isDesktop', false); + fixture.componentRef.setInput('isDesktop', true); + fixture.componentRef.setInput('supportsManagedExternalPlayers', false); fixture.detectChanges(); expect(fixture.nativeElement.textContent).not.toContain( @@ -215,6 +223,7 @@ describe('SettingsPlaybackSectionComponent', () => { it('does not show external-player path guidance for embedded players', () => { fixture.componentRef.setInput('isDesktop', true); + fixture.componentRef.setInput('supportsManagedExternalPlayers', true); fixture.detectChanges(); expect(fixture.nativeElement.textContent).not.toContain( @@ -233,6 +242,7 @@ describe('SettingsPlaybackSectionComponent', () => { it('shows MPV command-line arguments only when MPV is selected', () => { fixture.componentRef.setInput('form', createForm(VideoPlayer.MPV)); fixture.componentRef.setInput('isDesktop', true); + fixture.componentRef.setInput('supportsManagedExternalPlayers', true); fixture.detectChanges(); expect( @@ -258,6 +268,7 @@ describe('SettingsPlaybackSectionComponent', () => { it('shows VLC command-line arguments only when VLC is selected', () => { fixture.componentRef.setInput('form', createForm(VideoPlayer.VLC)); fixture.componentRef.setInput('isDesktop', true); + fixture.componentRef.setInput('supportsManagedExternalPlayers', true); fixture.detectChanges(); expect( diff --git a/apps/web/src/app/settings/settings-playback-section.component.ts b/apps/web/src/app/settings/settings-playback-section.component.ts index f5def7a1e..08962c16d 100644 --- a/apps/web/src/app/settings/settings-playback-section.component.ts +++ b/apps/web/src/app/settings/settings-playback-section.component.ts @@ -45,6 +45,7 @@ export class SettingsPlaybackSectionComponent { readonly players = input.required(); readonly streamFormatEnum = input.required(); readonly isDesktop = input(false); + readonly supportsManagedExternalPlayers = input(false); readonly selectRecordingFolder = output(); isExternalPlayerSelected(): boolean { diff --git a/apps/web/src/app/settings/settings.component.html b/apps/web/src/app/settings/settings.component.html index 314e151e4..9f2abe4b5 100644 --- a/apps/web/src/app/settings/settings.component.html +++ b/apps/web/src/app/settings/settings.component.html @@ -48,6 +48,7 @@ [players]="players()" [streamFormatEnum]="streamFormatEnum" [isDesktop]="isDesktop" + [supportsManagedExternalPlayers]="supportsManagedExternalPlayers" (selectRecordingFolder)="selectRecordingFolder()" /> diff --git a/apps/web/src/app/settings/settings.component.spec.ts b/apps/web/src/app/settings/settings.component.spec.ts index 006f003c7..c2c7d8d11 100644 --- a/apps/web/src/app/settings/settings.component.spec.ts +++ b/apps/web/src/app/settings/settings.component.spec.ts @@ -253,6 +253,8 @@ describe('SettingsComponent', () => { forceFetchEpg: jest.fn().mockResolvedValue({ success: true }), getAppVersion: jest.fn().mockResolvedValue('1.0.0'), getLocalIpAddresses: jest.fn().mockResolvedValue([]), + openInMpv: jest.fn(), + openInVlc: jest.fn(), platform: 'linux', saveFileDialog: jest.fn().mockResolvedValue('/tmp/backup.json'), setMpvPlayerPath: jest.fn().mockResolvedValue(undefined), @@ -472,8 +474,44 @@ describe('SettingsComponent', () => { ).toBe(true); }); + it('hides external player path settings when the Electron bridge is incomplete', async () => { + fixture.destroy(); + window.electron = { + getAppVersion: jest.fn().mockResolvedValue('1.0.0'), + platform: 'linux', + updateSettings: jest.fn().mockResolvedValue(undefined), + } as unknown as typeof window.electron; + + const partialBridgeFixture = + TestBed.createComponent(SettingsComponent); + const partialBridgeComponent = + partialBridgeFixture.componentInstance; + partialBridgeComponent.checkAppVersion = jest.fn(); + partialBridgeComponent.fetchLocalIpAddresses = jest + .fn() + .mockResolvedValue(undefined); + partialBridgeFixture.detectChanges(); + + expect(partialBridgeComponent.isDesktop).toBe(true); + expect( + partialBridgeComponent.supportsExternalPlayerPathSettings + ).toBe(false); + expect( + partialBridgeComponent + .players() + .some((player) => player.id === VideoPlayer.MPV) + ).toBe(false); + expect( + partialBridgeComponent + .players() + .some((player) => player.id === VideoPlayer.VLC) + ).toBe(false); + }); + it('does not block settings initialization while embedded mpv support is pending', async () => { - let resolveSupport: (value: EmbeddedMpvSupport) => void; + let resolveSupport: + | ((value: EmbeddedMpvSupport) => void) + | undefined; window.electron = { ...window.electron, getEmbeddedMpvSupport: jest.fn( @@ -495,8 +533,11 @@ describe('SettingsComponent', () => { ).toBe(false); if (!resolveSupport) { - throw new Error('Expected embedded MPV support resolver'); + throw new Error( + 'Expected embedded MPV support probe to start' + ); } + resolveSupport({ supported: true, platform: 'darwin', diff --git a/apps/web/src/app/settings/settings.component.ts b/apps/web/src/app/settings/settings.component.ts index 010f6cd93..7afed4d31 100644 --- a/apps/web/src/app/settings/settings.component.ts +++ b/apps/web/src/app/settings/settings.component.ts @@ -137,6 +137,8 @@ export class SettingsComponent implements OnInit, OnDestroy { /** Flag that indicates whether the app runs in electron environment */ readonly isDesktop = this.runtime.isElectron; + readonly supportsExternalPlayerPathSettings = + this.runtime.supportsExternalPlayerPathSettings; readonly embeddedMpvSupport = signal(null); readonly supportsEmbeddedMpv = computed( () => this.isDesktop && !!this.embeddedMpvSupport()?.supported @@ -156,13 +158,15 @@ export class SettingsComponent implements OnInit, OnDestroy { }, ] : []), - ...SETTINGS_OS_PLAYER_OPTIONS, + ...(this.supportsExternalPlayerPathSettings + ? SETTINGS_OS_PLAYER_OPTIONS + : []), ]); /** Player options */ readonly players = computed(() => [ ...SETTINGS_EMBEDDED_PLAYER_OPTIONS, - ...(this.isDesktop ? this.osPlayers() : []), + ...this.osPlayers(), ]); /** Current version of the app */ @@ -481,7 +485,9 @@ export class SettingsComponent implements OnInit, OnDestroy { if (window.electron) { window.electron.updateSettings(settings); + } + if (this.supportsExternalPlayerPathSettings && window.electron) { window.electron.setMpvPlayerPath(settings.mpvPlayerPath); window.electron.setVlcPlayerPath(settings.vlcPlayerPath); } diff --git a/docs/architecture/pwa-self-hosted.md b/docs/architecture/pwa-self-hosted.md index 0b721701e..9ed479317 100644 --- a/docs/architecture/pwa-self-hosted.md +++ b/docs/architecture/pwa-self-hosted.md @@ -77,9 +77,10 @@ requires the complete downloads preload API surface used by `DownloadsService`, bridge, `supportsXtreamSectionNavigation` is available in PWA and in Electron when either the SQLite Xtream data source or the Xtream API transport is available, and -`supportsManagedExternalPlayers` requires the MPV and VLC preload launch methods -(`openInMpv` and `openInVlc`); a partial Electron bridge must not expose -desktop-only actions in the PWA/shared UI. +`supportsManagedExternalPlayers` requires the MPV and VLC preload launch and +path-setting methods (`openInMpv`, `openInVlc`, `setMpvPlayerPath`, and +`setVlcPlayerPath`); a partial Electron bridge must not expose desktop-only +actions in the PWA/shared UI. ## Runtime Limitations diff --git a/libs/services/src/lib/runtime-capabilities.service.spec.ts b/libs/services/src/lib/runtime-capabilities.service.spec.ts index da931fa24..7ed2f8562 100644 --- a/libs/services/src/lib/runtime-capabilities.service.spec.ts +++ b/libs/services/src/lib/runtime-capabilities.service.spec.ts @@ -27,6 +27,7 @@ describe('RuntimeCapabilitiesService', () => { expect(service.supportsPortalActivityStorage).toBe(false); expect(service.supportsPlaylistRefresh).toBe(false); expect(service.supportsManagedExternalPlayers).toBe(false); + expect(service.supportsExternalPlayerPathSettings).toBe(false); expect(service.supportsEmbeddedMpv).toBe(false); expect(service.supportsDesktopFileSave).toBe(false); expect(service.supportsRemoteControl).toBe(false); @@ -97,6 +98,8 @@ describe('RuntimeCapabilitiesService', () => { onPlaylistRefreshEvent: jest.fn(), openInMpv: jest.fn(), openInVlc: jest.fn(), + setMpvPlayerPath: jest.fn(), + setVlcPlayerPath: jest.fn(), prepareEmbeddedMpv: jest.fn(), saveFileDialog: jest.fn(), writeFile: jest.fn(), @@ -120,6 +123,7 @@ describe('RuntimeCapabilitiesService', () => { expect(service.supportsPortalActivityStorage).toBe(true); expect(service.supportsPlaylistRefresh).toBe(true); expect(service.supportsManagedExternalPlayers).toBe(true); + expect(service.supportsExternalPlayerPathSettings).toBe(true); expect(service.supportsEmbeddedMpv).toBe(true); expect(service.supportsDesktopFileSave).toBe(true); expect(service.supportsRemoteControl).toBe(true); @@ -143,6 +147,7 @@ describe('RuntimeCapabilitiesService', () => { expect(service.supportsPortalActivityStorage).toBe(false); expect(service.supportsPlaylistRefresh).toBe(false); expect(service.supportsManagedExternalPlayers).toBe(false); + expect(service.supportsExternalPlayerPathSettings).toBe(false); expect(service.supportsEmbeddedMpv).toBe(false); expect(service.supportsDesktopFileSave).toBe(false); expect(service.supportsRemoteControl).toBe(false); @@ -170,22 +175,27 @@ describe('RuntimeCapabilitiesService', () => { expect(service.supportsXtreamSqliteDataSource).toBe(false); }); - it('requires both managed external player launch methods', () => { - testWindow.electron = { - openInMpv: jest.fn(), - }; - - const service = new RuntimeCapabilitiesService(); - - expect(service.isElectron).toBe(true); - expect(service.supportsManagedExternalPlayers).toBe(false); - + it('decouples external player launch support from path-setting support', () => { testWindow.electron = { openInMpv: jest.fn(), openInVlc: jest.fn(), }; + const service = new RuntimeCapabilitiesService(); + + expect(service.isElectron).toBe(true); expect(service.supportsManagedExternalPlayers).toBe(true); + expect(service.supportsExternalPlayerPathSettings).toBe(false); + + testWindow.electron = { + openInMpv: jest.fn(), + openInVlc: jest.fn(), + setMpvPlayerPath: jest.fn(), + setVlcPlayerPath: jest.fn(), + }; + + expect(service.supportsManagedExternalPlayers).toBe(true); + expect(service.supportsExternalPlayerPathSettings).toBe(true); }); it('requires the complete downloads preload surface', () => { diff --git a/libs/services/src/lib/runtime-capabilities.service.ts b/libs/services/src/lib/runtime-capabilities.service.ts index 313b5ce53..8d47c1cdd 100644 --- a/libs/services/src/lib/runtime-capabilities.service.ts +++ b/libs/services/src/lib/runtime-capabilities.service.ts @@ -136,6 +136,12 @@ export class RuntimeCapabilitiesService { ); } + get supportsExternalPlayerPathSettings(): boolean { + return ['setMpvPlayerPath', 'setVlcPlayerPath'].every((methodName) => + this.hasElectronMethod(methodName) + ); + } + get supportsEmbeddedMpv(): boolean { return this.hasElectronMethod('prepareEmbeddedMpv'); }