Skip to content

Commit b869a89

Browse files
Bugfix FXIOS-13855 [Share Manager] Add remoteURL capability to .file share types for the Send to Device activity (#30194)
* Add remoteURL capability to .file share types for the Send to Device activity. * Small fixes and doc updates. * Restructure the `tryDownloadingTabFileToShare` method and add documentation for clarity. * Add new share manager tests. --------- Co-authored-by: Alexandru Farcasanu <[email protected]>
1 parent 72945af commit b869a89

File tree

10 files changed

+161
-36
lines changed

10 files changed

+161
-36
lines changed
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
APP_VERSION = 145.0
1+
APP_VERSION = 145.1

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

Lines changed: 29 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -680,9 +680,13 @@ class BrowserCoordinator: BaseCoordinator,
680680

681681
func presentSavePDFController() {
682682
guard let selectedTab = browserViewController.tabManager.selectedTab else { return }
683+
683684
if selectedTab.mimeType == MIMEType.PDF {
684685
showShareSheetForCurrentlySelectedTab()
685686
} else {
687+
// Online PDFs viewed in a tab can be shared via this URL to other Firefox synced devices with Send to Device.
688+
let remoteURL = selectedTab.webView?.url
689+
686690
selectedTab.webView?.createPDF { [weak self] result in
687691
guard let self else { return }
688692
switch result {
@@ -694,7 +698,7 @@ class BrowserCoordinator: BaseCoordinator,
694698
do {
695699
try data.write(to: outputURL)
696700
startShareSheetCoordinator(
697-
shareType: .file(url: outputURL),
701+
shareType: .file(url: outputURL, remoteURL: remoteURL),
698702
shareMessage: nil,
699703
sourceView: self.browserViewController.addressToolbarContainer,
700704
sourceRect: nil,
@@ -864,7 +868,7 @@ class BrowserCoordinator: BaseCoordinator,
864868
/// There are many ways to share many types of content from various areas of the app. Code paths that go through this
865869
/// method include:
866870
/// * Sharing content from a long press on Home screen tiles (e.g. long press Jump Back In context menu)
867-
/// * From the old Menu > Share and the new Menu > Tools > Share
871+
/// * From the old Menu > Share and the new Menu > More > Share
868872
/// * From the new toolbar share button beside the address bar
869873
/// * From long pressing a link in the WKWebView and sharing from the context menu (via ActionProviderBuilder > addShare)
870874
/// * Via the sharesheet deeplink path in `RouteBuilder` (e.g. tapping home cards that initiate sharing content)
@@ -894,8 +898,13 @@ class BrowserCoordinator: BaseCoordinator,
894898
// FXIOS-10824 It's strange if the user has to wait a long time to download files that are literally already
895899
// being shown in the webview.
896900
var overrideShareType = shareType
897-
if case ShareType.tab = shareType {
898-
overrideShareType = await tryDownloadingTabFileToShare(shareType: shareType)
901+
if case let ShareType.tab(url, tab) = shareType {
902+
// For tabs displaying content other than HTML MIME types, we can download the temporary document (i.e. a PDF
903+
// file) and share that instead.
904+
overrideShareType = await tryDownloadingTabFileToShare(
905+
withTabURL: url,
906+
forShareTab: tab
907+
)
899908
}
900909

901910
await MainActor.run { [weak self, overrideShareType] in
@@ -1335,21 +1344,26 @@ class BrowserCoordinator: BaseCoordinator,
13351344

13361345
// MARK: - Private helpers
13371346

1338-
nonisolated private func tryDownloadingTabFileToShare(shareType: ShareType) async -> ShareType {
1339-
// We can only try to download files for `.tab` type shares that have a TemporaryDocument
1340-
guard case let ShareType.tab(_, tab) = shareType,
1341-
let temporaryDocument = await tab.temporaryDocument,
1342-
!temporaryDocument.isDownloading else {
1343-
return shareType
1344-
}
1345-
1346-
guard let fileURL = await temporaryDocument.download() else {
1347+
/// Tabs displaying content other than a HTML MIME type can be downloaded and treated as files when shared. This method
1348+
/// attempts to download any such files. If there is no file to download, returns just a regular `ShareType.tab`.
1349+
/// - Parameters:
1350+
/// - tabURL: The URL for the tab pointing to a website.
1351+
/// - tab: The current tab displaying the tabURL.
1352+
/// - Returns: Returns a `ShareType.file` containing a `file://` URL that points to a downloaded file on the device. If
1353+
/// no file was downloaded, then just returns a regular `ShareType.tab` with the passed in `tabURL` and `tab`.
1354+
private func tryDownloadingTabFileToShare(
1355+
withTabURL tabURL: URL,
1356+
forShareTab tab: ShareTab
1357+
) async -> ShareType {
1358+
guard let temporaryDocument = tab.temporaryDocument,
1359+
!temporaryDocument.isDownloading,
1360+
let fileURL = await temporaryDocument.download() else {
13471361
// If no file was downloaded, simply share the tab as usual with a web URL
1348-
return shareType
1362+
return .tab(url: tabURL, tab: tab)
13491363
}
13501364

13511365
// If we successfully got a temp file URL, share it like a downloaded file
1352-
return .file(url: fileURL)
1366+
return .file(url: fileURL, remoteURL: tabURL)
13531367
}
13541368

13551369
/// Utility. Performs the supplied action if a coordinator of the indicated type

firefox-ios/Client/Coordinators/Library/DownloadsCoordinator.swift

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,12 @@ class DownloadsCoordinator: BaseCoordinator,
7272
tabManager: tabManager
7373
)
7474
add(child: coordinator)
75+
76+
// Since this file is already downloaded, we don't have a remote URL to use for the "Send to Device" activity
77+
let shareType = ShareType.file(url: file.path, remoteURL: nil)
78+
7579
coordinator.start(
76-
shareType: .file(url: file.path),
80+
shareType: shareType,
7781
shareMessage: nil,
7882
sourceView: sourceView,
7983
sourceRect: nil,

firefox-ios/Client/Coordinators/ShareSheetCoordinator.swift

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -90,17 +90,26 @@ class ShareSheetCoordinator: BaseCoordinator,
9090
// be necessary, but this JS alert code is fragile right now so let's not touch it until FXIOS-10334 is underway.
9191
switch activityType {
9292
case CustomActivityAction.sendToDevice.actionType:
93-
// Cannot send file:// URLs to another synced device
94-
guard !shareType.wrappedURL.isFileURL else {
93+
var sendURL: URL = shareType.wrappedURL
94+
95+
if case .file(_, let remoteURL) = shareType,
96+
let remoteURL = remoteURL {
97+
// Some files might have a remote URL as a fallback for Send to Device (i.e. PDFs navigated to in a tab, but
98+
// not from Downloads Panel).
99+
sendURL = remoteURL
100+
}
101+
102+
// Note: Cannot send file:// URLs to another synced device
103+
guard !sendURL.isFileURL else {
95104
dequeueNotShownJSAlert()
96105
return
97106
}
98107

99108
switch shareType {
100109
case let .tab(_, tab):
101-
showSendToDevice(url: shareType.wrappedURL, relatedTab: tab)
110+
showSendToDevice(url: sendURL, relatedTab: tab)
102111
default:
103-
showSendToDevice(url: shareType.wrappedURL, relatedTab: nil)
112+
showSendToDevice(url: sendURL, relatedTab: nil)
104113
}
105114
case .copyToPasteboard:
106115
if case .file = shareType {

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -668,8 +668,10 @@ final class MainMenuActionHelper: PhotonActionSheetProtocol,
668668
@MainActor
669669
private func share(fileURL: URL, buttonView: UIView) {
670670
TelemetryWrapper.recordEvent(category: .action, method: .tap, object: .sharePageWith)
671+
672+
// Since this file is already downloaded, we don't have a remote URL to use for the "Send to Device" activity
671673
navigationHandler?.showShareSheet(
672-
shareType: .file(url: fileURL),
674+
shareType: .file(url: fileURL, remoteURL: nil),
673675
shareMessage: nil,
674676
sourceView: buttonView,
675677
sourceRect: nil,

firefox-ios/Client/Frontend/Library/Downloads/DownloadsPanel.swift

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,8 +157,11 @@ class DownloadsPanel: UIViewController,
157157
}
158158

159159
private func shareDownloadedFile(_ downloadedFile: DownloadedFile, indexPath: IndexPath) {
160+
// Since this file is already downloaded, we don't have a remote URL to use for the "Send to Device" activity
161+
let shareType = ShareType.file(url: downloadedFile.path, remoteURL: nil)
162+
160163
let shareActivityViewController = ShareManager.createActivityViewController(
161-
shareType: .file(url: downloadedFile.path),
164+
shareType: shareType,
162165
shareMessage: nil,
163166
completionHandler: { _, _ in }
164167
)

firefox-ios/Client/Frontend/Share/ShareManager.swift

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ class ShareManager: NSObject {
5353
var activityItems: [Any] = []
5454

5555
switch shareType {
56-
case .file(let fileURL):
56+
case .file(let fileURL, _):
5757
activityItems.append(URLActivityItemProvider(url: fileURL))
5858

5959
if let explicitShareMessage {
@@ -121,11 +121,22 @@ class ShareManager: NSObject {
121121
}
122122

123123
@MainActor
124-
private static func getApplicationActivities(forShareType shareType: ShareType) -> [UIActivity] {
124+
static func getApplicationActivities(forShareType shareType: ShareType) -> [UIActivity] {
125125
var appActivities = [UIActivity]()
126126

127-
// Only acts on non-file URLs to send links to synced devices. Will ignore file URLs it can't handle.
128-
appActivities.append(SendToDeviceActivity(activityType: .sendToDevice, url: shareType.wrappedURL))
127+
// Set up the "Send to Device" activity, which shares URLs between a Firefox account user's synced devices. We can
128+
// only share URLs to real websites, not internal `file://` URLs.
129+
switch shareType {
130+
case .file(_, let remoteURL):
131+
// Some downloaded files may have an associated remote URL (if the file was just downloaded in the tab).
132+
// Files which are shared from the Downloads Panel will NOT have any associated remote URL (we don't store that
133+
// history).
134+
if let remoteURL {
135+
appActivities.append(SendToDeviceActivity(activityType: .sendToDevice, url: remoteURL))
136+
}
137+
default:
138+
appActivities.append(SendToDeviceActivity(activityType: .sendToDevice, url: shareType.wrappedURL))
139+
}
129140

130141
return appActivities
131142
}

firefox-ios/Client/Frontend/Share/ShareTab.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ protocol ShareTab: Sendable {
1414
@MainActor
1515
var webView: TabWebView? { get }
1616

17-
// Tabs displaying content other than HTML mime type can optionally be downloaded and treated as files when shared
17+
// Tabs displaying content other than a HTML MIME type can optionally be downloaded and treated as files when shared.
1818
@MainActor
1919
var temporaryDocument: TemporaryDocument? { get }
2020
}

firefox-ios/Client/Frontend/Share/ShareType.swift

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,10 @@
55
import Foundation
66

77
/// Preconfigured sharing schemes which the share manager knows how to handle.
8-
/// file: Include a file URL (`file://`). Best used for sharing downloaded files.
8+
/// file: Include a file URL (`file://`). Best used for sharing downloaded files. If possible, the remote URL is saved
9+
/// as well for `Send to Device` and other activities which share the content off-device without an attachment.
10+
/// Note that remote URLs are only set for online files (e.g. PDFs viewed in the tab), not downloaded files (PDFs
11+
/// opened in a tab from the DownloadsPanel).
912
/// site: Include a website URL (`http(s)://`). Best used for sharing library/bookmarks, etc. without an active tab.
1013
/// Shares configured using .site will not append a title to Messages but will have a subtitle in Mail.
1114
/// tab: Include a URL and a tab to share. If sharing a tab with an active webView, then additional sharing
@@ -14,14 +17,14 @@ import Foundation
1417
/// scheme instead of `http(s)://`, so certain options, like Send to Device / Add to Home Screen, may not be
1518
/// available.
1619
enum ShareType {
17-
case file(url: URL)
20+
case file(url: URL, remoteURL: URL?)
1821
case site(url: URL)
1922
case tab(url: URL, tab: any ShareTab)
2023

2124
/// The share URL wrapped by the given type.
2225
var wrappedURL: URL {
2326
switch self {
24-
case let .file(url):
27+
case let .file(url, _):
2528
return url
2629
case let .site(url):
2730
return url

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

Lines changed: 84 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,37 @@ final class ShareManagerTests: XCTestCase {
3232

3333
// MARK: - Test sharing a file
3434

35-
func testGetActivityItems_forFileURL_withNoShareText() throws {
35+
func testGetActivityItems_forFileURL_withNoRemoteURL_withNoShareText() throws {
3636
let testShareActivityType = UIActivity.ActivityType.message
3737

3838
let activityItems = ShareManager.getActivityItems(
39-
forShareType: .file(url: testFileURL),
39+
forShareType: .file(url: testFileURL, remoteURL: nil),
40+
withExplicitShareMessage: nil
41+
)
42+
43+
let urlActivityItemProvider = try XCTUnwrap(activityItems[safe: 0] as? URLActivityItemProvider)
44+
let itemForURLActivity = urlActivityItemProvider.activityViewController(
45+
createStubActivityViewController(),
46+
itemForActivityType: testShareActivityType
47+
)
48+
49+
let telemetryActivityItemProvider = try XCTUnwrap(activityItems[safe: 1] as? ShareTelemetryActivityItemProvider)
50+
let itemForShareActivity = telemetryActivityItemProvider.activityViewController(
51+
createStubActivityViewController(),
52+
itemForActivityType: testShareActivityType
53+
)
54+
55+
XCTAssertEqual(activityItems.count, 2)
56+
XCTAssertEqual(itemForURLActivity as? URL, testFileURL)
57+
XCTAssertTrue(itemForShareActivity is NSNull)
58+
}
59+
60+
func testGetActivityItems_forFileURL_withRemoteURL_withNoShareText() throws {
61+
let testShareActivityType = UIActivity.ActivityType.message
62+
63+
// Should be no difference with or without remoteURL
64+
let activityItems = ShareManager.getActivityItems(
65+
forShareType: .file(url: testFileURL, remoteURL: testWebURL),
4066
withExplicitShareMessage: nil
4167
)
4268

@@ -62,7 +88,7 @@ final class ShareManagerTests: XCTestCase {
6288
let testMessage = "Test message"
6389
let testSubtitle = "Test subtitle"
6490
let activityItems = ShareManager.getActivityItems(
65-
forShareType: .file(url: testFileURL),
91+
forShareType: .file(url: testFileURL, remoteURL: nil),
6692
withExplicitShareMessage: ShareMessage(message: testMessage, subtitle: testSubtitle)
6793
)
6894

@@ -109,7 +135,7 @@ final class ShareManagerTests: XCTestCase {
109135
let testShareActivityType = UIActivity.ActivityType.message
110136

111137
let activityItems = ShareManager.getActivityItems(
112-
forShareType: .file(url: testWebURL),
138+
forShareType: .file(url: testWebURL, remoteURL: nil),
113139
withExplicitShareMessage: nil
114140
)
115141

@@ -136,7 +162,7 @@ final class ShareManagerTests: XCTestCase {
136162
let testSubtitle = "Test subtitle"
137163

138164
let activityItems = ShareManager.getActivityItems(
139-
forShareType: .file(url: testWebURL),
165+
forShareType: .file(url: testWebURL, remoteURL: nil),
140166
withExplicitShareMessage: ShareMessage(message: testMessage, subtitle: testSubtitle)
141167
)
142168

@@ -312,6 +338,59 @@ final class ShareManagerTests: XCTestCase {
312338
XCTAssertTrue(itemForShareActivity is NSNull)
313339
}
314340

341+
// MARK: - Custom SendToDeviceActivity
342+
343+
func testCustomApplicationActivities_forSiteShare() throws {
344+
let testShareActivityType = UIActivity.ActivityType("org.mozilla.ios.Fennec.sendToDevice")
345+
let testActivityTitle = "Send Link to Device"
346+
347+
let testShareType = ShareType.site(url: testWebURL)
348+
349+
let activityItems = ShareManager.getApplicationActivities(forShareType: testShareType)
350+
351+
let customActivityType = try XCTUnwrap(activityItems[safe: 0] as? SendToDeviceActivity)
352+
XCTAssertEqual(activityItems.count, 1)
353+
XCTAssertEqual(customActivityType.activityTitle, testActivityTitle)
354+
XCTAssertEqual(customActivityType.activityType, testShareActivityType)
355+
}
356+
357+
func testCustomApplicationActivities_forTabShare() throws {
358+
let testShareActivityType = UIActivity.ActivityType("org.mozilla.ios.Fennec.sendToDevice")
359+
let testActivityTitle = "Send Link to Device"
360+
361+
let testShareType = ShareType.tab(url: testWebURL, tab: testTab)
362+
363+
let activityItems = ShareManager.getApplicationActivities(forShareType: testShareType)
364+
365+
let customActivityType = try XCTUnwrap(activityItems[safe: 0] as? SendToDeviceActivity)
366+
XCTAssertEqual(activityItems.count, 1)
367+
XCTAssertEqual(customActivityType.activityTitle, testActivityTitle)
368+
XCTAssertEqual(customActivityType.activityType, testShareActivityType)
369+
}
370+
371+
func testCustomApplicationActivities_forFileShareWithRemoteURL_AddsSendToDevice() throws {
372+
let testShareActivityType = UIActivity.ActivityType("org.mozilla.ios.Fennec.sendToDevice")
373+
let testActivityTitle = "Send Link to Device"
374+
375+
let testShareType = ShareType.file(url: testFileURL, remoteURL: testWebURL)
376+
377+
let activityItems = ShareManager.getApplicationActivities(forShareType: testShareType)
378+
379+
let customActivityType = try XCTUnwrap(activityItems[safe: 0] as? SendToDeviceActivity)
380+
XCTAssertEqual(activityItems.count, 1)
381+
XCTAssertEqual(customActivityType.activityTitle, testActivityTitle)
382+
XCTAssertEqual(customActivityType.activityType, testShareActivityType)
383+
}
384+
385+
func testCustomApplicationActivities_forFileShareWithNoRemoteURL_DoesNotAddSendToDevice() throws {
386+
// Simulate file share for downloaded files
387+
let testShareType = ShareType.file(url: testFileURL, remoteURL: nil)
388+
389+
let activityItems = ShareManager.getApplicationActivities(forShareType: testShareType)
390+
391+
XCTAssertEqual(activityItems.count, 0)
392+
}
393+
315394
// MARK: - Helpers
316395

317396
private func createStubActivityViewController() -> UIActivityViewController {

0 commit comments

Comments
 (0)