Handle raw Data type in iOS share extension for screenshot sharing#84387
Handle raw Data type in iOS share extension for screenshot sharing#84387
Conversation
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>
|
🚧 @grgia has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
|
@MelvinBot I tested this build on an IOS device and it does not fix the issue |
|
🚧 @grgia has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
@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>
|
I investigated why the previous fix wasn't working and pushed a new commit ( What was wrongThe original fix added a What the new commit fixes1. Uses
2. Fixes TestingPlease trigger a new iOS adhoc build and re-test:
|
|
Rebuilding |
|
🚧 @grgia has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚧 @grgia has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
@MelvinBot merge main |
…ionScreenshotData Co-authored-by: Georgia Monahan <grgia@users.noreply.github.com>
|
Merged main into this branch. |
|
🚧 @grgia has triggered a test Expensify/App build. You can view the workflow run here. |
|
The failing check Analysis:
Recommendation: Re-run the |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚧 @grgia has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
@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>
|
I pushed a new commit ( What was wrongThe previous code used What the fix doesReplaced The check also changed from TestingPlease trigger a new iOS adhoc build and re-test:
|
|
🚧 @grgia has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
@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>
|
I pushed a new commit ( What was wrongThe previous fix used What the new commit doesReplaced
TestingPlease trigger a new iOS adhoc build and re-test:
|
|
The failing check ( Analysis:
Evidence: The PR changes zero JavaScript files. The No fix is needed on this PR. This perf-test failure should be investigated separately. |
|
The failing check ( Analysis: The perf test 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. |
|
Superseded by App#86984 |
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
Databytes rather than anNSURLorUIImage. The share extension'shandleDatamethod inShareViewController.swiftwas missing a case for this type, so the data silently fell through to the error handler. The app would still open (becauseopenMainApp()is called regardless of errors inprocessAttachments) but no file was saved to the shared container, resulting in nothing happening.This adds a
Datahandler that first attempts to decode the bytes as aUIImage(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
Offline tests
N/A — the share extension operates on local data before opening the app.
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.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