fix(m3u): make guide row ids collision-proof and gate the G shortcut

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

docs/architecture/m3u-playlist-module.md

@@ -1140,11 +1140,16 @@ are unscoped, like the timeline.
 
 Row ids are not channel ids: `createChannel` falls back to the stream URL for
 an entry without an explicit id, so one stream listed in two groups yields two
-channels sharing an id. The adapter numbers repeats within the scope (`<id>`,
-`<id>#1`, …) — the guide keys its programme, coverage and selection maps by
-row id, so without that both copies lit up as playing and activating either
-one played the first. The active channel marks the FIRST row carrying its id,
-and `activate(rowId)` resolves the row back to its own channel. Opening the
+channels sharing an id. The adapter therefore builds SCOPE-LOCAL row ids by
+prefixing every row with its position in the scope (`<index>:<channel id>`) —
+the guide keys its programme, coverage and selection maps by row id, so
+without that both copies lit up as playing and activating either one played
+the first, and suffixing only the repeats was not enough because a real
+channel id can itself look like a generated suffix (ids `x`, `x`, `x#1`
+produced `x#1` twice). Ids move with the scope and the channel list, so only
+ids the guide was just handed may be passed back. The active channel marks the
+FIRST row carrying its id (`null` when it is outside the scope), and
+`activate(rowId)` resolves the row back to its own channel. Opening the
 guide mirrors the sidebar view (`applyInitialScope`): favorites stays
 favorites, and the groups view opens on the group the sidebar's rail is
 SHOWING — forwarded from `GroupsViewComponent.selectedGroupChange` through the
@@ -1170,7 +1175,12 @@ toolbar (`EpgTimelineComponent.openGuide`) and the `G` key on the player page.
 The header action reports `disabled` whenever the guide cannot open, which
 greys out the header button and disables its palette command instead of
 offering a no-op. Player fullscreen, radio, recognised movies, switching to
-another playlist and the PWA close or withhold it. Inside the guide: single click on a row or an
+another playlist and the PWA close or withhold it. `openGuide()` itself also
+refuses while ANY element is fullscreen or a CDK dialog is open — the guide
+mounts in the page flow, so it would be painted over and still swallow the
+keyboard — and the `G` key additionally ignores a press raised inside a
+`.cdk-overlay-pane`, `[role="menu"]` or `[role="dialog"]`, the same surfaces
+the PageUp/PageDown zapping yields to. Inside the guide: single click on a row or an
 "on now" card switches the channel and keeps the guide open, double-click or
 Enter switches and closes, other cards open the programme dialog; ↑/↓ ←/→
 navigate, I details, N now, PgUp/PgDn day, Esc close — the keyboard controller

libs/playlist/m3u/feature-player/src/lib/epg-guide/m3u-epg-guide-source.service.spec.ts

@@ -111,16 +111,22 @@ describe('M3uEpgGuideSourceService', () => {
             1, 2, 3,
         ]);
         service.setScope('group:Sports');
+        // Row ids are scope-local: the position inside the scope, then the
+        // channel id.
         expect(service.channels().map((channel) => channel.id)).toEqual([
-            'b',
-            'c',
+            '0:b',
+            '1:c',
         ]);
         expect(service.channels()[0].number).toBe(1);
         service.setScope('favorites');
-        expect(service.channels().map((channel) => channel.id)).toEqual(['b']);
+        expect(service.channels().map((channel) => channel.id)).toEqual([
+            '0:b',
+        ]);
         // Legacy rows saved before URL keys are still matched by channel id.
         favoriteKeys.set(['c']);
-        expect(service.channels().map((channel) => channel.id)).toEqual(['c']);
+        expect(service.channels().map((channel) => channel.id)).toEqual([
+            '0:c',
+        ]);
     });
 
     it('derives the EPG key from tvg-id, then name, and null for blank names', () => {
@@ -142,12 +148,12 @@ describe('M3uEpgGuideSourceService', () => {
             fromMs: 1,
             toMs: 2,
         });
-        expect(programs.get('a')?.[0].title).toBe('a.tv show');
-        expect(programs.get('b')).toEqual([]);
-        expect(programs.has('c')).toBe(false);
+        expect(programs.get('0:a')?.[0].title).toBe('a.tv show');
+        expect(programs.get('1:b')).toEqual([]);
+        expect(programs.has('2:c')).toBe(false);
 
         const covered = await service.loadCoverage(window);
-        expect(covered).toEqual(new Set(['a']));
+        expect(covered).toEqual(new Set(['0:a']));
     });
 
     it('answers empty results when the bridge is unavailable', async () => {
@@ -159,8 +165,8 @@ describe('M3uEpgGuideSourceService', () => {
     });
 
     it('mirrors the active channel and dispatches playback on activate', () => {
-        expect(service.activeChannelId()).toBe('a');
-        service.activate('b');
+        expect(service.activeChannelId()).toBe('0:a');
+        service.activate('1:b');
         expect(dispatch).toHaveBeenCalledWith(
             ChannelActions.setActiveChannel({
                 channel: channels()[1],
@@ -173,10 +179,13 @@ describe('M3uEpgGuideSourceService', () => {
 
     it('keeps duplicate channel ids apart and activates the row that was clicked', () => {
         // `createChannel` falls back to the URL as the id, so one stream
-        // listed twice yields two channels sharing an id.
+        // listed twice yields two channels sharing an id. A real id can also
+        // look like a generated suffix (`x#1`), so suffixing the repeats made
+        // the third row collide with the second; the position prefix cannot.
         const duplicated = signal<Channel[]>([
-            makeChannel('dup', { name: 'First copy', group: 'News' }),
-            makeChannel('dup', { name: 'Second copy', group: 'News' }),
+            makeChannel('x', { name: 'First copy', group: 'News' }),
+            makeChannel('x', { name: 'Second copy', group: 'News' }),
+            makeChannel('x#1', { name: 'Hashed id', group: 'News' }),
         ]);
         service.bind({
             channels: duplicated,
@@ -186,18 +195,26 @@ describe('M3uEpgGuideSourceService', () => {
         });
 
         const rowIds = service.channels().map((channel) => channel.id);
-        expect(rowIds).toEqual(['dup', 'dup#1']);
+        expect(rowIds).toEqual(['0:x', '1:x', '2:x#1']);
+        expect(new Set(rowIds).size).toBe(3);
         // The playing channel marks the first of the two rows.
-        expect(service.activeChannelId()).toBe('dup');
+        expect(service.activeChannelId()).toBe('0:x');
 
-        service.activate('dup#1');
-
-        expect(dispatch).toHaveBeenCalledWith(
+        service.activate('1:x');
+        expect(dispatch).toHaveBeenLastCalledWith(
             ChannelActions.setActiveChannel({
                 channel: duplicated()[1],
                 startPlayback: true,
             })
         );
+
+        service.activate('2:x#1');
+        expect(dispatch).toHaveBeenLastCalledWith(
+            ChannelActions.setActiveChannel({
+                channel: duplicated()[2],
+                startPlayback: true,
+            })
+        );
     });
 
     it('seeds the initial scope from the sidebar view', () => {
@@ -256,7 +273,7 @@ describe('M3uEpgGuideSourceService', () => {
     it('forwards programme search to the bridge', async () => {
         searchPrograms.mockResolvedValue([program('a.tv'), program('x')]);
         const hits = await service.searchPrograms('news');
-        expect(hits.map((hit) => hit.channelId)).toEqual(['a', null]);
+        expect(hits.map((hit) => hit.channelId)).toEqual(['0:a', null]);
         expect(hits[1].program.title).toBe('x show');
         expect(searchPrograms).toHaveBeenCalledWith('news', 20);
     });

libs/playlist/m3u/feature-player/src/lib/epg-guide/m3u-epg-guide-source.service.ts

@@ -122,28 +122,28 @@ export class M3uEpgGuideSourceService implements EpgGuideSource {
      * explicit id, so one stream listed in two groups — or simply repeated —
      * yields two channels sharing an id. The guide keys its programme, status
      * and selection maps by row id, so duplicates used to collide: both rows
-     * lit up as playing and activating either one played the first. The first
-     * occurrence keeps the channel id (nothing changes for a playlist without
-     * duplicates) and every later one is suffixed.
+     * lit up as playing and activating either one played the first.
+     * Every row is therefore prefixed with its position in the scope: two rows
+     * always differ in that leading integer, so no channel id can collide with
+     * another row (suffixing only the repeats did not hold — a playlist whose
+     * ids are `x`, `x` and `x#1` produced `x#1` twice). Ids are SCOPE-LOCAL:
+     * they change with the scope and the channel list, and only ids the guide
+     * was just handed in `channels()` may be passed back.
      */
     private readonly scopedRows = computed<
         Array<{ rowId: string; channel: Channel }>
-    >(() => {
-        const occurrences = new Map<string, number>();
-        return this.scopedChannels().map((channel) => {
-            const seen = occurrences.get(channel.id) ?? 0;
-            occurrences.set(channel.id, seen + 1);
-            return {
-                rowId: seen === 0 ? channel.id : `${channel.id}#${seen}`,
-                channel,
-            };
-        });
-    });
+    >(() =>
+        this.scopedChannels().map((channel, index) => ({
+            rowId: `${index}:${channel.id}`,
+            channel,
+        }))
+    );
 
     private readonly channelsByRowId = computed(
         () => new Map(this.scopedRows().map((row) => [row.rowId, row.channel]))
     );
 
+    /** `EpgGuideChannel.id` is the scope-local row id, not `Channel.id`. */
     readonly channels = computed<EpgGuideChannel[]>(() => {
         const strip = this.settingsStore.stripCountryPrefix?.();
         return this.scopedRows().map(({ rowId, channel }, index) => ({
@@ -156,8 +156,10 @@ export class M3uEpgGuideSourceService implements EpgGuideSource {
     });
 
     /**
-     * The first row carrying the active channel's id — duplicates are
-     * indistinguishable from here, so the guide marks the first of them.
+     * The row id of the first row carrying the active channel's id —
+     * duplicates are indistinguishable from here, so the guide marks the first
+     * of them. `null` when the playing channel is outside the current scope:
+     * row ids are scope-local, so there is nothing to point at.
      */
     readonly activeChannelId = computed(() => {
         const active = this.inputs()?.activeChannel();
@@ -166,7 +168,7 @@ export class M3uEpgGuideSourceService implements EpgGuideSource {
         }
         return (
             this.scopedRows().find((row) => row.channel.id === active.id)
-                ?.rowId ?? active.id
+                ?.rowId ?? null
         );
     });
 

libs/playlist/m3u/feature-player/src/lib/video-player/video-player-guide-gating.spec.ts

@@ -0,0 +1,226 @@
+import { NO_ERRORS_SCHEMA, signal } from '@angular/core';
+import { ComponentFixture, TestBed } from '@angular/core/testing';
+import { ActivatedRoute, Router } from '@angular/router';
+import { Store } from '@ngrx/store';
+import { StorageMap } from '@ngx-pwa/local-storage';
+import { of } from 'rxjs';
+import { EpgService } from '@iptvnator/epg/data-access';
+import { PlaylistContextFacade } from '@iptvnator/playlist/shared/util';
+import { PORTAL_EXTERNAL_PLAYBACK } from '@iptvnator/portal/shared/util';
+import {
+    DataService,
+    PlaylistsService,
+    RuntimeCapabilitiesService,
+    SettingsStore,
+    TmdbEnrichmentService,
+} from '@iptvnator/services';
+import { Settings, VideoPlayer } from '@iptvnator/shared/interfaces';
+import type { VideoPlayerComponent as VideoPlayerComponentInstance } from './video-player.component';
+import {
+    dataServiceMock,
+    epgServiceMock,
+    epgUrlSetting,
+    epgViewMode,
+    externalSession,
+    player,
+    playlistId,
+    playlistsServiceMock,
+    routerMock,
+    sampleChannel,
+    showCaptions,
+    storeMock,
+    stripCountryPrefix,
+    syncStoreState,
+    translateServiceProvider,
+} from './video-player.spec-harness';
+
+jest.unstable_mockModule('video.js', () => ({
+    default: jest.fn(),
+}));
+jest.unstable_mockModule('@yangkghjh/videojs-aspect-ratio-panel', () => ({}));
+jest.unstable_mockModule('videojs-contrib-quality-levels', () => ({}));
+jest.unstable_mockModule('videojs-quality-selector-hls', () => ({}));
+
+/**
+ * What may NOT open the programme guide. The guide mounts in the page flow,
+ * so a fullscreen element or a dialog paints over it while it still owns the
+ * keyboard. Kept apart from `video-player.component.spec.ts`, which sits at
+ * the spec line budget; the template is reduced to the panel's `ng-template`
+ * so no child stubs are needed.
+ */
+describe('VideoPlayerComponent — guide gating', () => {
+    let VideoPlayerComponent: typeof import('./video-player.component').VideoPlayerComponent;
+    let fixture: ComponentFixture<VideoPlayerComponentInstance>;
+    let component: VideoPlayerComponentInstance;
+
+    function setFullscreenElement(element: Element | null): void {
+        Object.defineProperty(document, 'fullscreenElement', {
+            configurable: true,
+            get: () => element,
+        });
+    }
+
+    /** A stand-in for the inline live player, inside the component's host. */
+    function mountPlayerView(): HTMLElement {
+        const playerView = document.createElement('app-web-player-view');
+        fixture.nativeElement.appendChild(playerView);
+        return playerView;
+    }
+
+    function pressGuideKey(target: EventTarget = document): void {
+        target.dispatchEvent(
+            new KeyboardEvent('keydown', { key: 'g', bubbles: true })
+        );
+    }
+
+    beforeAll(async () => {
+        ({ VideoPlayerComponent } = await import('./video-player.component'));
+    });
+
+    beforeEach(async () => {
+        setFullscreenElement(null);
+        syncStoreState(sampleChannel);
+        playlistId.set('playlist-1');
+        player.set(VideoPlayer.VideoJs);
+        showCaptions.set(false);
+        stripCountryPrefix.set(false);
+        externalSession.set(null);
+        storeMock.dispatch.mockClear();
+
+        await TestBed.configureTestingModule({
+            imports: [VideoPlayerComponent],
+            schemas: [NO_ERRORS_SCHEMA],
+            providers: [
+                {
+                    provide: ActivatedRoute,
+                    useValue: {
+                        params: of({ id: playlistId(), view: 'all' }),
+                        queryParams: of({}),
+                        snapshot: {
+                            data: { layout: 'workspace' },
+                            queryParams: {},
+                        },
+                    },
+                },
+                { provide: Router, useValue: routerMock },
+                { provide: Store, useValue: storeMock },
+                translateServiceProvider,
+                { provide: DataService, useValue: dataServiceMock },
+                {
+                    provide: RuntimeCapabilitiesService,
+                    useValue: {
+                        supportsEpg: true,
+                        isElectron: false,
+                        supportsRemoteControl: false,
+                    },
+                },
+                { provide: PlaylistsService, useValue: playlistsServiceMock },
+                { provide: EpgService, useValue: epgServiceMock },
+                {
+                    provide: PlaylistContextFacade,
+                    useValue: { resolvedPlaylistId: playlistId },
+                },
+                {
+                    provide: TmdbEnrichmentService,
+                    useValue: { isEnabled: () => false },
+                },
+                {
+                    provide: SettingsStore,
+                    useValue: {
+                        player,
+                        showCaptions,
+                        stripCountryPrefix,
+                        m3uVodDetails: signal(true),
+                        resolvedEpgViewMode: epgViewMode,
+                        resolvedEpgOffsetMinutes: signal(0),
+                        epgUrl: epgUrlSetting,
+                    },
+                },
+                {
+                    provide: StorageMap,
+                    useValue: {
+                        get: jest.fn(() =>
+                            of({ player: player() } as Partial<Settings>)
+                        ),
+                    },
+                },
+                {
+                    provide: PORTAL_EXTERNAL_PLAYBACK,
+                    useValue: { activeSession: externalSession },
+                },
+            ],
+        })
+            .overrideComponent(VideoPlayerComponent, {
+                set: {
+                    imports: [],
+                    template:
+                        '<ng-template #fullscreenChannelPanel></ng-template>',
+                },
+            })
+            .compileComponents();
+
+        fixture = TestBed.createComponent(VideoPlayerComponent);
+        component = fixture.componentInstance;
+        fixture.detectChanges();
+    });
+
+    afterEach(() => {
+        fixture?.destroy();
+        setFullscreenElement(null);
+        document
+            .querySelectorAll('.cdk-overlay-container, [role="dialog"]')
+            .forEach((element) => element.remove());
+    });
+
+    it('leaves the guide closed while the live player owns fullscreen', () => {
+        expect(component.canOpenGuide()).toBe(true);
+        setFullscreenElement(mountPlayerView());
+
+        pressGuideKey();
+
+        // The fullscreen surface paints over the page: the guide would be
+        // invisible there and still swallow the keys.
+        expect(component.guideOpen()).toBe(false);
+        component.openGuide();
+        expect(component.guideOpen()).toBe(false);
+
+        setFullscreenElement(null);
+        pressGuideKey();
+        expect(component.guideOpen()).toBe(true);
+    });
+
+    it('leaves the guide closed while any element is fullscreen', () => {
+        const other = document.createElement('div');
+        document.body.appendChild(other);
+        setFullscreenElement(other);
+
+        component.openGuide();
+
+        expect(component.guideOpen()).toBe(false);
+        other.remove();
+    });
+
+    it('leaves the guide closed while a dialog is open', () => {
+        const overlayContainer = document.createElement('div');
+        overlayContainer.className = 'cdk-overlay-container';
+        const dialog = document.createElement('div');
+        dialog.setAttribute('role', 'dialog');
+        const dialogButton = document.createElement('button');
+        dialog.appendChild(dialogButton);
+        overlayContainer.appendChild(dialog);
+        document.body.appendChild(overlayContainer);
+
+        // The header action and the command palette go through `openGuide()`
+        // without an event to inspect.
+        component.openGuide();
+        expect(component.guideOpen()).toBe(false);
+
+        // A G from a focused control inside the dialog belongs to the dialog.
+        pressGuideKey(dialogButton);
+        expect(component.guideOpen()).toBe(false);
+
+        overlayContainer.remove();
+        pressGuideKey();
+        expect(component.guideOpen()).toBe(true);
+    });
+});

libs/playlist/m3u/feature-player/src/lib/video-player/video-player.component.ts

@@ -1239,13 +1239,41 @@ export class VideoPlayerComponent
     }
 
     openGuide(): void {
-        if (!this.canOpenGuide() || this.guideOpen()) {
+        if (!this.canOpenGuide() || this.guideOpen() || this.isGuideBlocked()) {
             return;
         }
         this.guideSource.applyInitialScope(this.activeView());
         this.guideOpen.set(true);
     }
 
+    /**
+     * The guide mounts in the page flow, so anything painting over the page
+     * would hide it while it still swallowed the keyboard: a fullscreen
+     * element (the live player's own, which has the fullscreen channel panel
+     * instead — see {@link onFullscreenChange} — or any other) and an open
+     * CDK dialog. Gated here rather than in the key handler alone so the
+     * header action and the command palette are covered too.
+     */
+    private isGuideBlocked(): boolean {
+        return (
+            !!document.fullscreenElement ||
+            !!document.querySelector('.cdk-overlay-container [role="dialog"]')
+        );
+    }
+
+    /** Whether the event was raised inside a menu, dialog or CDK overlay. */
+    private isInsideOverlaySurface(event: KeyboardEvent): boolean {
+        return event
+            .composedPath()
+            .some(
+                (target) =>
+                    target instanceof Element &&
+                    target.matches(
+                        '.cdk-overlay-pane, [role="menu"], [role="dialog"]'
+                    )
+            );
+    }
+
     closeGuide(): void {
         this.guideOpen.set(false);
     }
@@ -1301,8 +1329,14 @@ export class VideoPlayerComponent
             return;
         }
         if (isGuideKey && this.canOpenGuide()) {
-            event.preventDefault();
-            this.openGuide();
+            // A G from a focused control inside a dialog or menu belongs to
+            // that surface, not to the page behind it.
+            if (!this.isInsideOverlaySurface(event)) {
+                this.openGuide();
+            }
+            if (this.guideOpen()) {
+                event.preventDefault();
+            }
             return;
         }
         if (
@@ -1320,15 +1354,7 @@ export class VideoPlayerComponent
         // the one way to change channels from the keyboard in fullscreen.
         if (event.key === 'PageUp' || event.key === 'PageDown') {
             if (
-                event
-                    .composedPath()
-                    .some(
-                        (target) =>
-                            target instanceof Element &&
-                            target.matches(
-                                '.cdk-overlay-pane, [role="menu"], [role="dialog"]'
-                            )
-                    ) ||
+                this.isInsideOverlaySurface(event) ||
                 isInsideScrollableRegion(
                     event.target,
                     this.hostElement.nativeElement