Skip to content

Handle raw Data type in iOS share extension for screenshot sharing#84387

Closed
MelvinBot wants to merge 7 commits intomainfrom
claude-fixShareExtensionScreenshotData
Closed

Handle raw Data type in iOS share extension for screenshot sharing#84387
MelvinBot wants to merge 7 commits intomainfrom
claude-fixShareExtensionScreenshotData

Conversation

@MelvinBot
Copy link
Copy Markdown
Contributor

Explanation of Change

When sharing a screenshot directly from the iOS screenshot preview (the thumbnail that appears after taking a screenshot), iOS provides the image as raw Data bytes rather than an NSURL or UIImage. The share extension's handleData method in ShareViewController.swift was missing a case for this type, so the data silently fell through to the error handler. The app would still open (because openMainApp() is called regardless of errors in processAttachments) but no file was saved to the shared container, resulting in nothing happening.

This adds a Data handler that first attempts to decode the bytes as a UIImage (the common case for screenshots) and falls back to saving as a generic file for other data types.

Fixed Issues

$ https://github.com/Expensify/Expensify/issues/606736

Tests

  1. On iPhone, take a screenshot
  2. Tap the screenshot preview thumbnail in the bottom-left corner
  3. Tap "Share", then select "Expensify"
  4. Verify the "Share to Expensify" page appears and the image is loaded
  5. Also verify sharing from the Photos app still works as before (select an existing photo → Share → Expensify)
  • Verify that no errors appear in the JS console

Offline tests

N/A — the share extension operates on local data before opening the app.

QA Steps

  1. On iPhone, take a screenshot
  2. Tap the screenshot preview thumbnail in the bottom-left corner
  3. Tap "Share", then select "Expensify"
  4. Verify the "Share to Expensify" page appears and the image is loaded
  5. Also verify sharing from the Photos app still works as before (select an existing photo → Share → Expensify)
  • Verify that no errors appear in the JS console

PR Author Checklist

  • I linked the correct issue in the ### Fixed Issues section above
  • I wrote clear testing steps that cover the changes made in this PR
    • I added steps for local testing in the Tests section
    • I added steps for the expected offline behavior in the Offline steps section
    • I added steps for Staging and/or Production testing in the QA steps section
    • I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
    • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
    • I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
  • I included screenshots or videos for tests on all platforms
  • I ran the tests on all platforms & verified they passed on:
    • Android: Native
    • Android: mWeb Chrome
    • iOS: Native
    • iOS: mWeb Safari
    • MacOS: Chrome / Safari
  • I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
  • I followed proper code patterns (see Reviewing the code)
    • I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick)
    • I verified that comments were added to code that is not self explanatory
    • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
    • I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
      • If any non-english text was added/modified, I used JaimeGPT to get English > Spanish translation. I then posted it in #expensify-open-source and it was approved by an internal Expensify engineer. Link to Slack message:
    • I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
    • I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
    • I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
    • I verified the JSDocs style guidelines (in STYLE.md) were followed
  • If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
  • I followed the guidelines as stated in the Review Guidelines
  • I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
  • I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
  • I verified any variables that can be defined as constants (ie. in CONST.ts or at the top of the file that uses the constant) are defined as such
  • I verified that if a function's arguments changed that all usages have also been updated correctly
  • If any new file was added I verified that:
    • The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
  • If a new CSS style is added I verified that:
    • A similar style doesn't already exist
    • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))
  • If new assets were added or existing ones were modified, I verified that:
    • The assets are optimized and compressed (for SVG files, run npm run compress-svg)
    • The assets load correctly across all supported platforms.
  • If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
  • If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
  • If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
  • If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
  • If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
    • I verified that all the inputs inside a form are aligned with each other.
    • I added Design label and/or tagged @Expensify/design so the design team can review the changes.
  • If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
  • I added unit tests for any new feature or bug fix in this PR to help automatically prevent regressions in this user flow.
  • If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
  • If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.

Screenshots/Videos

Android: Native

N/A — iOS-only change

Android: mWeb Chrome

N/A — iOS-only change

iOS: Native

Requires manual testing on a physical iOS device with the share extension.

iOS: mWeb Safari

N/A — iOS-only change

MacOS: Chrome / Safari

N/A — iOS-only change

When sharing a screenshot directly from the iOS screenshot preview,
iOS provides the image as raw Data bytes rather than a NSURL or UIImage.
The share extension's handleData method was missing a case for this type,
causing the data to silently fall through to the error handler. This meant
the app would open but no file was saved to the shared container.

Add a Data handler that first attempts to decode the bytes as a UIImage
(the common case for screenshots) and falls back to saving as a generic
file for other data types.

Co-authored-by: Georgia Monahan <grgia@users.noreply.github.com>
@MelvinBot MelvinBot requested a review from a team March 6, 2026 10:51
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 6, 2026

🚧 @grgia has triggered a test Expensify/App build. You can view the workflow run here.

@github-actions

This comment has been minimized.

@grgia
Copy link
Copy Markdown
Contributor

grgia commented Mar 6, 2026

@MelvinBot I tested this build on an IOS device and it does not fix the issue

@github-actions
Copy link
Copy Markdown
Contributor

🚧 @grgia has triggered a test Expensify/App build. You can view the workflow run here.

@github-actions
Copy link
Copy Markdown
Contributor

🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
Built from App PR #84387.

Android 🤖 iOS 🍎
⏩ SKIPPED ⏩ https://ad-hoc-expensify-cash.s3.us-east-1.amazonaws.com/rock-artifacts/ad-hoc/rock-ios-device-AdHoc-4326bf5-6d1dfbe-a3c4a7c3c1ae148095723ebcedcd16eb8bcb16ea/index.html
The build for Android was skipped iOS
Web 🕸️
⏩ SKIPPED ⏩
The build for Web was skipped

👀 View the workflow run that generated this build 👀

@grgia
Copy link
Copy Markdown
Contributor

grgia commented Mar 13, 2026

@MelvinBot this is still reproducible on ios build

The previous fix added a Data type handler, but loadItem(forTypeIdentifier:
kUTTypeData) may fail entirely when the NSItemProvider (e.g. from the iOS
screenshot preview) registers images via loadObject rather than as raw data
representations. This switches to using canLoadObject/loadObject(ofClass:
UIImage.self) for image-type attachments, which is the Apple-recommended
API and works regardless of how the provider registered the image.

Also fixes handleStringData silently dropping file:// URL strings without
calling completion or saving any file.

Co-authored-by: Georgia Monahan <grgia@users.noreply.github.com>
@MelvinBot
Copy link
Copy Markdown
Contributor Author

I investigated why the previous fix wasn't working and pushed a new commit (2f02d1f) that addresses the root cause.

What was wrong

The original fix added a Data type handler in handleData, but the problem occurs before that handler is reached. When sharing a screenshot from the iOS screenshot preview thumbnail, the NSItemProvider registers the image using object-based representations (e.g. via NSItemProviderWriting), not as raw data. Calling loadItem(forTypeIdentifier: kUTTypeData) on such a provider may fail entirely because the provider has no registered data representation for public.data — only an object representation for public.image. When loadItem fails, it returns an error and the Data handler is never reached.

What the new commit fixes

1. Uses canLoadObject/loadObject(ofClass: UIImage.self) for images — This is the Apple-recommended API for loading images from NSItemProvider. It works regardless of how the image was registered by the provider (screenshot preview, Photos app, etc.). The loadData method now:

  • Checks isURL → loads via kUTTypeURL (unchanged)
  • Checks canLoadObject(ofClass: UIImage.self) → loads via loadObject API (new — handles screenshots)
  • Falls back to kUTTypeData for all other data types (files, etc.)

2. Fixes handleStringData silently dropping file:// strings — The old code had a if !dataString.hasPrefix("file://") guard that silently returned without calling completion or saving any file when a file:// string was received. The fix now properly converts file:// strings to NSURL and processes them through handleURLData.

Testing

Please trigger a new iOS adhoc build and re-test:

  1. Take a screenshot on iPhone
  2. Tap the screenshot preview thumbnail in the bottom-left corner
  3. Tap "Share", then select "Expensify"
  4. Verify the "Share to Expensify" page appears and the image is loaded
  5. Also verify sharing from the Photos app still works

@grgia
Copy link
Copy Markdown
Contributor

grgia commented Mar 24, 2026

Rebuilding

@github-actions
Copy link
Copy Markdown
Contributor

🚧 @grgia has triggered a test Expensify/App build. You can view the workflow run here.

@github-actions
Copy link
Copy Markdown
Contributor

🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
Built from App PR #84387.

Android 🤖 iOS 🍎
❌ FAILED ❌ ❌ FAILED ❌
The QR code can't be generated, because the Android build failed The QR code can't be generated, because the iOS build failed
Web 🕸️
https://84387.pr-testing.expensify.com
Web

👀 View the workflow run that generated this build 👀

@github-actions
Copy link
Copy Markdown
Contributor

🚧 @grgia has triggered a test Expensify/App build. You can view the workflow run here.

@github-actions
Copy link
Copy Markdown
Contributor

🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
Built from App PR #84387.

Android 🤖 iOS 🍎
❌ FAILED ❌ ❌ FAILED ❌
The QR code can't be generated, because the Android build failed The QR code can't be generated, because the iOS build failed
Web 🕸️
https://84387.pr-testing.expensify.com
Web

👀 View the workflow run that generated this build 👀

@grgia
Copy link
Copy Markdown
Contributor

grgia commented Mar 26, 2026

@MelvinBot merge main

…ionScreenshotData

Co-authored-by: Georgia Monahan <grgia@users.noreply.github.com>
@MelvinBot
Copy link
Copy Markdown
Contributor Author

Merged main into this branch.

@github-actions
Copy link
Copy Markdown
Contributor

🚧 @grgia has triggered a test Expensify/App build. You can view the workflow run here.

@MelvinBot
Copy link
Copy Markdown
Contributor Author

The failing check build (New Expensify Dev, DebugDevelopment, false) is unrelated to this PR's changes.

Analysis:

  • The only file changed in this PR is ios/ShareViewController/ShareViewController.swift
  • The ShareViewController target compiled successfully (only a deprecation warning for kUTTypeURL)
  • The build failed with Command SwiftCompile failed with a nonzero exit code in the ExpoModulesCore target — a CocoaPod dependency that this PR does not modify
  • The remote build cache was unavailable (NoSuchKey), forcing a full build — this likely exposed a pre-existing or transient build environment issue
  • Other recent PRs (e.g. Show deactivated Expensify Cards in workspace card management #86420) have this same check passing

Recommendation: Re-run the Remote Build iOS workflow. This appears to be a transient infrastructure/cache issue.

@github-actions
Copy link
Copy Markdown
Contributor

🚧 @grgia has triggered a test Expensify/App build. You can view the workflow run here.

@github-actions
Copy link
Copy Markdown
Contributor

🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
Built from App PR #84387.

Android 🤖 iOS 🍎
⏩ SKIPPED ⏩ https://ad-hoc-expensify-cash.s3.us-east-1.amazonaws.com/rock-artifacts/ad-hoc/rock-ios-device-AdHoc-b5fca79-23e70e6-d65d6f80e3485a1253fb9301aed069fcc0b04e01/index.html
The build for Android was skipped iOS
Web 🕸️
⏩ SKIPPED ⏩
The build for Web was skipped

👀 View the workflow run that generated this build 👀

@grgia
Copy link
Copy Markdown
Contributor

grgia commented Mar 26, 2026

@MelvinBot this is still not working on IOS physical device

loadFileRepresentation asks the provider for a file-based representation,
which fails for iOS screenshot previews that only register object-based
representations. loadObject is the Apple-recommended API that handles
both file-backed and in-memory image representations.

Co-authored-by: Georgia Monahan <grgia@users.noreply.github.com>
@MelvinBot
Copy link
Copy Markdown
Contributor Author

I pushed a new commit (bff9668) that fixes the root cause of the physical device failure.

What was wrong

The previous code used loadFileRepresentation(forTypeIdentifier:) to load images. This API asks the NSItemProvider to write its content to a temporary file. However, when sharing a screenshot from the iOS screenshot preview thumbnail, the system registers the image via object-based representations (using NSItemProviderWriting), not file-based ones. On a physical device, loadFileRepresentation fails because there is no file representation available for that item — the provider only has an in-memory object representation.

What the fix does

Replaced loadFileRepresentation with loadObject(ofClass: UIImage.self), which is Apple's recommended API for loading images from NSItemProvider. This API handles both file-backed and in-memory representations, so it works regardless of how the provider registered the image (screenshot preview, Photos app, clipboard, etc.).

The check also changed from hasItemConformingToTypeIdentifier(UTType.image) to canLoadObject(ofClass: UIImage.self), which directly asks "can you give me a UIImage?" rather than "do you have something that conforms to public.image?"

Testing

Please trigger a new iOS adhoc build and re-test:

  1. Take a screenshot on iPhone
  2. Tap the screenshot preview thumbnail in the bottom-left corner
  3. Tap "Share", then select "Expensify"
  4. Verify the "Share to Expensify" page appears and the image is loaded
  5. Also verify sharing from the Photos app still works

@github-actions
Copy link
Copy Markdown
Contributor

🚧 @grgia has triggered a test Expensify/App build. You can view the workflow run here.

@github-actions
Copy link
Copy Markdown
Contributor

🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
Built from App PR #84387.

Android 🤖 iOS 🍎
⏩ SKIPPED ⏩ https://ad-hoc-expensify-cash.s3.us-east-1.amazonaws.com/rock-artifacts/ad-hoc/rock-ios-device-AdHoc-bff9668-3063164-bc7ac9031d99fa10eb198f1019fe5396f3652a9a/index.html
The build for Android was skipped iOS
Web 🕸️
⏩ SKIPPED ⏩
The build for Web was skipped

👀 View the workflow run that generated this build 👀

@grgia
Copy link
Copy Markdown
Contributor

grgia commented Apr 2, 2026

@MelvinBot no share menu opens on ios with this build

loadObject(ofClass: UIImage.self) may not reliably invoke its completion
handler in iOS share extensions on physical devices, causing the extension
to hang indefinitely (group.leave never called, openMainApp never fires).

Switch to loadItem(forTypeIdentifier: "public.image") which is the older,
more battle-tested NSItemProvider API. This handles all possible return
types from the provider: UIImage (in-memory), Data (raw bytes), or URL
(file-backed). If the image path fails, falls back to loading via
public.data to ensure the extension never silently drops data.

Co-authored-by: Georgia Monahan <grgia@users.noreply.github.com>
@MelvinBot
Copy link
Copy Markdown
Contributor Author

I pushed a new commit (fbd2aa6) that replaces the loadObject(ofClass: UIImage.self) API with a more reliable approach.

What was wrong

The previous fix used loadObject(ofClass: UIImage.self) which relies on NSItemProviderReading protocol support. This API may not reliably invoke its completion handler in share extension contexts on physical iOS devices. When the callback never fires, group.leave() is never called, openMainApp() never executes, and the extension hangs indefinitely — which explains why "no share menu opens."

What the new commit does

Replaced loadObject with the older, more battle-tested loadItem(forTypeIdentifier: "public.image") API. Key changes:

  1. Uses hasItemConformingToTypeIdentifier(UTType.image.identifier) instead of canLoadObject(ofClass: UIImage.self) — checks the type hierarchy directly
  2. Uses loadItem(forTypeIdentifier: "public.image") instead of loadObject — this asks the provider for image data using the standard type identifier system rather than the object-based loading API
  3. Handles all possible return types from the callback: UIImage (in-memory), Data (raw bytes), or URL (file-backed) — covering screenshots, Photos app, and clipboard sources
  4. Falls back to public.data loading if the image path fails or returns an unrecognized type, ensuring the extension never silently drops data or hangs

Testing

Please trigger a new iOS adhoc build and re-test:

  1. Take a screenshot on iPhone
  2. Tap the screenshot preview thumbnail in the bottom-left corner
  3. Tap "Share", then select "Expensify"
  4. Verify the "Share to Expensify" page appears and the image is loaded
  5. Also verify sharing from the Photos app still works

@MelvinBot
Copy link
Copy Markdown
Contributor Author

The failing check (perf-tests) is unrelated to this PR's changes.

Analysis:

  • This PR only modifies ios/ShareViewController/ShareViewController.swift — a native iOS Swift file
  • The perf-test failure is: [ReportActionCompose] should press create button — render count difference exceeded the allowed deviation of 0 (current difference: 1)
  • This is a React render count regression test for a JavaScript component, which cannot be affected by a Swift-only change

Evidence: The PR changes zero JavaScript files. The ReportActionCompose render count regression is a pre-existing or flaky issue on main.

No fix is needed on this PR. This perf-test failure should be investigated separately.

@MelvinBot
Copy link
Copy Markdown
Contributor Author

The failing check (perf-tests) is unrelated to this PR.

Analysis: The perf test [ReportActionCompose] should press create button failed due to a render count deviation of 1. This PR only modifies ios/ShareViewController/ShareViewController.swift — a native iOS Swift file in the share extension. It cannot affect React component render counts.

Evidence: The PR changes zero JavaScript/TypeScript files. The Reassure performance test measures React component renders, which are entirely outside the scope of native iOS share extension code. This appears to be a flaky perf test — render count differences of 1 are inherently non-deterministic.

I attempted to re-run the failed check but don't have approval to do so. You can re-run it manually from the Actions tab.

@grgia
Copy link
Copy Markdown
Contributor

grgia commented Apr 7, 2026

Superseded by App#86984

@grgia grgia closed this Apr 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants