Bugfix FXIOS-13855 [Share Manager] Add remoteURL capability to .file share types for the Send to Device activity #30194

ih-codes · 2025-10-27T20:14:03Z

📜 Tickets

💡 Description

New:

This PR addresses the issue in FXIOS-13855 where online PDFs viewed in a tab did not show "Send Link to Device" as a share option in the share sheet. This was by original design, but the code has been updated now to include a "remote URL" for .file type shares. This means online PDFs viewed in a tab can now be shared via this remote URL to other Firefox synced devices, like your desktop browser.

Note that files shared from the Downloads Panel, or opened into a tab from the Downloads Panel, do not have a remote URL. That means "Send Link to Device" will not appear in the share options.

(Testing steps in ticket under QA notes)

Demo:

Share.Send.Link.to.Device.Demo.mov

Old:

Just a quick draft reference PR for discussion on #30059.

📝 Checklist

I filled in the ticket numbers and a description of my work
I updated the PR name to follow our PR naming guidelines
I ensured unit tests pass and wrote tests for new code
If working on UI, I checked and implemented accessibility (Dynamic Text and VoiceOver)
If adding telemetry, I read the data stewardship requirements and will request a data review
If adding or modifying strings, I read the guidelines and will request a string review from l10n
If needed, I updated documentation and added comments to complex code

…activity.

dicarobinho

LGTM!

mobiletest-ci-bot · 2025-10-30T08:08:37Z

	Warnings
⚠️	Changes detected in files: firefox-ios/Client/Frontend/Share/ShareTab.swift. Ensure that necessary updates are also ported to the WebEngine project if required (cc @lmarceau).

	Messages
📖	Project coverage: 37.19%

💪 Quality guardian

1 tests files modified. You're a champion of test coverage! 🚀

🥇 Perfect PR size

Smaller PRs are easier to review. Thanks for making life easy for reviewers! ✨

💬 Description craftsman

Great PR description! Reviewers salute you 🫡

❌ Per-file test coverage gate

The following changed file(s) are below 35.0% coverage:

File	Coverage	Required
`firefox-ios/Client/Coordinators/ShareSheetCoordinator.swift`	27.4%	35.0%
`firefox-ios/Client/Frontend/Browser/MainMenuActionHelper.swift`	0.0%	35.0%

Client.app: Coverage: 37.11

File	Coverage
MainMenuActionHelper.swift	0.0%	⚠️
DownloadsPanel.swift	35.03%	⚠️
ShareSheetCoordinator.swift	27.44%	⚠️
ShareManager.swift	97.25%	✅
ShareType.swift	95.0%	✅
DownloadsCoordinator.swift	57.38%	✅
BrowserCoordinator.swift	68.01%	✅

Generated by 🚫 Danger Swift against f85003b

ih-codes · 2025-10-30T15:14:01Z

Thanks @dicarobinho for testing and updating the unit tests! 🎉 Opening this PR now.

cyndichin

thanks for adding this feature and all the clarifying comments 🔥 ~~ I added some thoughts I had while reviewing and some nits. Nothing blocking from my side.

I also followed the QA steps you listed and it looks good 👍 . Nice work!

firefox-ios/Client/Coordinators/Browser/BrowserCoordinator.swift

cyndichin · 2025-11-03T14:10:57Z

firefox-ios/firefox-ios-tests/Tests/ClientTests/Sharing/ShareManagerTests.swift


        let activityItems = ShareManager.getActivityItems(
-            forShareType: .file(url: testFileURL),
+            forShareType: .file(url: testFileURL, remoteURL: nil),


Do we want to add tests for when remoteURL != nil.

Good call, I'll add this. Also, I'll look at adding some SendToDeviceActivity tests since I noticed we don't have any specific to custom app activities. It's a little different than the other share types.

cyndichin · 2025-11-03T14:14:23Z

firefox-ios/Client/Frontend/Browser/MainMenuActionHelper.swift

        TelemetryWrapper.recordEvent(category: .action, method: .tap, object: .sharePageWith)
        navigationHandler?.showShareSheet(
-            shareType: .file(url: fileURL),
+            shareType: .file(url: fileURL, remoteURL: remoteURL),


instead of passing in nil, could we set remoteURL: nil and add the comment here instead. Unless there is an existing case in which we want the value to not be nil?

Ahh good catch, I'll remove the param and just set nil here with a comment now!

firefox-ios/Client/Coordinators/ShareSheetCoordinator.swift

cyndichin · 2025-11-03T14:22:04Z

firefox-ios/Client/Coordinators/Browser/BrowserCoordinator.swift


        // If we successfully got a temp file URL, share it like a downloaded file
-        return .file(url: fileURL)
+        return .file(url: fileURL, remoteURL: shareType.wrappedURL)


I was a bit confused at first since the wrappedURL is pulling from the url and not remoteURL in ShareType, but it seems this is in the right format so we use the existing shareType url for the new shared type.

Edit: Replied to the wrong comment, let me move what I'd said to the right chain!

Yeahhhhh I was a bit concerned the 2 URLs might be confusing. I am open to adjusting the naming or documentation anywhere I can to make this clearer. 🙏

The distinction here is that the wrapped file URL is always a file:// URL, whereas the remoteURL will always (if present) link to online web content (a https:// type URL).

If you could elaborate why we are passing in the wrapped file in the remoteURL that would be helpful.

So, tryDownloadingTabFileToShare will download the PDF in the background when a user views an online PDF in the browser. Then, if the user tries to share the current tab, the share activity gets the file to attach to emails, iMessages, etc. But for Send to Device, we actually just want to send the link, not the downloaded file. So the shareType passed into tryDownloadingTabFileToShare is always a tab share and thus we can get the "remote URL" from the tab share's wrapped URL. I will add some comments to hopefully make this a little clearer.

Ahhh okay that makes sense. Yeah I think if you add that, it will be a lot more helpful. Maybe adding this would help too? With a clarifying comment above?
let tabRemoteUrl = shareType.wrappedURL

Actually, I just finished restructuring this function and adding some comments. I think you'll find it a lot clearer now since it can only apply the logic to a tabURL and a ShareTabe instead of a generic ShareType. Will commit soon after I test! 😁

See if you think it's clearer now @cyndichin !

Oh yes much better!!! Thank you for restructuring it! 🥳

…ile-remote-url * mozilla/main: (45 commits) Bugfix FXIOS-13995 ⁃ Toggle for “Website Dark Mode” is labeled “Turn On Website Dark Mode” instead of using On/Off state (#30333) Revert FXIOS-13998 - "Add FXIOS-13799 - Update keyboard's accessory view for iOS 26 (#30058)" (#30334) Bugfix FXIOS-13771 ⁃ The initial menu needs to be scrolled (#30326) (Local AS flow) Nightly auto-update (146.0.20251101050307) (#30317) Bugfix FXIOS-13925 ⁃ [New Error Pages] [Accessibility] - Homepage items announced with voice over (#30292) Update Firefox icons from Acorn repo (#30323) Add FXIOS-13673 [Debug Menu] New item to remove credit card encryption keys for testing (#29656) Refactor FXIOS-12995 [Swift 6 Migration] Update NotificationCenter usage to use Notifiable pattern (part 1) (#30250 Add FXIOS-30299 [Stories Feed] Use standalone tab for stories web view (#30300) [no-ticket] Disable 18.x for Robo tests (#30303) Refactor FXIOS-12796 [Swift 6 Migration] Fix GleanPlumbMessageManager shared state warnings (#30286) Refactor FXIOS-12796 [Swift 6 Migration] Fix misc. missing "final" class warnings for `Sendable` conforming classes (#30287) Refactor FXIOS-13481 [Swift 6 Migration] Turn on Concurrency for ComponenetLibrary Tests in BrowserKit (#29763) Refactor FXIOS-13481 [Swift 6 Migration] Turn on Concurrency for OnboardingKitTests Tests in BrowserKit (#29775) Refactor FXIOS-13944 [Swift 6] remove nonisolated from unsubscribeFromRedux (#30276) Bump version to 145.1 Add FXIOS-13967 [Translations] toast UI for handling error messages (#30278) Add FXIOS-13648 [Trending Searches] initial recent searches UI (#30274) Bugfix FXIOS-13970 [Homepage] Blank homepage on iOS 15 (#30282) Bugfix FXIOS-13881 [iOS 26] [New first run onboarding] Add glass effect to default browser popup (#30071) ...

…tation for clarity.

ih-codes · 2025-11-03T18:47:27Z

@cyndichin I've addressed all your comments and added a bunch more tests!

firefox-ios/firefox-ios-tests/Tests/ClientTests/Sharing/ShareManagerTests.swift

cyndichin

Looks good to me now, way better!! Thank you!!! Seems like there may be a dupe in the tests? If it passes CI, then good to merge! Don't want to block ya since its towards my EOD.

ih-codes · 2025-11-03T19:10:12Z

Looks good to me now, way better!! Thank you!!! Seems like there may be a dupe in the tests? If it passes CI, then good to merge! Don't want to block ya since its towards my EOD.

Yep whoops, just noticed the dupe I pushed up before my meeting. It's fixed now! 😆 Always fun writing tests for really old code (Send to Device) lol. Thanks for the thorough review! 🙏

github-actions · 2025-11-03T19:59:50Z

🚀 PR merged to main, targeting version: 145.1

ih-codes and others added 3 commits October 27, 2025 14:11

Add remoteURL capability to .file share types for the Send to Device …

0f18c4f

…activity.

Merge branch 'main' into ih/FXIOS-13855-add-file-remote-url

5cac52c

Fixed ShareManagerTests

52cdc33

dicarobinho approved these changes Oct 30, 2025

View reviewed changes

ih-codes marked this pull request as ready for review October 30, 2025 15:13

ih-codes requested a review from a team as a code owner October 30, 2025 15:13

ih-codes requested a review from cyndichin October 30, 2025 15:13

ih-codes changed the title ~~Bugfix FXIOS-13855 (Draft for discussion) Add remoteURL capability to .file share types for the Send to Device activity~~ Bugfix FXIOS-13855 [Share Manager] Add remoteURL capability to .file share types for the Send to Device activity Oct 30, 2025

Small fixes and doc updates.

ad616f4

cyndichin approved these changes Nov 3, 2025

View reviewed changes

ih-codes added 3 commits November 3, 2025 09:42

Review updates round 1.

c061103

Restructure the tryDownloadingTabFileToShare method and add documen…

914ee99

…tation for clarity.

cyndichin reviewed Nov 3, 2025

View reviewed changes

firefox-ios/firefox-ios-tests/Tests/ClientTests/Sharing/ShareManagerTests.swift Show resolved Hide resolved

cyndichin approved these changes Nov 3, 2025

View reviewed changes

Add new share manager tests.

f85003b

ih-codes force-pushed the ih/FXIOS-13855-add-file-remote-url branch from 7b18723 to f85003b Compare November 3, 2025 19:09

ih-codes merged commit b869a89 into main Nov 3, 2025
9 checks passed

ih-codes deleted the ih/FXIOS-13855-add-file-remote-url branch November 3, 2025 19:59

Bugfix FXIOS-13855 [Share Manager] Add remoteURL capability to .file share types for the Send to Device activity #30194

Bugfix FXIOS-13855 [Share Manager] Add remoteURL capability to .file share types for the Send to Device activity #30194

Uh oh!

Conversation

ih-codes commented Oct 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📜 Tickets

💡 Description

New:

Demo:

Old:

📝 Checklist

Uh oh!

dicarobinho left a comment

Choose a reason for hiding this comment

Uh oh!

mobiletest-ci-bot commented Oct 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💪 Quality guardian

🥇 Perfect PR size

💬 Description craftsman

❌ Per-file test coverage gate

Client.app: Coverage: 37.11

Uh oh!

ih-codes commented Oct 30, 2025

Uh oh!

cyndichin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ih-codes Nov 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ih-codes commented Nov 3, 2025

Uh oh!

Uh oh!

cyndichin left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ih-codes commented Nov 3, 2025

Uh oh!

Uh oh!

github-actions bot commented Nov 3, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ih-codes commented Oct 27, 2025 •

edited

Loading

mobiletest-ci-bot commented Oct 30, 2025 •

edited

Loading

ih-codes Nov 3, 2025 •

edited

Loading

cyndichin left a comment •

edited

Loading