[Due for payment 2025-02-28] [$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 contributors@expensify.com 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): applausetester+kh1311020@applause.expensifail.com
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: @VictoriaExpensify

[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.

[melvin-bot]

This issue has not been updated in over 15 days. @suneox, @VictoriaExpensify, @aldo-expensify, @mkzie2 eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

[suneox]

We are still working on an edge case in the PR.

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.1.3-4 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• fix video does not stop playing when going to inbox from search #54552

If no regressions arise, payment will be issued on 2025-02-28. 🎊

For reference, here are some details about the assignees on this issue:

• @suneox requires payment automatic offer (Reviewer)
• @mkzie2 requires payment automatic offer (Contributor)

[melvin-bot]

@suneox @VictoriaExpensify @suneox The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

[suneox]

BugZero Checklist:

• [Contributor] Classify the bug:

Bug classification

Source of bug:

• 1a. Result of the original design (eg. a case wasn't considered)
• 1b. Mistake during implementation
• 1c. Backend bug
• 1z. Other:

Where bug was reported:

• 2a. Reported on production (eg. bug slipped through the normal regression and PR testing process on staging)
• 2b. Reported on staging (eg. found during regression or PR testing)
• 2d. Reported on a PR
• 2z. Other:

Who reported the bug:

• 3a. Expensify user
• 3b. Expensify employee
• 3c. Contributor
• 3d. QA
• 3z. Other:

• [Contributor] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake.

  Link to comment: https://github.com/Expensify/App/pull/40132/files#r1976537901

• [Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner.

  Link to discussion: N/A

• [Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again.

Regression Test Proposal

Precondition:

• Login with any account.

Test:

1. Upload a video with sound to any chat.
2. Navigate to Reports > Chats.
3. Click on the video to play it (do not switch to full screen yet) and enable audio if disabled.
4. Let the video play for a while, then click on the full screen icon.
5. Verify that the video sound is not playing simultaneously in both the Report page and the full screen view.
6. Close the full screen view.
7. Click on Inbox.
8. Verify that the video has stopped playing.

Do we agree 👍 or 👎

[VictoriaExpensify]

Payment summary:
C: @mkzie2 paid $250 via Upwork

@suneox I have sent you an offer in Upwork. Please accept it and I'll process your payment.

Thanks!

[VictoriaExpensify]

Raised a request to add this to test rail - https://github.com/Expensify/Expensify/issues/477097

[suneox]

@VictoriaExpensify Thank you! I have accepted the offer.

[VictoriaExpensify]

Thanks @suneox !

Updated payment summary:
C: @mkzie2 paid $250 via Upwork
C+: @suneox paid $250 via Upwork

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.