[AWAITING CHECKLIST][Due for payment 2025-02-18] [$250] Web - Attachments - Incorrect file format is downloaded when downloading video from preview

[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: v9.0.88-4
Reproducible in staging?: Yes
Reproducible in production?: Yes
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/5488354
Issue reported by: Applause Internal Team
Device used: Windows 11/ Chrome
App Component: Chat Report View

Action Performed:

1. Sign in to staging.new.expensify.com
2. Navigate to a conversation that has a video in the conversation history (An mp4 video)
3. Click on three dots on the video
4. Click on "Download" from the option.

Expected Result:

Video with an .mp4 format is downloaded.

Actual Result:

Downloaded video is not in .mp4 format.

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Bug6720001_1737474016295.bandicam_2025-01-21_18-31-17-703.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021881748185068126898
• Upwork Job ID: 1881748185068126898
• Last Price Increase: 2025-01-21
• Automatic offers:
• hellohublot | Contributor | 105874356

Issue Owner

Current Issue Owner: @garrettmknight

[melvin-bot]

Triggered auto assignment to @JmillsExpensify (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.

[Shahidullah-Muffakir]

Proposal

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

Incorrect file format is downloaded when downloading video from preview

What is the root cause of that problem?

1. When downloading a file we are not passing the file name to the fileDownload here:
   

   App/src/components/VideoPlayerContexts/VideoPopoverMenuContext.tsx

   Line 39 in 191eb45

   fileDownload(source.uri);

2. This issue occurs for all video formats not just mp4

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

1. we already have logic for extracting file name here:
   

   App/src/libs/fileDownload/getAttachmentDetails.ts

   Line 8 in 191eb45

   const getAttachmentDetails: GetAttachmentDetails = (html) => {

2. we can separate the filename logic from the above function so we can use it anywhere we need as:

Refactored function

**
 * Extract the original filename from the HTML based on tag type.
 */
const getOriginalFileName = (html: string, isImageTag: boolean): string | null => {
    const ORIGINAL_FILENAME_REGEX = isImageTag ? new RegExp(`${CONST.ATTACHMENT_ORIGINAL_FILENAME_ATTRIBUTE}*=*"(.+?)"`, 'i') : new RegExp('<(?:a|video)[^>]*>([^<]+)</(?:a|video)>', 'i');

    return html.match(ORIGINAL_FILENAME_REGEX)?.[1] ?? null;
};

/**
 * Extract the thumbnail URL, source URL from the HTML.
 */
const getAttachmentDetails: GetAttachmentDetails = (html) => {
    if (!html) {
        return {
            previewSourceURL: null,
            sourceURL: null,
            originalFileName: null,
        };
    }

    // Files can be rendered either as anchor tag or as an image
    const IS_IMAGE_TAG = /<img([\w\W]+?)\/>/i.test(html);
    const PREVIEW_SOURCE_REGEX = new RegExp(`${CONST.ATTACHMENT_PREVIEW_ATTRIBUTE}*=*"(.+?)"`, 'i');
    const SOURCE_REGEX = new RegExp(`${CONST.ATTACHMENT_SOURCE_ATTRIBUTE}*=*"(.+?)"`, 'i');

    // Files created/uploaded/hosted by App should resolve from API ROOT. Other URLs aren't modified
    const sourceURL = tryResolveUrlFromApiRoot(html.match(SOURCE_REGEX)?.[1] ?? '');
    const imageURL = IS_IMAGE_TAG ? tryResolveUrlFromApiRoot(html.match(PREVIEW_SOURCE_REGEX)?.[1] ?? '') : null;
    const previewSourceURL = IS_IMAGE_TAG ? imageURL : sourceURL;
    const originalFileName = getOriginalFileName(html, IS_IMAGE_TAG);

    return {
        previewSourceURL,
        sourceURL,
        originalFileName,
    };
};

3. then here

   App/src/components/VideoPlayerContexts/VideoPopoverMenuContext.tsx

   Line 39 in 191eb45

   fileDownload(source.uri);

Get the file name and pass it to the fileDownload:

        const filename = getOriginalFileName(source.uri, false);
        fileDownload(source.uri, filename);

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

NA

What alternative solutions did you explore? (Optional)

NA

new.video.jan45.mov

[melvin-bot]

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

[melvin-bot]

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

[bernhardoj]

Proposal

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

The downloaded video doesn't have a correct format.

What is the root cause of that problem?

When we add a new video, it's still uploading. The video we see is a local file. When we press download, it will download based on the local file uri (blob://...).

App/src/components/VideoPlayerContexts/VideoPopoverMenuContext.tsx

Line 39 in 0190b0a

fileDownload(source.uri);

We already have a logic to hide the Download button if it's a local file.

App/src/components/VideoPlayerContexts/VideoPopoverMenuContext.tsx

Lines 45 to 51 in 0190b0a

	if (!isOffline && !isLocalFile) {
	// eslint-disable-next-line react-compiler/react-compiler
	items.push({
	icon: Expensicons.Download,
	text: translate('common.download'),
	onSelected: () => {
	downloadAttachment();

But the isLocalFile checks for the currently playing URL, which is null because no video is currently playing.

App/src/components/VideoPlayerContexts/VideoPopoverMenuContext.tsx

Line 20 in 0190b0a

const isLocalFile = currentlyPlayingURL && CONST.ATTACHMENT_LOCAL_URL_PREFIX.some((prefix) => currentlyPlayingURL.startsWith(prefix));

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

Instead of using the currently playing URL, we should use the URL of the video that we open the popover menu. We will set the URL when opening the popover, just like we did with the playback rate.

App/src/components/VideoPlayer/BaseVideoPlayer.tsx

Lines 155 to 165 in 0190b0a

	const showPopoverMenu = (event?: GestureResponderEvent | KeyboardEvent) => {
	videoPopoverMenuPlayerRef.current = videoPlayerRef.current;
	videoPlayerRef.current?.getStatusAsync().then((status) => {
	if (!('rate' in status && status.rate)) {
	return;
	}
	if (shouldUseNewRate) {
	setCurrentPlaybackSpeed(status.rate as PlaybackSpeed);
	}
	setIsPopoverVisible(true);
	});

const showPopoverMenu = (event?: GestureResponderEvent | KeyboardEvent) => {
    ...
    setPopoverMenuSource(url);

setPopoverMenuSource is a set state function from VideoPopoverMenuContext.

We need to add the new source state here and update the isLocalFile to use the new source state.

App/src/components/VideoPlayerContexts/VideoPopoverMenuContext.tsx

Lines 15 to 20 in 0190b0a

	function VideoPopoverMenuContextProvider({children}: ChildrenProps) {
	const {currentlyPlayingURL} = usePlaybackContext();
	const {translate} = useLocalize();
	const [currentPlaybackSpeed, setCurrentPlaybackSpeed] = useState<PlaybackSpeed>(CONST.VIDEO_PLAYER.PLAYBACK_SPEEDS[3]);
	const {isOffline} = useNetwork();
	const isLocalFile = currentlyPlayingURL && CONST.ATTACHMENT_LOCAL_URL_PREFIX.some((prefix) => currentlyPlayingURL.startsWith(prefix));

const [source, setSource] = useState('');
const isLocalFile = source && CONST.ATTACHMENT_LOCAL_URL_PREFIX.some((prefix) => source.startsWith(prefix));

App/src/components/VideoPlayerContexts/VideoPopoverMenuContext.tsx

Lines 35 to 39 in 0190b0a

	const {source} = videoPopoverMenuPlayerRef.current?.props ?? {};
	if (typeof source === 'number' || !source) {
	return;
	}
	fileDownload(source.uri);

Then, when downloading the file, we use the source state and add encrypted auth token to it.

const downloadAttachment = useCallback(() => {
    if (typeof source === 'number' || !source) {
        return;
    }
    fileDownload(addEncryptedAuthTokenToURL(source));
}, [source]);

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

N/A

[hellohublot]

Proposal

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

Web - Attachments - Incorrect file format is downloaded when downloading video from preview

What is the root cause of that problem?

When I added a console.log(url, sourceURL) in BaseVideoPlayer, I found that different results are generated. This happens because the url changes from blob to https://mp4 after being uploaded to the server, but the sourceURL remains as blob.

This issue occurs because we used useState instead of useMemo, so sourceURL does not update when url changes.

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

1. • We should modify the BaseVideoPlayer by replacing const [sourceURL] = useState with const sourceURL = useMemo(). This will ensure that after the video is uploaded to the server, we can retrieve the correct sourceURL and video name.
2. • Additionally, before the upload is complete, we need to hide the "Download" option in the VideoPopoverMenuContextProvider (this is reasonable because the upload is not yet successful, so downloading is not possible. Users can still find the file on their local hard drive, And when the upload is not complete, we already hide the Download option in ContextMenuActions. Therefore, to maintain consistency, we should hide the Download option when the video upload is not yet complete).

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

None

What alternative solutions did you explore? (Optional)

None

[Kalydosos]

Proposal

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

Incorrect file format is downloaded when downloading video from preview. This happens only when the user that has submitted the video did not reload the chat yet.

What is the root cause of that problem?

The VideoPlayer (more precisely VideoPopoverMenuContext) is missing the video extension in the video file name and thus the downloaded file does not contain the file format extension

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

We should propagate the video file format extension from the original file name to the video player download feature using the PlaybackContext.

We retrieve the extension by adding the following code in

src/components/HTMLEngineProvider/HTMLRenderers/VideoRenderer.tsx

const fileExtension = FileUtils.splitExtensionFromFileName(htmlAttribs[CONST.ATTACHMENT_ORIGINAL_FILENAME_ATTRIBUTE] ?? '').fileExtension;
let fileName = FileUtils.getFileName(${sourceURL});
if (!fileName.endsWith(fileExtension)){
fileName = ${fileName}.${fileExtension};
}

The fileName passed below will then have the file format extension*

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

Lines 36 to 45 in 4ccb6ec

	<VideoPlayerPreview
	key={key}
	videoUrl={sourceURL}
	reportID={currentReportIDValue?.currentReportID ?? '-1'}
	fileName={fileName}
	thumbnailUrl={thumbnailUrl}
	videoDimensions={{width, height}}
	videoDuration={duration}
	isDeleted={isDeleted}
	onShowModalPress={() => {

The extension is then retrieved and passed to VideoPlayer here

App/src/components/VideoPlayerPreview/index.tsx

Line 86 in 4ccb6ec

<VideoPlayer

as

<VideoPlayer
url={videoUrl}
videoExtension={fileExtension}

and then added to PlaybackContext

App/src/components/VideoPlayerContexts/types.ts

Line 10 in 4ccb6ec

type PlaybackContext = {

and setted by BaseVideoPlayer as

useEffect(() => {
setVideoFileExtension(videoExtension);
}, []);

the extension is then retrieved by VideoPopoverMenuContext through its call to PlaybackContext from

App/src/components/VideoPlayerContexts/VideoPopoverMenuContext.tsx

Line 16 in 4ccb6ec

const {currentlyPlayingURL} = usePlaybackContext();

into

const {currentlyPlayingURL, videoFileExtension} = usePlaybackContext();

and then passed to its downloadAttachment function if necessary by changing the following line

App/src/components/VideoPlayerContexts/VideoPopoverMenuContext.tsx

Line 39 in 4ccb6ec

fileDownload(source.uri);

into

const fileName = FileUtils.getFileName(source.uri);
fileDownload(source.uri, fileName.endsWith(videoFileExtension??'')?fileName:${fileName}.${videoFileExtension});

RESULT

2025-01-23.16-21-15.mp4

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

None

What alternative solutions did you explore? (Optional)

None

[melvin-bot]

@JmillsExpensify, @sobitneupane Huh... This is 4 days overdue. Who can take care of this?

[sobitneupane]

Thanks for the proposal, everyone.

Proposal from @hellohublot looks good to me.

🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[melvin-bot]

📣 @hellohublot 🎉 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 📖

[hellohublot]

@sobitneupane @Gonals

❤️ Thank you very much! However, after further investigation, I found that syncing the value of sourceURL causes a loading effect, which I think makes the UX feel odd, so I think it's unacceptable.

ux.mp4

Maybe this proposal would be more appropriate: #55541 (comment).

Waiting for CPLUS to determine the better proposal moving forward 🙏

[sobitneupane]

Thanks @hellohublot

The root cause of the issue is the use of outdated videoPopoverMenuPlayerRef which contains reference to the local file. If we sync the sourceURL (and in turn videoPopoverMenuPlayerRef) to use the server url, the video reloads as mentioned here.

App/src/components/VideoPlayerContexts/VideoPopoverMenuContext.tsx

Lines 35 to 39 in 0190b0a

	const {source} = videoPopoverMenuPlayerRef.current?.props ?? {};
	if (typeof source === 'number' || !source) {
	return;
	}
	fileDownload(source.uri);

It is possible to avoid this situation by using the local file for playback while using the server URL for downloading, as proposed by @bernhardoj. Let's go with the @bernhardoj's proposal

cc: @Gonals

[Gonals]

Sounds good!

[bernhardoj]

PR is ready

cc: @sobitneupane

[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.0.95-6 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix download option is shown for local file #56294

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

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

• @hellohublot requires payment automatic offer (Contributor)
• @sobitneupane requires payment through NewDot Manual Requests
• @bernhardoj requires payment through NewDot Manual Requests

[melvin-bot]

@sobitneupane @JmillsExpensify @sobitneupane 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]

[garrettmknight]

Taking over for payment. @sobitneupane can you complete the checklist today?

[melvin-bot]

@garrettmknight @Gonals @hellohublot @sobitneupane @bernhardoj this issue is now 4 weeks old, please consider:

• Finding a contributor to fix the bug
• Closing the issue if BZ has been unable to add the issue to a VIP or Wave project
• If you have any questions, don't hesitate to start a discussion in #expensify-open-source

Thanks!

[garrettmknight]

Payment Summary:

• Contributor: @bernhardoj $250 via NewDot - go ahead and request
• Contributor+: @sobitneupane $250 via NewDot

[bernhardoj]

Requested in ND.

[sobitneupane]

Regression Test Proposal:

1. Upload a video attachment to a chat.
2. Press the three-dot menu on the video. (On small screens, press the play button on the thumbnail first to open the attachment page.)
3. If the Download button is not visible, the video is still uploading. Wait for it to finish uploading.
4. Once the upload is complete, the Download button should appear.
5. Press Download.
6. After the video is downloaded, play it from the local storage.
7. Verify that the video plays without any issues.

Do we agree 👍 or 👎

[sobitneupane]

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: Incorrect video downloaded #40647 #41054 (comment)

• [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:

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

• [BugZero Assignee] Create a GH issue for creating/updating the regression test once above steps have been agreed upon.

  Link to issue:

[JmillsExpensify]

$250 approved for @bernhardoj

[garrettmknight]

Payment summary here

@sobitneupane go ahead and request.