[$250] Attachment - Video orientation changes after editing a message

[applause-bot]

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: v9.4.17-0
Reproducible in staging?: Yes
Reproducible in production?: Yes
If this was caught during regression testing, add the test name, ID and link from BrowserStack: #93971
Email or phone of affected tester (no customers): n/a
Issue reported by: Applause Internal Team
Bug source: Exploratory - Significant User Experience Deterioration
Device used: Mav 15.4.1 Safari, Chrome
App Component: Chat Report View

Action Performed:

1. Go to https://staging.new.expensify.com
2. Go to any chat
3. Upload a video in portrait orientation
4. Edit that message and save.

Expected Result:

The video orientation should remain unchanged after editing a message

Actual Result:

Video orientation changes after editing a message

Workaround:

Unknown

Platforms:

• Android: App
• Android: mWeb Chrome
• iOS: App
• iOS: mWeb Safari
• iOS: mWeb Chrome
• Windows: Chrome
• MacOS: Chrome Safari

Screenshots/Videos

Bug7188138_1782176719190.Recording__2451.mp4

View all open jobs on GitHub

Applause Internal Information
Exported by: Murugesan Natarajan
Bug ID: 7188138

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~022069471607528210960
• Upwork Job ID: 2069471607528210960
• Last Price Increase: 2026-07-07

[applause-bot]

While Applause doesn't embed attachments for this bug, here are the links for quick view:
https://platform.applause.com/services/links/v1/external/b2cf7491b1c538d351672a70398027b8733ef07e8346cc7b69998b758598efcd

[MelvinBot]

Proposal / Analysis

Problem

Editing (and saving) a chat message that contains an uploaded portrait video flips the video to landscape. Reproducible on web (Chrome/Safari), staging + production, v9.4.17-0.

Root Cause

A video's display orientation comes from its data-expensify-width / data-expensify-height attributes. Editing a message round-trips its HTML through Markdown and back to HTML — but the Markdown form of a video is only ![name](url.mp4), which cannot encode width/height. Those attributes survive an edit only via a side-channel videoAttributeCache keyed by the video source URL.

When that cache lookup misses at save time, the rebuilt <video> tag has no width/height. VideoRenderer.tsx:23-24 then computes Number(undefined) → NaN, and useThumbnailDimensions.ts:19-20 falls back to NAN_ASPECT_RATIO: 1.5 — a 3:2 landscape ratio. So a portrait video renders landscape.

This is a regression in the videoAttributeCache round-trip wiring (the area touched by #93971). The most likely trigger is a key mismatch: the cache is keyed by the HTML-encoded video source captured during htmlToMarkdown, while the markdown→html rebuild looks it up using an escaped/decoded source. For real attachment URLs (which contain & in query params/auth tokens), the keys diverge, the lookup misses, and dimensions are dropped — which is exactly why PR #93971's regression test passed (it used a clean URL https://example.com/video.mp4 with no special characters).

Evidence chain (file:line)

• Markdown can't hold dimensions; cache is the only preservation path. editReportComment rebuilds the edited HTML and threads the cache through: Report/index.ts:3199-3206 and :3242.
• Cache populated on entering edit, keyed by video source: useDraftMessageVideoAttributeCache.ts:31-37; forwarded on save in useEditMessage.ts:92-103.
• Serialization & lookup logic lives in expensify-common@2.0.181 ExpensiMark — the html→markdown video rule caches (videoSource, attrs) and the markdown→html rule re-attaches attrCache[videoSource]. The encode/decode/escape ordering between these two is where the key desyncs for URLs containing &.
• Downstream fallback to landscape: VideoRenderer → NaN width/height → useThumbnailDimensions → NAN_ASPECT_RATIO: 1.5 (all linked above). On web the player can partially self-heal via onloadedmetadata re-measurement (VideoPlayerPreview/index.tsx), but the initial/thumbnail aspect ratio is already wrong, which is what the tester observed.

Recommended Fix

Make the video-attribute cache lookup encoding-insensitive so the key can't desync across the markdown↔html round trip — e.g. normalize the key (htmlDecode / sanitizeURL) on both the cacheVideoAttributes setter and the attrCache[videoSource] lookup. The canonical matching logic is in Expensify/expensify-common (ExpensiMark video rules), so the robust fix likely requires a change there (then a version bump in App). An App-side mitigation in the cache wiring (useDraftMessageVideoAttributeCache.ts / editReportComment) may also be possible.

Regardless of the chosen path, the regression test should be strengthened to use a video URL containing &/query params and assert the rebuilt HTML still contains data-expensify-width/data-expensify-height.

⚠️ One thing to confirm before implementing: that the cache miss is caused by the &-encoding desync (vs. the module-singleton cache being cleared/clobbered by a concurrent mount). Both produce the identical NaN→landscape symptom. Inspecting a real saved Edit_Comment payload for a portrait video — does the post-edit HTML drop data-expensify-width/-height, and does the source URL contain &? — would settle it.

Related issues

None of the open issues matched this exact regression (the "orientation/landscape" matches were unrelated layout/a11y bugs).

---

Next Steps for Contributor+ team:
To accept: @MelvinBot implement [this](https://github.com/Expensify/App/issues/94341) to create a draft PR.
To refine: @MelvinBot <your feedback>
To reject: Explain why you are rejecting Melvin's proposal.

[MobileMage]

🚨 Edited by proposal-police: This proposal was edited at 2026-06-23 17:24:43 UTC.

Proposal

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

When a chat message that contains a portrait (e.g. 1080×1920) video attachment is edited and saved, the video re-renders with the wrong orientation/aspect ratio — a portrait clip is shown in a landscape (≈1.5) frame. The video itself is unchanged; only its on-screen dimensions flip after the edit round-trip. This reproduces on web (Chrome/Safari) and is independent of the actual edit text.

What is the root cause of that problem?

The orientation of a video preview is driven entirely by the data-expensify-width / data-expensify-height attributes that live on the stored <video> HTML. VideoRenderer reads them into videoDimensions={{width, height}} and hands them to useThumbnailDimensions. When either value is missing/NaN, useThumbnailDimensions does not fall back to the real ratio — it returns a fixed landscape ratio:

if (!width || !height) {
    return {width: fixedDimension, aspectRatio: CONST.THUMBNAIL_IMAGE.NAN_ASPECT_RATIO}; // 1.5 → landscape
}

App/src/hooks/useThumbnailDimensions.ts

Lines 18 to 21 in bad4883

	const thumbnailDimensionsStyles = useMemo(() => {
	if (!width || !height) {
	return {width: fixedDimension, aspectRatio: CONST.THUMBNAIL_IMAGE.NAN_ASPECT_RATIO};
	}

So a portrait video that loses its width/height ends up rendered at 1.5:1. The question is why the edit drops those attributes. The edit write-path is editReportComment → handleUserDeletedLinksInHtml, which re-serializes the edited draft markdown back to HTML via getParsedMessageWithShortMentions with extras: {videoAttributeCache}. The draft markdown only carries ![name](source) — the dimensions are NOT in the markdown. They are re-attached purely by an in-memory cache lookup keyed on the video source string:

App/src/libs/actions/Report/index.ts

Lines 3199 to 3206 in bad4883

	const htmlForNewComment = getParsedMessageWithShortMentions({
	text: newCommentText,
	userEmailDomain,
	availableMentionLogins: allPersonalDetailLogins,
	parserOptions: {
	extras: {videoAttributeCache},
	},
	});

I reproduced the full round-trip with the actual expensify-common parser. When the cache lookup misses (empty cache, or the draft's source string doesn't exactly match the key captured from data-expensify-source), the parser emits a <video> tag with no dimension attributes: <video data-expensify-source="…/w_hash.mp4" >Recording.mp4</video>. The edited action's HTML is then persisted without data-expensify-width/height, so on every subsequent render videoDimensions is {NaN, NaN} and the portrait clip is forced to the landscape NAN_ASPECT_RATIO. The cache is fragile because the value is re-built per edit by useDraftMessageVideoAttributeCache keyed on the raw source, and any divergence between that key and the source the parser sees in the draft markdown (query-string sources like ?encryptedAuthToken=…, HTML-entity escaping of & → & under shouldEscapeText: true, or a not-yet-repopulated cache) silently drops the attributes. (Note: PR #93971 is the regression-test PR, but it only swapped allPersonalDetails for a threaded personalDetails arg in this same function — it did not touch the video-attribute path, and this bug reproduces in production where #93971 isn't shipped, so the cache fragility is pre-existing.)

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

Make the write-path resilient so the edit can never strip dimensions that were already present on the original action. In editReportComment (src/libs/actions/Report/index.ts), we already have the original action HTML (originalCommentHTML from getReportActionHtml(originalReportAction)). Before building optimisticReportActions, re-apply any data-expensify-width/data-expensify-height/data-expensify-thumbnail-url/data-expensify-duration from the original <video> (matched by data-expensify-source) onto the freshly parsed htmlForNewComment for any <video> that lost them. Concretely, seed the cache from the original HTML as the source of truth rather than relying solely on the transient module-level draftMessageVideoAttributeCache:

// Build a guaranteed cache from the persisted action HTML, then merge the draft cache on top
const fallbackVideoCache: Record<string, string> = {};
Parser.htmlToMarkdown(originalCommentHTML ?? '', {cacheVideoAttributes: (src, attrs) => (fallbackVideoCache[src] = attrs)});
const mergedCache = {...fallbackVideoCache, ...videoAttributeCache};

Passing mergedCache into handleUserDeletedLinksInHtml guarantees the dimension attributes are re-attached from the original action even when the draft cache is empty or keyed slightly differently, so the saved HTML keeps data-expensify-width/height and the portrait ratio is preserved. This is safe because it only adds attributes that already existed on the original action keyed by the exact same source, and it does not alter text/link/mention handling.

What alternative solutions did you explore? (Optional)

Hardening only the read side — making useThumbnailDimensions fall back to measuring the real video instead of the 1.5 NAN_ASPECT_RATIO — would mask the symptom but still persist HTML that has lost its dimensions, breaking native (which has no off-screen web measurement) and the attachment carousel; fixing the write-path keeps the stored HTML correct for every consumer.

[lorretheboy]

Proposal

What is the root cause of that problem?

The orientation of a video in the chat preview comes from the data-expensify-width / data-expensify-height attributes on the stored <video> tag, which VideoRenderer reads into videoDimensions:

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

Lines 24 to 25 in bad4883

	const width = Number(htmlAttribs[CONST.ATTACHMENT_THUMBNAIL_WIDTH_ATTRIBUTE]);
	const height = Number(htmlAttribs[CONST.ATTACHMENT_THUMBNAIL_HEIGHT_ATTRIBUTE]);

On web, though, VideoPlayerPreview does not trust those attributes. It probes the file with a throwaway <video> element and, whenever the browser-measured intrinsic size differs from the attributes, it overwrites the dimensions with the measured ones:

App/src/components/VideoPlayerPreview/index.tsx

Lines 76 to 95 in bad4883

	useEffect(() => {
	if (!videoUrl || getPlatform() !== CONST.PLATFORM.WEB) {
	return;
	}
	const video = document.createElement('video');
	video.onloadedmetadata = () => {
	if (video.videoWidth === videoDimensions.width && video.videoHeight === videoDimensions.height) {
	return;
	}
	setWebMeasuredDimensions({
	width: video.videoWidth,
	height: video.videoHeight,
	});
	};
	video.src = videoUrl;
	video.load();
	return () => {
	video.src = '';
	};
	}, [videoUrl, videoDimensions.width, videoDimensions.height]);

and that measured value then takes priority over the attribute value:

App/src/components/VideoPlayerPreview/index.tsx

Line 67 in bad4883

const measuredDimensions = getPlatform() === CONST.PLATFORM.WEB && videoUrl && webMeasuredDimensions ? webMeasuredDimensions : videoDimensions;

The browser's videoWidth/videoHeight reflect the raw encoded frame and don't consistently apply the rotation metadata that makes a phone-portrait clip display upright, so the measured size is landscape while the stored attributes are portrait. When the message is edited, editReportComment rebuilds the comment HTML by re-parsing the draft markdown, the preview re-renders/re-measures, and the destructive override at setWebMeasuredDimensions replaces the correct portrait dimensions with the rotation-unaware landscape ones — so the orientation flips. This is also why it only reproduces on web (the measurement block is web-only) and not on the apps/mWeb.

Note this is not introduced by the referenced PR #93971 — that change only threads personalDetails into editReportComment/handleUserDeletedLinksInHtml and leaves the videoAttributeCache argument and the whole video path unchanged.

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

Make the web measurement a fallback instead of an override. The attributes that came with the message (videoDimensions) are authoritative; the browser probe should only fill them in when they are missing/invalid.

Concretely, gate the override here so we keep videoDimensions whenever it already carries valid numbers, and only adopt webMeasuredDimensions when videoDimensions is absent (e.g. width/height are NaN/0):

App/src/components/VideoPlayerPreview/index.tsx

Line 67 in bad4883

const measuredDimensions = getPlatform() === CONST.PLATFORM.WEB && videoUrl && webMeasuredDimensions ? webMeasuredDimensions : videoDimensions;

and apply the same "only when the attributes are missing" condition before calling setWebMeasuredDimensions in the probe effect and in onSourceLoaded:

App/src/components/VideoPlayerPreview/index.tsx

Line 82 in bad4883

if (video.videoWidth === videoDimensions.width && video.videoHeight === videoDimensions.height) {

This keeps the original orientation across edits while still letting the measurement help legacy videos that were stored without dimensions.

What alternative solutions did you explore? (Optional)

Fix it at the data level so the rebuilt comment carries the original dimensions: have the edit round-trip in editReportComment always restore data-expensify-width/height from draftMessageVideoAttributeCache. If the attributes are reliably preserved on edit, the web preview never sees a mismatch and never re-measures. This is the deeper fix but touches the shared parse/cache path; the preview-level change above is the smaller, web-isolated fix that directly stops the orientation flip.

[neerajbachani]

proposal

[melvin-bot]

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

[melvin-bot]

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

[godzillaxbox19982014-lgtm]

Proposal

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

[$250] Attachment - Video orientation changes after editing a message

What is the root cause of that problem?

I traced this to the path that handles the issue-specific state:

• src/pages/inbox/report/ReportActionCompose/useAttachmentPicker.ts:53 (useFilesValidation): const {validateFiles, PDFValidationComponent, ErrorModal} = useFilesValidation(onFilesValidated);
• src/pages/inbox/report/ReportActionCompose/useAttachmentPicker.ts:97 (useAttachmentPicker): return {pickAttachments, PDFValidationComponent, ErrorModal};
• src/pages/inbox/report/actionContents/ChatMessageContent.tsx:48 (attachmentContextValue): const attachmentContextValue = isOnSearch ? {type: CONST.ATTACHMENT_TYPE.SEARCH} : {reportID, type: CONST.ATTACHMENT_TYPE.REPORT};
• The local evidence matches this recurring failure shape: the composer visibility gate and recipient lookup are derived from different Onyx selector states. Here, src/pages/inbox/report/ReportActionCompose/useAttachmentPicker.ts:53 and src/pages/inbox/report/ReportActionCompose/useAttachmentPicker.ts:97 do not agree on the same the issue-specific state value, so the user-visible path can show a missing or stale state even though nearby data already has the needed context.

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

• Update src/pages/inbox/report/ReportActionCompose/useAttachmentPicker.ts:53 to derive the issue-specific state from the report/transaction source that owns this flow before the value is consumed.
• Carry the same normalized value through src/pages/inbox/report/ReportActionCompose/useAttachmentPicker.ts:97; that follows the safer family move to derive the recipient accountID once, then let both the eligibility selector and recipient selector use that same ID, so the write/optimistic/read paths agree instead of only changing the visible component.
• Keep the change inside the Chat Report, Composer, Recipient Local Time surface and avoid changing chat composer rendering, Onyx selector churn.

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

1. Add or update a regression in tests/unit/ComposerLocalTimeTest.tsx for the exact issue flow: [$250] Attachment - Video orientation changes after editing a message.
2. Cover the counter-case near src/pages/inbox/report/ReportActionCompose/useAttachmentPicker.ts:53 so valid existing the issue-specific state behavior still works.
3. Manually repeat the issue steps after the patch and confirm the visible state/action is no longer stale.

What alternative solutions did you explore? (Optional)

• I would not add a broad new condition outside these referenced paths; the safer fix is to keep the issue-specific state consistent at the first read/write boundary shown above.