[$250] Reports - Video sound is playing simultaneously on Reports page and full screen view

[IuliiaHerets]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: 9.0.76-6
Reproducible in staging?: Yes
Reproducible in production?: Yes
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: N/A
If this was caught during regression testing, add the test name, ID and link from TestRail: Exp
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause Internal Team
Device used: Mac 15.0 / Chrome
App Component: Search

Action Performed:

1. Go to staging.new.expensify.com
2. Upload a video with sound to any chat.
3. Go to Reports > Chats.
4. Click on the video to play it (do not play in full screen yet), enable the audio if disabled.
5. After a while, click on the full screen icon.
6. Note that the video sound is playing simultaneously in Report page and also in full screen view.
7. Close the full screen view.
8. Click Inbox.
9. Click Reports.
10. Note that the video sound is still playing in Step 8 and 9 after switching tab.

Expected Result:

In Step 6, when the video is played in full screen, the video will not play simultaneously in Report page and also in full screen view.
In Step 8 and 9, the video will stop playing after switching tab.

Actual Result:

In Step 6, when the video is played in full screen, the video plays simultaneously in Report page and also in full screen view.
In Step 8 and 9, the video is still playing after switching tab.

Workaround:

Unknown

Platforms:

• Android: Standalone
• Android: HybridApp
• Android: mWeb Chrome
• iOS: Standalone
• iOS: HybridApp
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Bug6694737_1734253452793.Screen_Recording_2024-12-15_at_16.56.10.mov

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021868517100126911249
• Upwork Job ID: 1868517100126911249
• Last Price Increase: 2024-12-23
• Automatic offers:
• suneox | Reviewer | 105453253
• mkzie2 | Contributor | 105453254

Issue Owner

Current Issue Owner: @suneox

[melvin-bot]

Triggered auto assignment to @VictoriaExpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~021868517100126911249

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @suneox (External)

[mkzie2]

Edited by proposal-police: This proposal was edited at 2024-12-18 09:43:24 UTC.

Proposal

Please re-state the problem that we are trying to solve in this issue.

In Step 6, when the video is played in full screen, the video plays simultaneously in Report page and also in full screen view.
In Step 8 and 9, the video is still playing after switching tab.

What is the root cause of that problem?

1. When we play a video on the search page, a videoPlayerRef is initialized. After we click on expand, isUsedInCarousel is false for the video on the search page then another videoPlayerRef is initialized again then we can see the video is started again.

App/src/components/Attachments/AttachmentView/index.tsx

Lines 293 to 295 in 653af04

	shouldUseSharedVideoElement={isUsedInCarousel}
	isHovered={isHovered}
	duration={duration}

2. When we change from inbox to search page or search page to inbox,

currentReportIDValue?.currentReportID is not updated at the time the `VideoPlayerPreview` is mounted

App/src/components/HTMLEngineProvider/HTMLRenderers/VideoRenderer.tsx

Line 28 in b6ebedb

const currentReportIDValue = useCurrentReportID();

App/src/components/HTMLEngineProvider/HTMLRenderers/VideoRenderer.tsx

Line 39 in b6ebedb

reportID={currentReportIDValue?.currentReportID ?? '-1'}

Then isThumnail is false which causes the VideoPlayer to be rendered in both places and it shares the element here

App/src/components/VideoPlayerPreview/index.tsx

Lines 69 to 72 in b6ebedb

	if (videoUrl !== currentlyPlayingURL || reportID !== currentlyPlayingURLReportID) {
	return;
	}
	setIsThumbnail(false);

And here we don't video player data when we change from inbox to search page and vice versa

App/src/components/VideoPlayerContexts/PlaybackContext.tsx

Lines 103 to 106 in b6ebedb

	if (!currentReportID || !prevCurrentReportID || currentReportID === '-1' || prevCurrentReportID === '-1' || currentReportID === prevCurrentReportID) {
	return;
	}
	resetVideoPlayerData();

What changes do you think we should make in order to solve the problem?

1. We should always pass shouldUseSharedVideoElement as true if the user is on the large screen. We can remove shouldUseSharedVideoElement prop.

shouldUseSharedVideoElement={!shouldUseNarrowLayout} 

App/src/components/Attachments/AttachmentView/AttachmentViewVideo/index.tsx

Line 21 in 653af04

shouldUseSharedVideoElement={shouldUseSharedVideoElement && !shouldUseNarrowLayout}

2. We should use report?.reportID instead of currentReportIDValue?.currentReportID

App/src/components/HTMLEngineProvider/HTMLRenderers/VideoRenderer.tsx

Line 28 in b6ebedb

const currentReportIDValue = useCurrentReportID();

3. If the currentReportID is -1 and we're in the search page, we should reset the player data

useEffect(() => {
    // This logic ensures that resetVideoPlayerData is only called when currentReportID
    // changes from one valid value (i.e., not an empty string or '-1') to another valid value.
    // This prevents the video that plays when the app opens from being interrupted when currentReportID
    // is initially empty or '-1', or when it changes from empty/'-1' to another value
    // after the report screen in the central pane is mounted on the large screen.
    if (
        !currentReportID ||
        !prevCurrentReportID ||
        (currentReportID === '-1' && !isSearchTopmostCentralPane()) ||
        (prevCurrentReportID === '-1' && isSearchTopmostCentralPane()) ||
        currentReportID === prevCurrentReportID
    ) {
        return;
    }
    resetVideoPlayerData();
}, [currentReportID, prevCurrentReportID, resetVideoPlayerData]);

App/src/components/VideoPlayerContexts/PlaybackContext.tsx

Lines 103 to 106 in b6ebedb

	if (!currentReportID || !prevCurrentReportID || currentReportID === '-1' || prevCurrentReportID === '-1' || currentReportID === prevCurrentReportID) {
	return;
	}
	resetVideoPlayerData();

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

What alternative solutions did you explore? (Optional)

NA

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

[kaushiktd]

Please re-state the problem that we are trying to solve in this issue.

Reports - Video sound is playing simultaneously on Reports page and full screen view

What is the root cause of that problem?

1. Unmounting of Video Component:

The video playback component lacked integration with the navigation lifecycle of the application. React Navigation provides hooks such as useFocusEffect or useIsFocused to determine whether the current screen is active (focused) or inactive (unfocused). Without leveraging these hooks, the app has no way to detect when the user navigates away from a screen containing an active video. As a result:

The video component continued playing in the background, even when the screen was no longer visible.
In this specific case, when the user switched to another tab (e.g., Step 8 → Step 9), the app didn’t notify the video component to pause, leading to the playback continuing in the background.
2. Lack of State Management:

Without proper management of the playback state, the video may continue playing in the background or not resume correctly when the user returns to the screen.

What changes do you think we should make in order to solve the problem?

Use useFocusEffect:

Implement the useFocusEffect hook to manage video playback based on the screen's focus state. This ensures that the video plays when the screen is focused and pauses when it loses focus.

change in file

/Users/techdoodles/Downloads/App-main/src/components/VideoPlayer/BaseVideoPlayer.tsx

import { useFocusEffect } from '@react-navigation/native';

useFocusEffect(
    useCallback(() => {
        if (shouldPlay) {
            playVideo(); // Start playing the video if it should play
        } else {
            pauseVideo(); // Pause the video if it should not play
        }

        return () => {
            pauseVideo(); // Ensure the video is paused when leaving the screen
        };
    }, [shouldPlay, playVideo, pauseVideo])
);

video:-

https://www.loom.com/share/265b1fbca8404ad1a0fcb31a1abb32c3?sid=50985390-7393-442f-93e4-2fff0e737c03

[suneox]

Thanks for all the proposals. Before diving into the detailed solutions, we should clarify the RCA and ensure the solution addresses the issue comprehensively.

After we click on expand, isUsedInCarousel is false for the video on the search page then another videoPlayerRef is initialized again then we can see the video is started again.

@mkzie2 Could you please clarify which part of the code is responsible for initializing another videoPlayerRef, leading to the video restarting? Additionally, based on your solution, it seems the video still plays when switching to another route (Inbox, Setting page)

1. Unmounting of Video Component:
2. Lack of State Management

@kaushiktd We should provide a more detailed RCA with references to the relevant code
Additionally, your solution also prevents the video from playing when opening the filter, so we have to confirm this behavior

[kaushiktd]

@suneox
For deatailed RCA i updated my proposal

When we click on the filter option, the URL changes, causing a focus change, and the video stops playing.

Do we need the video to continue playing when the filter is opened?

[mkzie2]

@suneox I updated my proposal.

[melvin-bot]

@suneox, @VictoriaExpensify Eep! 4 days overdue now. Issues have feelings too...

[suneox]

Thanks for all the updates. I'll review the new changes today.

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[suneox]

@mkzie2 proposal LGTM

🎀 👀 🎀 C+ reviewed

[melvin-bot]

Triggered auto assignment to @aldo-expensify, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[melvin-bot]

📣 @suneox 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[melvin-bot]

📣 @mkzie2 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[mkzie2]

@suneox The PR is ready.