[$500] PDF receipt preview loads each time visiting 1:1 DM, report and transaction thread

[kavimuru]

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: 1.4.48-0
Reproducible in staging?: y
Reproducible in production?: n
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause internal team
Slack conversation:

Action Performed:

1. Go to staging.new.expensify.com
2. Go offline.
3. Go to FAB > Request money > Scan.
4. Select a PDF receipt.
5. Select any participant and create the Scan request.
6. in 1:1 DM, click on the scan preview.
7. In IOU report, click on the scan preview.
8. Click on the header subtitle to return to report and 1:1 DM.

Expected Result:

The PDF receipt preview will not load each time going back and forth between 1:1 DM, IOU report and transaction thread.

Actual Result:

The PDF receipt preview loads each time going back and forth between 1:1 DM, IOU report and transaction thread. Sometimes it shows blue "loading page" briefly when the receipt is rendering.

Workaround:

unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6404710_1709766997050.bandicam_2024-03-07_07-10-11-388.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~016df3671b30e63c80
• Upwork Job ID: 1806060107499225786
• Last Price Increase: 2024-07-24

Issue Owner

Current Issue Owner: @

[github-actions]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[melvin-bot]

Triggered auto assignment to @anmurali (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

Triggered auto assignment to @deetergp (Engineering), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[kavimuru]

@anmurali FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors.

[deetergp]

Is this our culprit PR? #35255 (cc @eh2077 @s77rt @luacmartins)

[youssef-lr]

We have just added support for PDF receipts, so I'm not sure this would qualify as a deploy blocker because we don't support PDF receipts in production right now.

[deetergp]

Thanks @youssef-lr, that sounds reasonable. I'll remove the label and demote this back down to Daily.

[s77rt]

@eh2077 Can you please take a quick look on this?

[eh2077]

@s77rt Sure. I'm looking into it.

[eh2077]

It seems not easy to avoid reloading PDF when going back and forth between 1:1 DM, IOU report and transaction thread. This is because the browser by default can't cache the canvas of the PDFThumbnail but it's able to cache image by default.

While checking this issue, I also found another flaw that's easier to avoid - PDF thumbnail reloads when hovering on it

Screen.Recording.2024-03-08.at.3.51.11.PM.mov

[eh2077]

I think the reloading issue when hovering is a legit regression. I can make a quick PR to fix it.

But the reloading issue when re-visiting different pages seems a tricky one. Can this be a followup improvement instead of a regression?

@s77rt @luacmartins What're your opinions?

[s77rt]

@eh2077 Can you please raise a PR fixing the hover issue? I thought we fixed that here #35255 (comment) Is this another case that we missed?

[melvin-bot]

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

[melvin-bot]

Upwork job price has been updated to $500

[anmurali]

Increased the bounty to $500 on the job.

[tsa321]

Proposal

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

In Offline mode, PDF thumbnail of receipt is always re-rendered when revisiting the report

What is the root cause of that problem?

When showing PDF thumbnail of the receipt in ReportActionItemImage:

App/src/components/ReportActionItem/ReportActionItemImage.tsx

Lines 111 to 112 in fa738c6

	} else if (isLocalFile && filename && Str.isPDF(filename) && typeof attachmentModalSource === 'string') {
	propsObj = {isPDFThumbnail: true, source: attachmentModalSource};

the PDFThumbnail:

App/src/components/PDFThumbnail/index.tsx

Line 15 in fa738c6

function PDFThumbnail({previewSourceURL, style, isAuthTokenRequired = false, enabled = true, onPassword, onLoadError}: PDFThumbnailProps) {

doesn't cache the rendered image. So the pdf file will be rendered each time the pdfthumbnail mounted.

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

We could implement caching logic in PDFThumbnail to store the cached image in an object variable using previewSourceURL as the key. To generate the cached image, we could add the canvasRef property to the Thumbnail of PDFThumbnail and convert the canvas element into a blob object.
The changes in PDFThumbnail code could be:

    import pdfWorkerSource from 'pdfjs-dist/legacy/build/pdf.worker';
+ import React, {useCallback, useMemo, useRef, useState} from 'react';
   import {View} from 'react-native';
   import {Document, Image, pdfjs, Thumbnail} from 'react-pdf';
   import FullScreenLoadingIndicator from '@components/FullscreenLoadingIndicator';
+ import * as Expensicons from '@components/Icon/Expensicons';
+ import ThumbnailImage from '@components/ThumbnailImage';
   import useThemeStyles from '@hooks/useThemeStyles';
   import addEncryptedAuthTokenToURL from '@libs/addEncryptedAuthTokenToURL';
+ import CONST from '@src/CONST';
   import PDFThumbnailError from './PDFThumbnailError';
   import type PDFThumbnailProps from './types';

if (!pdfjs.GlobalWorkerOptions.workerSrc) {
    pdfjs.GlobalWorkerOptions.workerSrc = URL.createObjectURL(new Blob([pdfWorkerSource], {type: 'text/javascript'}));
}

+ let pdfThumbnailImageCaches = {};

function PDFThumbnail({
    previewSourceURL,
    cachedPDFThumbnail = null,
    style,
    isAuthTokenRequired = false,
    enabled = true,
    onPassword,
    onLoadError,
    onRenderSuccess = () => {},
}: PDFThumbnailProps) {
      const styles = useThemeStyles();
+    const canvasRef = useRef();
      const [failedToLoad, setFailedToLoad] = useState(false);

+    const [thumbnailCache, setThumbnailCache] = useState(pdfThumbnailImageCaches[previewSourceURL]);

+    const cachePDFThumbnailImage = useCallback(() => {
+        if (!!canvasRef.current) {
+            canvasRef.current.toBlob((blob) => {
+                const href = URL.createObjectURL(blob);
+                pdfThumbnailImageCaches[previewSourceURL] = href;
+                setThumbnailCache(href);
+            }, 'image/png');
+        }
+    }, [previewSourceURL, pdfThumbnailImageCaches]);
   
      const thumbnail = useMemo(() => {
+        if (!!thumbnailCache) {
+            return (
+                <ThumbnailImage
+                    previewSourceURL={thumbnailCache}
+                    style={[styles.w100, styles.h100]}
+                    isAuthTokenRequired={false}
+                    shouldDynamicallyResize={false}
+                    fallbackIcon={Expensicons.Document}
+                    objectPosition={CONST.IMAGE_OBJECT_POSITION.TOP}
+                />
+            );
+        }

          return (
              <Document
                  loading={<FullScreenLoadingIndicator />}
                  file={isAuthTokenRequired ? addEncryptedAuthTokenToURL(previewSourceURL) : previewSourceURL}
                  options={{
                      cMapUrl: 'cmaps/',
                      cMapPacked: true,
                  }}
                  externalLinkTarget="_blank"
                  onPassword={onPassword}
                  onLoad={() => {
                      setFailedToLoad(false);
                  }}
                  onLoadError={() => {
                      if (onLoadError) {
                          onLoadError();
                      }
                      setFailedToLoad(true);
                  }}
                  error={() => null}
              >
                  <View pointerEvents="none">
                      <Thumbnail
                          pageIndex={0}
+                        canvasRef={canvasRef}
+                        onRenderSuccess={cachePDFThumbnailImage}
                      />
                  </View>
              </Document>
        );
      }, [isAuthTokenRequired, previewSourceURL, onPassword, onLoadError, thumbnailCache]);

      return (
          <View style={[style, styles.overflowHidden, failedToLoad && styles.h100]}>
              <View style={[styles.w100, styles.h100, !failedToLoad && {...styles.alignItemsCenter, ...styles.justifyContentCenter}]}>
                  {enabled && !failedToLoad && thumbnail}
                  {failedToLoad && <PDFThumbnailError />}
              </View>
          </View>
      );
  }

  PDFThumbnail.displayName = 'PDFThumbnail';
  export default React.memo(PDFThumbnail);

If we want to store the cached image in onyx as base64 we could do that too.

Alternative Solution

If we want to move the caching logic in ReportActionImageItem, the details:

alternative solution

We could use canvasRef property of Thumbail of the PDFThumnail:

App/src/components/PDFThumbnail/index.tsx

Line 42 in fa738c6

<Thumbnail pageIndex={0} />

and use the canvasElement to generate URL blob image of the canvas. (I don't know if storing base64 of the image is allowed in onyx, if it is allowed we could use the base64 instead of the url blob)

The rough code could be, in PDFThumbnail we add canvas ref and use canvasRef and onRenderSuccess property of the Thumbnail:

+ const canvas = useRef();
...
<Thumbnail 
      pageIndex={0} 
+      canvasRef={canvas} 
+      onRenderSuccess={() => onRenderSuccess(canvas.current)}
}} />

Then in ReportActionItemImage we add:

const cachedPDFReceiptThumbnail = useRef(ReceiptUtils.getPDFThumbnailCache(transaction.transactionID));
...
...
const cachePDFThumbnail = useCallback((canvasElement) => {
    if (!!canvasElement) {
        canvasElement.toBlob((blob) => {
            const href = URL.createObjectURL(blob);
            cachedPDFReceiptThumbnail.current = href;
            ReceiptUtils.cachePDFReceiptThumbnail(transaction.transactionID, href);
        }, 'image/png');
    }
}, [transaction]);


....
else if (isLocalFile && filename && Str.isPDF(filename) && typeof attachmentModalSource === 'string') {
    propsObj = {
        isPDFThumbnail: true,
        source: attachmentModalSource,
        onPDFRenderSuccess: cachePDFThumbnail,
        cachedPDFThumbnail: cachedPDFReceiptThumbnail.current
    };
}

The ReceiptUtils.cachePDFReceiptThumbnail could be simple :

function cachePDFReceiptThumbnail(transactionID: string, pdfThumbnailCache: string) {
    pdfThumbnailImageCaches[transactionID] = pdfThumbnailCache;  
}

function getPDFThumbnailCache(transactionID: string) {
    return pdfThumbnailImageCaches[transactionID];
}

Maybe we could combine transactionID with lastModified as key of the pdfThumbnailImageCaches.

Then in ReceiptImage:

if (isPDFThumbnail) {
        if (!!cachedPDFThumbnail) {
            return (
            <ThumbnailImage
                previewSourceURL={cachedPDFThumbnail ?? ''}
                style={[styles.w100, styles.h100]}
                isAuthTokenRequired={false}
                shouldDynamicallyResize={false}
                fallbackIcon={fallbackIcon}
                fallbackIconSize={fallbackIconSize}
                fallbackIconColor={fallbackIconColor}
                fallbackIconBackground={fallbackIconBackground}
                objectPosition={shouldUseInitialObjectPosition ? CONST.IMAGE_OBJECT_POSITION.INITIAL : CONST.IMAGE_OBJECT_POSITION.TOP}
            />);
        }
        return (
            <PDFThumbnail
                previewSourceURL={source ?? ''}
                style={[styles.w100, styles.h100]}
                onRenderSuccess={onPDFRenderSuccess}
                cachedPDFThumbnail={cachedPDFThumbnail}
            />
        );
    }

[tsa321]

Proposal

updated

[s77rt]

The bug is no longer reproducible. The PDF preview loads only when going forth (opening new reports), when going back the pdf preview is kept rendered as expected.

Screen.Recording.2024-07-21.at.1.56.14.PM.mov

@deetergp We can close this one. Looks we have a room for improvement

[tsa321]

@s77rt I can still reproduce it in the latest main:

macos-web-d.mp4

[tsa321]

@s77rt for navigating back to main expense chat, you can switch to another chat and then return to the main expense chat; you will see the loading spinner.

[s77rt]

@tsa321 Going back to the IOU report via the "From" link does not cause the preview to reload, but doing so from the LHN reloads the preview. Can you explain why is that happening?

Screen.Recording.2024-07-21.at.5.58.13.PM.mov

[tsa321]

@s77rt, please wait. I will try to take a look.

[s77rt]

Waiting on proposals...

[tsa321]

@s77rt The PDF also loads immediately when the user presses the browser's back button. When the user presses the link to the parent report in the report header, it functions similarly to going back (much like pressing the browser's back button). Perhaps the stack navigation/browser preserves the content of the previous screen/route, allowing the PDF to load immediately.

[melvin-bot]

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

[s77rt]

@tsa321 I don't think we really have a bug then. Opening a new report renders a new PDF Previewer and the loader is expected since the pdf is loaded at library level (unlike images where they are loaded at the browser level where caching is implemented). This can be a feature request but if we implement a caching mechanism it should be at react-pdf level and we should cache the pdf content and not its thumbnail.

(Same note applies for mobile if the bug is reproducible there)

[tsa321]

@s77rt Actually, we can cache all pages of the PDF as images. Is this the expected result? Caching the PDF means converting it into images, correct? Also, the server returns the PDF thumbnail as images too. Why should we do this at the react-pdf level? We can do it in the app.

[s77rt]

@tsa321 Caching is to save the pdf data (bytes) for a given uri. If we use a known uri we'd use the cached data instead of fetching that uri contents again.

Why should we do this at the react-pdf level?

It's a feature for react-pdf and not related to the app.

For this case the feature is a low priority since the uri that the PDF Previewer will load always points to a local file

[tsa321]

@s77rt, the PDF file is already stored as a blob URI, meaning it's stored in memory as bytes.

I believe we may have different perspectives on PDF caching:
My understanding is that PDF rendering involves drawing various elements such as text, lines, shapes, rectangles, paths, vectors, and images specified in the PDF file onto a canvas. This process is managed by react-pdf or whichever PDF engine library is utilized (indicated by the loading spinner during rendering).

On the other hand, caching the PDF means storing the rendered result as an image, so that it doesn't need to be redrawn.

However, I agree that this issue is of very low priority. I'm okay with closing this issue. @deetergp, what are your thoughts on this issue? Thank you.

[s77rt]

@deetergp Let's close this based on #37852 (comment)

[deetergp]

Sounds like a plan @s77rt 👍