[Due for payment 2025-05-27] [$250] Attachment - Attachment not found text moves, download modal not shown for broken image

[jponikarchuk]

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.1.30-0
Reproducible in staging?: Yes
Reproducible in production?: Yes
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: No, reproducible on hybrid only
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Email or phone of affected tester (no customers): Slottwo1 [email protected]
Issue reported by: Applause Internal Team
Device used: Redminote 10s android 13
App Component: Chat Report View

Action Performed:

1. Launch app
2. Open a chat
3. Send a broken image by pasting this text -
   
4. Open the broken image
5. Swipe left & right attachment not found text
6. Note attachment not found text moves
7. Tap download button on top
8. Note download button turns green but no attachment modal displayed.
9. In mweb, open the site
10. Perform step 2 - step 5
11. Note after performing step 2 in mweb, broken image is misaligned and shown in compose box, similarly after performing step 5 in mweb, attachment not found text doesn't move.

Expected Result:

In android, for broken image download button musnt be displayed or attachment not download modal must be shown.
Similarly like mweb, attachment not found must not move.

Actual Result:

In android, attachment not found text moves, download modal not shown for broken image. In mweb, image broken is misaligned and attachment not found text doesn't move.

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

1.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021915486228184113065
• Upwork Job ID: 1915486228184113065
• Last Price Increase: 2025-05-01

Issue Owner

Current Issue Owner: @lydiabarclay

[melvin-bot]

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

[lydiabarclay]

Posted here

[trjExpensify]

👋 @lydiabarclay - I think you should ship this External now, yeah?

[melvin-bot]

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

[melvin-bot]

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

[truph01]

Proposal

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

• Attachment - Attachment not found text moves, download modal not shown for broken image

What is the root cause of that problem?

Bug – "Attachment not found" text moves:

• In the following line:
  

  App/src/components/Attachments/AttachmentCarousel/Pager/index.tsx

  Line 140 in ab60cd2

  <AnimatedPagerView

  
  overScrollMode is always set to 'auto', without accounting for cases where there is only one attachment. As a result, the "Attachment not found" view can still be dragged when it shouldn't be.

Bug – Download modal not shown for broken image:

• In this function:
  

  App/src/libs/fileDownload/index.android.ts

  Line 66 in ab60cd2

  fetchedAttachment

  
  if the image is broken, the fetch promise neither resolves nor rejects, preventing the error modal from being triggered.

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

Fix Bug Attachment - Attachment not found text moves:

• Update the overScrollMode prop to disable overscroll when there's only one item in the carousel by adding overScrollMode={carouselItems.length > 1 ? 'auto' : 'never'} to:
  

  App/src/components/Attachments/AttachmentCarousel/Pager/index.tsx

  Line 140 in ab60cd2

  <AnimatedPagerView

Fix Bug Download modal not shown for broken image:

• Add a timeout to ensure the fetch promise doesn't hang indefinitely by update:
  

  App/src/libs/fileDownload/index.android.ts

  Line 66 in ab60cd2

  fetchedAttachment

    let didTimeout = false
    const timeoutPromise = new Promise<never>((_, reject) => {
        setTimeout(() => {
            didTimeout = true;
            reject(new Error('timeout'));
        }, 2000);
    });
    Promise.race([fetchedAttachment, timeoutPromise])
  

And update:

App/src/libs/fileDownload/index.android.ts

Lines 92 to 94 in ab60cd2

	.catch(() => {
	showGeneralErrorAlert();
	})

        .catch(() => {
            if(didTimeout){
                fetchedAttachment?.cancel?.()
            }
            showGeneralErrorAlert();
        })

• Note: The timeout and the alert modal can be decided in PR phase.

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)

[hungvu193]

I can only reproduce the first one:

In android, attachment not found text moves

The download button worked fine for me on latest main:

Screen.Recording.2025-04-28.at.15.02.03.mov

@truph01 Can you reproduce the bug where the download button doesn't show up?

[truph01]

download modal not shown for broken image

@hungvu193 I can reproduce it in the latest main (standalone app):

Screen.Recording.2025-04-28.at.15.27.38.mov

[truph01]

@hungvu193 I just tested one more time, I observed that the download modal is just shown after a long time (> 1 min):

Screen.Recording.2025-04-28.at.15.39.44.mov

[hungvu193]

Mind sharing your image message? The download modal shows everytime for me 😄

[truph01]

@hungvu193 Here is video from my side (the message is ![broken image]() ![Alt text](https://invalid-url/image.jpg)), the download modal is still shown, but after a long time:

video.mp4

Update: By adding the log in here, the error is Error: Download manager failed to download from https://invalid-url/image.jpg. Status Code = 16

[hungvu193]

• if the image is broken, the fetch promise neither resolves nor rejects, preventing the error modal from being triggered.

Your explanation here can't convince me. I still see the response after 3,4 mins later 🤔 . I think this definitely a bug from react-native-blob-util.

Still waiting for proposals.

[melvin-bot]

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

[hungvu193]

Not overdue, Still looking for proposals 👀

[hungvu193]

Posted on Slack to get more 👀

[melvin-bot]

@hungvu193 @lydiabarclay this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[lydiabarclay]

Boosted on Slack

[JakubKorytko]

I'm from SWM Expert Agency and I can take care of this, please assign me.

[JakubKorytko]

Posted update in PR comment #61459 (comment) to keep it in one place, please take a look and address listed problems/updates so I can continue.

[JakubKorytko]

Okay, many things were said in the PR if someone wants to know the full context, but to sum it up in my opinion we need a solid refractor of AttachmentModal and its children.

Discovered problems while fixing this issue

• NativeViewGestureHandlerProps in AttachmentCarouselPager is deprecated
• On Native onError is not called at all, and we don't see the "attachment not found" fallback unless we:
  • Open the first attachment in the chat, then we can scroll through the carousel and everything works
  • Open any attachment, scroll through the carousel to the first one and scroll back
• @hungvu193 found out while testing main branch that DefaultAttachmentView is displayed instead of AttachmentViewImage on Native when the image is broken, so there is no info that the attachment is broken
• We have 3 context values for AttachmentCarouselPagerContext, each overwriting the previous one, in:
  • AttachmentModal
  • AttachmentCarousel
  • AttachmentCarouselPager
• States used in the modal are redundant/misleading, e.g. isAttachmentInvalid, attachmentInvalidReason, attachmentInvalidReasonTitle - for some reason these are not set on missing attachment.

Videos for context

Screen.Recording.2025-05-09.at.15.01.37.mov

NewProblem.mp4

Compare.mp4

I've made a fix that hides the download button for now, and I haven't fixed moving the broken image left/right because we can move the image even when we have a single attachment chat on native (but not on m-web, web etc), so to keep consistency I haven't touched that - let me know what you think.

That said, this is just a band-aid, and sooner or later new issues will arise around the whole attachment modal. It will probably take some time to refractor this logic, as it is complex and huge, but at this point I think it is worth doing.

My idea is to go with the normal flow with this issue and after the merge of my PR simply raise an issue for refractor and assign me + @hungvu193 as the reviewer as we have the context because of the last days work on this. @mountiny what do you think about this?

[mountiny]

@chrispader is looking into refactoring the modal too, might have to align on the next steps with jim hut otherwise looks good

[JakubKorytko]

Great, if someone else has a plan for it then I don't mind - @chrispader please confirm if you will be dealing with the above issues & the refractor (or if I should) so that @hungvu193 can continue to review the download button issue without worrying about the other related issues as they will be covered in another PR.

[chrispader]

I've made a fix that hides the download button for now, and I haven't fixed moving the broken image left/right because we can move the image even when we have a single attachment chat on native (but not on m-web, web etc), so to keep consistency I haven't touched that - let me know what you think.

Your PR looks good to me and does not conflict with my work in #56219

Great, if someone else has a plan for it then I don't mind - @chrispader please confirm if you will be dealing with the above issues & the refractor (or if I should) so that @hungvu193 can continue to review the download button issue without worrying about the other related issues as they will be covered in another PR.

@JakubKorytko if you don't mind i'd like to look into this after #56219 has been merged. The PR includes quite a lot of re-structuring and simplification of all the AttachmentModal modals and i'm sure we can remove even more complexity from all the attachment components and logic.

I agree with all the discovered problems mentioned above, especially the problem with the context 🙌🏼

NativeViewGestureHandlerProps in AttachmentCarouselPager is deprecated

This was also causing ESLint to fail in my other PR. Feel free to create a follow-up PR for this, alternatively i can handle it a follow-up.

[JakubKorytko]

Great, okay then, if you are aware of this and working/will work on it then awesome, I will wait for @hungvu193 to accept the PR and possibly create a PR for NativeViewGestureHandlerProps, the rest is up to you! If you need any help LMK.

[hungvu193]

Yep. I'll review the PR today

[JakubKorytko]

FYI: I've created a quick PR for the failing ESLint check #61841

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

• Fix attachment modal for broken images #61459

If no regressions arise, payment will be issued on 2025-05-27. 🎊

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

• @hungvu193 requires payment through NewDot Manual Requests
• @JakubKorytko does not require payment (Contractor)

[melvin-bot]

@hungvu193 @lydiabarclay @hungvu193 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]