fix(filmstrip) Fixes an issue where remote tiles can disappear when SS is started

* fix(filmstrip) Fixes an issue where remote tiles can disappear when SS is started. Regression caused by 82d4628976. More testcases have been added.
2025-12-30 11:22:31 +00:00 · 2025-10-21 19:13:15 -04:00
parent b2f7b3be6c
commit 077602c427
4 changed files with 150 additions and 16 deletions
--- a/react/features/filmstrip/functions.any.ts
+++ b/react/features/filmstrip/functions.any.ts
@@ -73,33 +73,32 @@ export function updateRemoteParticipants(store: IStore, force?: boolean, partici
    for (const screenshare of screenShareParticipants) {
        const ownerId = getVirtualScreenshareParticipantOwnerId(screenshare);

-        remoteParticipants.delete(ownerId);
-        remoteParticipants.delete(screenshare);
        speakers.delete(ownerId);
    }

+    // Calculate the number of slots available for active speakers and then sort them alphabetically to ensure
+    // consistent order.
+    const numberOfActiveSpeakerSlots
+        = visibleRemoteParticipants.size - screenShareParticipants.length - sharedVideos.length;
+    const activeSpeakersDisplayed = _takeFirstN(speakers, numberOfActiveSpeakerSlots)
+        .sort((a: string, b: string) => {
+            return (getParticipantById(state, a)?.name ?? defaultRemoteDisplayName)
+                .localeCompare(getParticipantById(state, b)?.name ?? defaultRemoteDisplayName);
+        });
+
    for (const sharedVideo of sharedVideos) {
        remoteParticipants.delete(sharedVideo);
    }
    for (const speaker of speakers.keys()) {
        remoteParticipants.delete(speaker);
    }
-
-    // Calculate the number of slots available for active speakers and then sort them alphabetically to ensure
-    // consistent order.
-    const numberOfActiveSpeakerSlots
-        = visibleRemoteParticipants.size - (screenShareParticipants.length * 2) - sharedVideos.length;
-    const activeSpeakersDisplayed = _takeFirstN(speakers, numberOfActiveSpeakerSlots)
-        .sort((a: string, b: string) => {
-            return (getParticipantById(state, a)?.name ?? defaultRemoteDisplayName)
-                .localeCompare(getParticipantById(state, b)?.name ?? defaultRemoteDisplayName);
-        });
-
    const participantsWithScreenShare = screenShareParticipants.reduce<string[]>((acc, screenshare) => {
        const ownerId = getVirtualScreenshareParticipantOwnerId(screenshare);

        acc.push(ownerId);
        acc.push(screenshare);
+        remoteParticipants.delete(ownerId);
+        remoteParticipants.delete(screenshare);

        return acc;
    }, []);
--- a/react/features/filmstrip/subscriber.any.ts
+++ b/react/features/filmstrip/subscriber.any.ts
@@ -1,3 +1,4 @@
+import { getParticipantById } from '../base/participants/functions';
 import StateListenerRegistry from '../base/redux/StateListenerRegistry';

 import { isFilmstripScrollVisible, updateRemoteParticipants } from './functions';
@@ -25,7 +26,15 @@ StateListenerRegistry.register(
 */
 StateListenerRegistry.register(
    /* selector */ state => state['features/base/participants'].dominantSpeaker,
-    /* listener */ (dominantSpeaker, store) => updateRemoteParticipants(store));
+    /* listener */ (dominantSpeaker, store) => {
+        const { visibleRemoteParticipants } = store.getState()['features/filmstrip'];
+        const dominant = getParticipantById(store.getState(), dominantSpeaker);
+
+        // Only update the remote participants if the dominant speaker is not currently visible.
+        if (!dominant?.local && !visibleRemoteParticipants.has(dominantSpeaker)) {
+            updateRemoteParticipants(store);
+        }
+    });

 /**
 * Listens for changes in the filmstrip scroll visibility.
--- a/tests/specs/media/activeSpeaker.spec.ts
+++ b/tests/specs/media/activeSpeaker.spec.ts
@@ -1,6 +1,6 @@
 import type { Participant } from '../../helpers/Participant';
 import { setTestProperties } from '../../helpers/TestProperties';
-import { ensureThreeParticipants } from '../../helpers/participants';
+import { checkForScreensharingTile, ensureThreeParticipants, hangupAllParticipants } from '../../helpers/participants';
 import { muteAudioAndCheck } from '../helpers/mute';

 setTestProperties(__filename, {
@@ -66,6 +66,16 @@ describe('Active speaker', () => {
        await p1.getToolbar().clickAudioUnmuteButton();
        await p1.waitForParticipantOnLargeVideo(p3EndpointId, 'P1 should see P3 (last remote speaker) on stage when local is dominant');

+        // Check that p2 and p3 tiles remain visible in filmstrip on p1
+        await p1.driver.$(`//span[@id='participant_${p2EndpointId}']`).waitForDisplayed({
+            timeout: 3_000,
+            timeoutMsg: 'P2 tile should remain visible in filmstrip during screenshare'
+        });
+        await p1.driver.$(`//span[@id='participant_${p3EndpointId}']`).waitForDisplayed({
+            timeout: 3_000,
+            timeoutMsg: 'P3 tile should remain visible in filmstrip during screenshare'
+        });
+
        await p1.getToolbar().clickAudioMuteButton();
        await p1.getToolbar().clickStopDesktopSharingButton();
    });
@@ -145,6 +155,108 @@ describe('Active speaker', () => {

        await p1.getToolbar().clickAudioMuteButton();
    });
+
+    it('testRemoteTilesVisibleWhenRemoteStartsScreenshare', async () => {
+        await hangupAllParticipants();
+        await ensureThreeParticipants();
+
+        const { p1, p2, p3 } = ctx;
+        const p2EndpointId = await p2.getEndpointId();
+        const p3EndpointId = await p3.getEndpointId();
+
+        // Exit tile view on p1
+        await p1.getToolbar().clickExitTileViewButton();
+        await p1.waitForTileViewDisplayed(true);
+
+        // Mute all participants
+        await muteAudioAndCheck(p1, p2);
+        await muteAudioAndCheck(p2, p1);
+        await muteAudioAndCheck(p3, p1);
+
+        // Verify both p2 and p3 are visible before screenshare
+        await p1.driver.$(`//span[@id='participant_${p2EndpointId}']`).waitForDisplayed({
+            timeout: 3_000,
+            timeoutMsg: 'P2 tile should be visible before screenshare'
+        });
+        await p1.driver.$(`//span[@id='participant_${p3EndpointId}']`).waitForDisplayed({
+            timeout: 3_000,
+            timeoutMsg: 'P3 tile should be visible before screenshare'
+        });
+
+        // Unmute p3 to make them dominant speaker.
+        await p3.getToolbar().clickAudioUnmuteButton();
+        await p1.waitForParticipantOnLargeVideo(
+            p3EndpointId,
+            'P3 should be dominant speaker on P1 before screenshare'
+        );
+        await p3.getToolbar().clickAudioMuteButton();
+
+        // p2 starts screensharing and becomes dominant speaker
+        await p2.getToolbar().clickDesktopSharingButton();
+        await p2.getToolbar().clickAudioUnmuteButton();
+
+        // Check screenshare tiles are created
+        await checkForScreensharingTile(p2, p1);
+        await checkForScreensharingTile(p2, p2);
+
+        // Wait for p2 to be on large video on p1
+        await p1.waitForParticipantOnLargeVideo(
+            `${p2EndpointId}-v1`,
+            'P2 screenshare should be on large video on P1',
+            10_000
+        );
+
+        // Verify that both p2 and p3 camera tiles remain visible in filmstrip
+        // Even though p2 is screensharing, their camera tile should still be visible
+        await p1.driver.$(`//span[@id='participant_${p2EndpointId}']`).waitForDisplayed({
+            timeout: 3_000,
+            timeoutMsg: 'P2 camera tile should remain visible in filmstrip during their screenshare'
+        });
+        await p1.driver.$(`//span[@id='participant_${p3EndpointId}']`).waitForDisplayed({
+            timeout: 3_000,
+            timeoutMsg: 'P3 tile should remain visible in filmstrip during P2 screenshare'
+        });
+    });
+
+    it('testFilmstripTilesWithMultipleScreenshares', async () => {
+        await hangupAllParticipants();
+        await ensureThreeParticipants();
+
+        const { p1, p2, p3 } = ctx;
+        const p1EndpointId = await p1.getEndpointId();
+        const p2EndpointId = await p2.getEndpointId();
+
+        // Exit tile view on all participants
+        await p1.getToolbar().clickExitTileViewButton();
+        await p1.waitForTileViewDisplayed(true);
+
+        // Mute all participants
+        await muteAudioAndCheck(p1, p2);
+        await muteAudioAndCheck(p2, p1);
+        await muteAudioAndCheck(p3, p1);
+
+        // p1 starts screensharing
+        await p1.getToolbar().clickDesktopSharingButton();
+        await checkForScreensharingTile(p1, p2);
+
+        // p2 also starts screensharing
+        await p2.getToolbar().clickDesktopSharingButton();
+        await checkForScreensharingTile(p2, p1);
+
+        // Verify all camera tiles are still visible on p3's view
+        await p3.driver.$(`//span[@id='participant_${p1EndpointId}']`).waitForDisplayed({
+            timeout: 3_000,
+            timeoutMsg: 'P1 camera tile should be visible on P3 during multiple screenshares'
+        });
+        await p3.driver.$(`//span[@id='participant_${p2EndpointId}']`).waitForDisplayed({
+            timeout: 3_000,
+            timeoutMsg: 'P2 camera tile should be visible on P3 during multiple screenshares'
+        });
+
+        // Verify screenshare tiles are present
+        await checkForScreensharingTile(p1, p3);
+        await checkForScreensharingTile(p2, p3);
+    });
 });

 /**
--- a/tests/specs/media/desktopSharing.spec.ts
+++ b/tests/specs/media/desktopSharing.spec.ts
@@ -67,7 +67,21 @@ describe('Desktop sharing', () => {
        // Check if a remote screen share tile is created on all participants.
        await checkForScreensharingTile(p2, p1);
        await checkForScreensharingTile(p2, p2);
-        await checkForScreensharingTile(p2, p2);
+        await checkForScreensharingTile(p2, p3);
+
+        // Verify that all participant camera tiles remain visible in filmstrip
+        const p2EndpointId = await p2.getEndpointId();
+        const p3EndpointId = await p3.getEndpointId();
+
+        // Check from p1's perspective that camera tiles are visible
+        await p1.driver.$(`//span[@id='participant_${p2EndpointId}']`).waitForDisplayed({
+            timeout: 3_000,
+            timeoutMsg: 'P2 camera tile should be visible on P1 during screenshare'
+        });
+        await p1.driver.$(`//span[@id='participant_${p3EndpointId}']`).waitForDisplayed({
+            timeout: 3_000,
+            timeoutMsg: 'P3 camera tile should be visible on P1 during P2 screenshare'
+        });

        expect(await p3.execute(() => JitsiMeetJS.app.testing.isLargeVideoReceived())).toBe(true);
    });