fix: Attachment - Edit comment option is shown for .doc file

cspell.json

@@ -737,6 +737,7 @@
         "onloaderror",
         "onopentag",
         "onplayerror",
+        "ontext",
         "onxy",
         "openxmlformats",
         "ordinality",

src/libs/ReportUtils.ts

@@ -114,7 +114,6 @@ import {getMicroSecondOnyxErrorWithTranslationKey, isReceiptError} from './Error
 import getAttachmentDetails from './fileDownload/getAttachmentDetails';
 import type {FormulaContext} from './Formula';
 import getBase62ReportID from './getBase62ReportID';
-import {isReportMessageAttachment} from './isReportMessageAttachment';
 import {formatPhoneNumber as formatPhoneNumberPhoneUtils} from './LocalePhoneNumber';
 import {translateLocal} from './Localize';
 import Log from './Log';
@@ -5063,13 +5062,16 @@ function canEditFieldOfMoneyRequest({
  * Can only edit if:
  *
  * - It was written by the current user
- * - It's an ADD_COMMENT that is not an attachment
+ * - It's an ADD_COMMENT or IOU action
+ * - It's not an optimistic attachment (still uploading)
  * - It's an expense where conditions for modifications are defined in canEditMoneyRequest method
  * - It's not pending deletion
  */
 function canEditReportAction(reportAction: OnyxInputOrEntry<ReportAction>, linkedTransaction: OnyxEntry<Transaction>): boolean {
     const isCommentOrIOU = reportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.ADD_COMMENT || reportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.IOU;
-    const message = reportAction ? getReportActionMessageReportUtils(reportAction) : undefined;
+
+    // Block editing only while the attachment is still uploading; once synced, attachments are editable.
+    const isOptimisticAttachment = !!reportAction?.isOptimisticAction && (!!reportAction?.isAttachmentOnly || !!reportAction?.isAttachmentWithText);
 
     // For money request actions on settled/approved/closed expense reports, the action cannot be edited.
     // canEditMoneyRequest has an admin/manager bypass for field-level edits (via canEditFieldOfMoneyRequest),
@@ -5087,9 +5089,8 @@ function canEditReportAction(reportAction: OnyxInputOrEntry<ReportAction>, linke
     return !!(
         reportAction?.actorAccountID === deprecatedCurrentUserAccountID &&
         isCommentOrIOU &&
-        (!isMoneyRequestAction(reportAction) || canEditMoneyRequest(reportAction, linkedTransaction)) && // Returns true for non-IOU actions
-        !isReportMessageAttachment(message) &&
-        ((!reportAction.isAttachmentWithText && !reportAction.isAttachmentOnly) || !reportAction.isOptimisticAction) &&
+        (!isMoneyRequestAction(reportAction) || canEditMoneyRequest(reportAction, linkedTransaction)) &&
+        !isOptimisticAttachment &&
         !isDeletedAction(reportAction) &&
         !isCreatedTaskReportAction(reportAction) &&
         reportAction?.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE

src/libs/isReportMessageAttachment.ts

@@ -1,37 +1,88 @@
-import {Str} from 'expensify-common';
+import {Parser as HtmlParser} from 'htmlparser2';
 import CONST from '@src/CONST';
 import type {Message} from '@src/types/onyx/ReportAction';
 
-const attachmentRegex = new RegExp(` ${CONST.ATTACHMENT_SOURCE_ATTRIBUTE}="(.*)"`, 'i');
+const ATTACHMENT_TAGS = new Set(['a', 'img', 'video']);
+// Leading space + `="` (not a bare substring) so a URL query param like `?data-expensify-source=` isn't a false positive.
+const ATTACHMENT_SOURCE_TOKEN = ` ${CONST.ATTACHMENT_SOURCE_ATTRIBUTE}="`;
 
-/**
- * Check whether a report action is Attachment or not.
- * Ignore messages containing [Attachment] as the main content. Attachments are actions with only text as [Attachment].
- *
- * @param message report action's message as text, html and translationKey
- */
-function isReportMessageAttachment(message: Message | undefined): boolean {
-    if (!message?.text || !message.html) {
+// Keyed by Onyx message identity: re-renders pass the same immutable object, so parse at most once per message.
+const resultCache = new WeakMap<Message, boolean>();
+
+function computeIsReportMessageAttachment(html: string, text: string, translationKey: string | undefined): boolean {
+    // Optimistic attachment-only messages carry this translationKey; the server drops it after sync.
+    if (translationKey === CONST.TRANSLATION_KEYS.ATTACHMENT) {
+        return true;
+    }
+
+    // Fast path: no source attribute → not an attachment (avoids the parser for most messages).
+    if (!html.includes(ATTACHMENT_SOURCE_TOKEN)) {
         return false;
     }
 
-    if (message.translationKey) {
-        return message.text === CONST.ATTACHMENT_MESSAGE_TEXT && message.translationKey === CONST.TRANSLATION_KEYS.ATTACHMENT;
+    // image/video attachment-only keep text "[Attachment]"; only .doc/.pdf (text = "filename URL") need the parser.
+    if (text === CONST.ATTACHMENT_MESSAGE_TEXT) {
+        return true;
     }
 
-    const hasAttachmentHtml = attachmentRegex.test(message.html);
+    let hasAttachment = false;
+    let hasOtherContent = false;
+    // Depth >0 = inside the attachment's subtree; its inner nodes (filename/markup) aren't user content.
+    let depth = 0;
 
-    if (!hasAttachmentHtml) {
+    const parser = new HtmlParser({
+        ontext: (nodeText) => {
+            if (depth > 0 || !nodeText.trim()) {
+                return;
+            }
+            hasOtherContent = true;
+        },
+        onopentag: (name, attribs) => {
+            if (depth > 0) {
+                depth += 1;
+                return;
+            }
+            if (name === 'br') {
+                return;
+            }
+            if (ATTACHMENT_TAGS.has(name) && !!attribs[CONST.ATTACHMENT_SOURCE_ATTRIBUTE]) {
+                hasAttachment = true;
+                if (name === 'a' || name === 'video') {
+                    depth = 1;
+                }
+                return;
+            }
+            hasOtherContent = true;
+        },
+        onclosetag: () => {
+            if (depth <= 0) {
+                return;
+            }
+            depth -= 1;
+        },
+    });
+    parser.write(html);
+    parser.end();
+
+    return hasAttachment && !hasOtherContent;
+}
+
+/**
+ * Returns true for attachment-only messages (no user-typed text), false for attachment+text and non-attachment messages.
+ */
+function isReportMessageAttachment(message: Message | undefined): boolean {
+    if (!message?.text || !message.html) {
         return false;
     }
 
-    const isAttachmentMessageText = message.text === CONST.ATTACHMENT_MESSAGE_TEXT;
-
-    if (isAttachmentMessageText) {
-        return true;
+    const cached = resultCache.get(message);
+    if (cached !== undefined) {
+        return cached;
     }
 
-    return Str.isVideo(message.text);
+    const result = computeIsReportMessageAttachment(message.html, message.text, message.translationKey);
+    resultCache.set(message, result);
+    return result;
 }
 
 // eslint-disable-next-line import/prefer-default-export

tests/perf-test/isReportMessageAttachment.perf-test.ts

@@ -0,0 +1,37 @@
+import {measureFunction} from 'reassure';
+import type {Message} from '@src/types/onyx/ReportAction';
+import {isReportMessageAttachment} from '../../src/libs/isReportMessageAttachment';
+
+const SAMPLE_MESSAGES: Message[] = [
+    // .doc attachment-only after OpenReport: text = "filename URL" (the parser-bound case)
+    {
+        text: 'sample.doc https://www.expensify.com/chat-attachments/123/w_abc.doc',
+        html: '<a href="https://www.expensify.com/chat-attachments/123/w_abc.doc" data-expensify-source="https://www.expensify.com/chat-attachments/123/w_abc.doc">sample.doc</a>',
+        type: '',
+    },
+    // image attachment-only (translationKey path)
+    {
+        text: '[Attachment]',
+        html: '<img src="https://www.expensify.com/chat-attachments/123/img.jpg" data-expensify-source="https://www.expensify.com/chat-attachments/123/img.jpg" />',
+        type: '',
+        translationKey: 'common.attachment',
+    },
+    {
+        text: 'Here is the file\nsample.pdf https://www.expensify.com/chat-attachments/123/s.pdf',
+        html: 'Here is the file<br /><br /><a href="https://www.expensify.com/chat-attachments/123/s.pdf" data-expensify-source="https://www.expensify.com/chat-attachments/123/s.pdf">sample.pdf</a>',
+        type: '',
+    },
+    {text: 'Just a normal chat message with some words in it', html: 'Just a normal chat message with some words in it', type: ''},
+    {text: 'https://example.com', html: '<a href="https://example.com" data-raw-href="https://example.com" target="_blank" rel="noreferrer noopener">https://example.com</a>', type: ''},
+];
+
+// One classification pass over a long chat history (called per message on list renders / LHN).
+const LONG_CHAT_HISTORY: Message[] = Array.from({length: 2000}, () => SAMPLE_MESSAGES).flat();
+
+test('[isReportMessageAttachment] classify a 10k-message chat history', async () => {
+    await measureFunction(() => {
+        for (const message of LONG_CHAT_HISTORY) {
+            isReportMessageAttachment(message);
+        }
+    });
+});

tests/unit/ReportUtilsTest.ts

@@ -5745,6 +5745,111 @@ describe('ReportUtils', () => {
 
             expect(canEditReportAction(moneyRequestAction, transaction)).toEqual(false);
         });
+
+        it('should return false for an optimistic attachment-only action (still uploading)', () => {
+            const transaction = createRandomTransaction(300);
+            const reportAction: ReportAction = {
+                reportActionID: '300',
+                actorAccountID: currentUserAccountID,
+                actionName: CONST.REPORT.ACTIONS.TYPE.ADD_COMMENT,
+                message: [
+                    {
+                        type: 'COMMENT',
+                        html: '<a href="blob:..." data-expensify-source="blob:...">file.doc</a>',
+                        text: '[Attachment]',
+                    },
+                ],
+                isAttachmentOnly: true,
+                isOptimisticAction: true,
+                created: '2025-03-05 16:34:27',
+            };
+
+            expect(canEditReportAction(reportAction, transaction)).toEqual(false);
+        });
+
+        it('should return false for an optimistic attachment+text action (still uploading)', () => {
+            const transaction = createRandomTransaction(301);
+            const reportAction: ReportAction = {
+                reportActionID: '301',
+                actorAccountID: currentUserAccountID,
+                actionName: CONST.REPORT.ACTIONS.TYPE.ADD_COMMENT,
+                message: [
+                    {
+                        type: 'COMMENT',
+                        html: 'Hi<br /><br /><a href="blob:..." data-expensify-source="blob:...">file.doc</a>',
+                        text: 'Hi\n[Attachment]',
+                    },
+                ],
+                isAttachmentWithText: true,
+                isOptimisticAction: true,
+                created: '2025-03-05 16:34:27',
+            };
+
+            expect(canEditReportAction(reportAction, transaction)).toEqual(false);
+        });
+
+        it('should return true for a synced attachment-only action (optimistic flags cleared)', () => {
+            const transaction = createRandomTransaction(302);
+            // Post-sync state: successData clears isOptimisticAction (null in Onyx, undefined here); isAttachmentOnly persists.
+            const reportAction: ReportAction = {
+                reportActionID: '302',
+                actorAccountID: currentUserAccountID,
+                actionName: CONST.REPORT.ACTIONS.TYPE.ADD_COMMENT,
+                message: [
+                    {
+                        type: 'COMMENT',
+                        html: '<a href="https://www.expensify.com/chat-attachments/123/file.doc" data-expensify-source="https://www.expensify.com/chat-attachments/123/file.doc">file.doc</a>',
+                        text: 'file.doc https://www.expensify.com/chat-attachments/123/file.doc',
+                    },
+                ],
+                isAttachmentOnly: true,
+                isOptimisticAction: undefined,
+                created: '2025-03-05 16:34:27',
+            };
+
+            expect(canEditReportAction(reportAction, transaction)).toEqual(true);
+        });
+
+        it('should return true for a synced attachment+text action', () => {
+            const transaction = createRandomTransaction(303);
+            const reportAction: ReportAction = {
+                reportActionID: '303',
+                actorAccountID: currentUserAccountID,
+                actionName: CONST.REPORT.ACTIONS.TYPE.ADD_COMMENT,
+                message: [
+                    {
+                        type: 'COMMENT',
+                        html: 'Hi<br /><br /><a href="https://www.expensify.com/chat-attachments/123/file.doc" data-expensify-source="https://www.expensify.com/chat-attachments/123/file.doc">file.doc</a>',
+                        text: 'Hi\nfile.doc https://www.expensify.com/chat-attachments/123/file.doc',
+                    },
+                ],
+                isAttachmentWithText: true,
+                isOptimisticAction: undefined,
+                created: '2025-03-05 16:34:27',
+            };
+
+            expect(canEditReportAction(reportAction, transaction)).toEqual(true);
+        });
+
+        it('should return true for an optimistic plain-text comment (no attachment)', () => {
+            const transaction = createRandomTransaction(304);
+            const reportAction: ReportAction = {
+                reportActionID: '304',
+                actorAccountID: currentUserAccountID,
+                actionName: CONST.REPORT.ACTIONS.TYPE.ADD_COMMENT,
+                message: [
+                    {
+                        type: 'COMMENT',
+                        html: 'hello',
+                        text: 'hello',
+                    },
+                ],
+                isOptimisticAction: true,
+                created: '2025-03-05 16:34:27',
+            };
+
+            expect(canEditReportAction(reportAction, transaction)).toEqual(true);
+        });
     });
 
     describe('getChatByParticipants', () => {