diff --git a/BrowserKit/Sources/Common/Logger/LoggerCategory.swift b/BrowserKit/Sources/Common/Logger/LoggerCategory.swift index 90b26f647f6c..7712499432ca 100644 --- a/BrowserKit/Sources/Common/Logger/LoggerCategory.swift +++ b/BrowserKit/Sources/Common/Logger/LoggerCategory.swift @@ -48,6 +48,9 @@ public enum LoggerCategory: String { /// Related to the setup of services on app launch. case setup + /// Related to showing the share sheet from multiple places in the app. + case shareSheet + /// Related to storage (keychain, SQL database, store of different types, etc). case storage diff --git a/firefox-ios/Client.xcodeproj/project.pbxproj b/firefox-ios/Client.xcodeproj/project.pbxproj index feac008249dc..be06355bd635 100644 --- a/firefox-ios/Client.xcodeproj/project.pbxproj +++ b/firefox-ios/Client.xcodeproj/project.pbxproj @@ -1844,9 +1844,11 @@ ED371A682CB881EF0099F3C8 /* SearchEngineSelectionCoordinator.swift in Sources */ = {isa = PBXBuildFile; fileRef = ED371A672CB881EF0099F3C8 /* SearchEngineSelectionCoordinator.swift */; }; ED390D202CF4DE2E00A775F9 /* URLActivityItemProvider.swift in Sources */ = {isa = PBXBuildFile; fileRef = ED390D1F2CF4DE2E00A775F9 /* URLActivityItemProvider.swift */; }; ED390D222CF4DEDB00A775F9 /* TitleSubtitleActivityItemProvider.swift in Sources */ = {isa = PBXBuildFile; fileRef = ED390D212CF4DEDB00A775F9 /* TitleSubtitleActivityItemProvider.swift */; }; + ED4294852CF1286E0077D7CB /* ShareManagerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = ED4294842CF1286E0077D7CB /* ShareManagerTests.swift */; }; ED45893E2CC800D9006F2C0B /* SearchEngineSelectionViewControllerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = ED45893D2CC800D9006F2C0B /* SearchEngineSelectionViewControllerTests.swift */; }; ED4589402CC8220A006F2C0B /* MockSearchEngineSelectionCoordinator.swift in Sources */ = {isa = PBXBuildFile; fileRef = ED45893F2CC8220A006F2C0B /* MockSearchEngineSelectionCoordinator.swift */; }; ED55DC8C2CC2D7DA00E3FE3A /* SearchEngineSelectionCoordinatorTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = ED55DC8B2CC2D7DA00E3FE3A /* SearchEngineSelectionCoordinatorTests.swift */; }; + ED575BD32D00F9D00056BCDA /* MockTemporaryDocument.swift in Sources */ = {isa = PBXBuildFile; fileRef = ED575BD22D00F9D00056BCDA /* MockTemporaryDocument.swift */; }; ED6C8DAC2CE6A4BB00D7F7F3 /* Sentry-Dynamic in Frameworks */ = {isa = PBXBuildFile; productRef = ED6C8DAB2CE6A4BB00D7F7F3 /* Sentry-Dynamic */; }; ED70CD812CE3BD2C0018761B /* MockStoreForMiddleware.swift in Sources */ = {isa = PBXBuildFile; fileRef = ED70CD802CE3BD2C0018761B /* MockStoreForMiddleware.swift */; }; ED7A08DB2CF674730035EC8F /* ShareMessage.swift in Sources */ = {isa = PBXBuildFile; fileRef = ED7A08DA2CF674730035EC8F /* ShareMessage.swift */; }; @@ -9377,10 +9379,12 @@ ED371A672CB881EF0099F3C8 /* SearchEngineSelectionCoordinator.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SearchEngineSelectionCoordinator.swift; sourceTree = ""; }; ED390D1F2CF4DE2E00A775F9 /* URLActivityItemProvider.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = URLActivityItemProvider.swift; sourceTree = ""; }; ED390D212CF4DEDB00A775F9 /* TitleSubtitleActivityItemProvider.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TitleSubtitleActivityItemProvider.swift; sourceTree = ""; }; + ED4294842CF1286E0077D7CB /* ShareManagerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ShareManagerTests.swift; sourceTree = ""; }; ED45893D2CC800D9006F2C0B /* SearchEngineSelectionViewControllerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SearchEngineSelectionViewControllerTests.swift; sourceTree = ""; }; ED45893F2CC8220A006F2C0B /* MockSearchEngineSelectionCoordinator.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockSearchEngineSelectionCoordinator.swift; sourceTree = ""; }; ED5144A0BE3A8C7B15047C3F /* anp */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = anp; path = anp.lproj/3DTouchActions.strings; sourceTree = ""; }; ED55DC8B2CC2D7DA00E3FE3A /* SearchEngineSelectionCoordinatorTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SearchEngineSelectionCoordinatorTests.swift; sourceTree = ""; }; + ED575BD22D00F9D00056BCDA /* MockTemporaryDocument.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockTemporaryDocument.swift; sourceTree = ""; }; ED70CD802CE3BD2C0018761B /* MockStoreForMiddleware.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockStoreForMiddleware.swift; sourceTree = ""; }; ED7A08DA2CF674730035EC8F /* ShareMessage.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ShareMessage.swift; sourceTree = ""; }; ED7A08DC2CF6749B0035EC8F /* ShareType.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ShareType.swift; sourceTree = ""; }; @@ -12771,6 +12775,7 @@ 8A7653C328A2E68B00924ABF /* MockPocketAPI.swift */, 281B2BE91ADF4D90002917DC /* MockProfile.swift */, ED7A08E02CF679CE0035EC8F /* MockShareTab.swift */, + ED575BD22D00F9D00056BCDA /* MockTemporaryDocument.swift */, 8A7A26E029D4785900EA76F1 /* MockRouter.swift */, C8C3FE9C29F907B30038E3BA /* MockSearchHandlerRouteCoordinator.swift */, 21D884402A79628E00AF144C /* MockSettingsDelegate.swift */, @@ -14135,6 +14140,7 @@ D3BA41671BD82F2200DA5457 /* XCTestCaseExtensions.swift */, 8A96C4BA28F9E7B300B75884 /* XCTestCaseRootViewController.swift */, 0ECB6B672CCFE663006A7C82 /* DateGroupedTableDataTests.swift */, + ED4294842CF1286E0077D7CB /* ShareManagerTests.swift */, ); path = ClientTests; sourceTree = ""; @@ -17046,6 +17052,7 @@ 8AA7347B28AEDB3100443D24 /* PocketViewModelTests.swift in Sources */, C2446B312A856D13000C527D /* MockLibraryCoordinatorDelegate.swift in Sources */, C869915728917809007ACC5C /* NetworkingMock.swift in Sources */, + ED575BD32D00F9D00056BCDA /* MockTemporaryDocument.swift in Sources */, 8A4190D22A6B0848001E8401 /* StatusBarOverlayTests.swift in Sources */, C8EDDBF029DD83FC003A4C07 /* RouteTests.swift in Sources */, 0A7693612C7DD19600103A6D /* CertificatesViewModelTests.swift in Sources */, @@ -17145,6 +17152,7 @@ 5A9A09D428B01D8700B6F51E /* MockTelemetryWrapper.swift in Sources */, C80C11F028B3C9150062922A /* MockUserDefaults.swift in Sources */, D8BA1790206D47830023AC00 /* DeferredTestUtils.swift in Sources */, + ED4294852CF1286E0077D7CB /* ShareManagerTests.swift in Sources */, 5AA75A652A46274A00533F8D /* MockThemeManager.swift in Sources */, 8A5038142A5DFCE000A1B02A /* MockBrowserProfile.swift in Sources */, C23889E52A50329200429673 /* MockParentCoordinator.swift in Sources */, diff --git a/firefox-ios/Client/Coordinators/Browser/BrowserCoordinator.swift b/firefox-ios/Client/Coordinators/Browser/BrowserCoordinator.swift index af2dd41bb168..c2373b6e0080 100644 --- a/firefox-ios/Client/Coordinators/Browser/BrowserCoordinator.swift +++ b/firefox-ios/Client/Coordinators/Browser/BrowserCoordinator.swift @@ -301,12 +301,8 @@ class BrowserCoordinator: BaseCoordinator, case let .searchURL(url, tabId): handle(searchURL: url, tabId: tabId) - case .sharesheet: - // FIXME: FXIOS-10669 Implement with real data, (shareType, shareMessage) -// let tempURL = URL(string: "https://www.google.ca") -// let tempTitle = "Test Title" -// showShareSheet(with: tempURL, title: tempTitle) - break + case let .sharesheet(shareType, shareMessage): + handleShareRoute(shareType: shareType, shareMessage: shareMessage) case let .glean(url): glean.handleDeeplinkUrl(url: url) @@ -393,6 +389,22 @@ class BrowserCoordinator: BaseCoordinator, browserViewController.presentSignInViewController(fxaParams) } + /// Starts the share sheet coordinator for the deep link `.sharesheet` route (share content via Nimbus Messaging). + /// - Parameters: + /// - shareType: The content to share. + /// - shareMessage: An optional textual message to accompany the shared content. May contain a Mail subject line. + private func handleShareRoute(shareType: ShareType, shareMessage: ShareMessage?) { + // FIXME: FXIOS-10829 Deep link shares should set a reasonable sourceView for iPad... or sourceView could be optional + startShareSheetCoordinator( + shareType: shareType, + shareMessage: shareMessage, + sourceView: browserViewController.addressToolbarContainer, + sourceRect: nil, + toastContainer: browserViewController.contentContainer, + popoverArrowDirection: .any + ) + } + private func canHandleSettings(with section: Route.SettingsSection) -> Bool { guard !childCoordinators.contains(where: { $0 is SettingsCoordinator }) else { return false // route is handled with existing child coordinator @@ -551,8 +563,24 @@ class BrowserCoordinator: BaseCoordinator, browserViewController.updateZoomPageBarVisibility(visible: true) } - func showShareSheet(with url: URL?) { - showShareSheet(with: url, title: nil) + /// Share the currently selected tab using the share sheet. + /// + /// Part of the MainMenuCoordinatorDelegate implementation, called from the New Menu > Tools > Share. + func showShareSheetForCurrentlySelectedTab() { + // We share the tab's displayURL to make sure we don't share reader mode localhost URLs + guard let selectedTab = tabManager.selectedTab, + let url = selectedTab.canonicalURL?.displayURL else { + return + } + + startShareSheetCoordinator( + shareType: .tab(url: url, tab: selectedTab), + shareMessage: nil, + sourceView: self.browserViewController.addressToolbarContainer, + sourceRect: nil, + toastContainer: self.browserViewController.contentContainer, + popoverArrowDirection: .any + ) } private func makeMenuNavViewController() -> DismissableNavigationViewController? { @@ -642,15 +670,17 @@ class BrowserCoordinator: BaseCoordinator, // MARK: - BrowserNavigationHandler - func showShareSheet(url: URL, - title: String?, - sourceView: UIView, - sourceRect: CGRect?, - toastContainer: UIView, - popoverArrowDirection: UIPopoverArrowDirection) { + func showShareSheet( + shareType: ShareType, + shareMessage: ShareMessage?, + sourceView: UIView, + sourceRect: CGRect?, + toastContainer: UIView, + popoverArrowDirection: UIPopoverArrowDirection + ) { startShareSheetCoordinator( - url: url, - title: title, + shareType: shareType, + shareMessage: shareMessage, sourceView: sourceView, sourceRect: sourceRect, toastContainer: toastContainer, @@ -741,35 +771,78 @@ class BrowserCoordinator: BaseCoordinator, return coordinator } + /// Starts the ShareSheetCoordinator, which initiates opening the iOS share sheet using an `UIActivityViewController`. + /// + /// For shared tabs where the user is currently on a non-HTML page (e.g. viewing a PDF), this method will initiate a + /// file download, and then share the file instead. This currently blocks without any UI indication, which is less + /// than ideal but has been the existing behaviour for a long time (task to address this: FXIOS-10823). + /// + /// - Parameters: + /// - shareType: The type of content to share. + /// - shareMessage: An optional accompanying message to share (with optional email subject line). + /// - sourceView: The view tapped to initiate share. iPad share sheet popovers will point to this element. + /// - sourceRect: The source rect for the view tapped to initiate share. iPad share sheet popovers will point to this + /// element. + /// - toastContainer: The container for displaying toast information. + /// - popoverArrowDirection: The arrow direction for iPad share sheet popovers. + /// + /// There are many ways to share many types of content from various areas of the app. Code paths that go through this + /// method include: + /// * Sharing content from a long press on Home screen tiles (e.g. long press Jump Back In context menu) + /// * From the old Menu > Share and the new Menu > Tools > Share + /// * From the new toolbar share button beside the address bar + /// * From long pressing a link in the WKWebView and sharing from the context menu (via ActionProviderBuilder > addShare) + /// * Via the sharesheet deeplink path in `RouteBuilder` (e.g. tapping home cards that initiate sharing content) + /// * Currently this is the only path that is using the ShareMessage param (Info Card Referral experiment FXE-1090) func startShareSheetCoordinator( - url: URL, - title: String?, + shareType: ShareType, + shareMessage: ShareMessage?, sourceView: UIView, sourceRect: CGRect?, toastContainer: UIView, popoverArrowDirection: UIPopoverArrowDirection ) { - guard childCoordinators.first(where: { $0 is ShareSheetCoordinator }) as? ShareSheetCoordinator == nil - else { - // If this case is hitted it means the share extension coordinator wasn't removed - // correctly in the previous session. - return + if let coordinator = childCoordinators.first(where: { $0 is ShareSheetCoordinator }) as? ShareSheetCoordinator { + // The share sheet extension coordinator wasn't correctly removed in the last share session. Attempt to recover. + logger.log( + "ShareSheetCoordinator already exists when it shouldn't. Removing and recreating it to access share sheet", + level: .info, + category: .shareSheet, + extra: ["existing ShareSheetCoordinator UUID": "\(coordinator.windowUUID)", + "BrowserCoordinator windowUUID": "\(windowUUID)"] + ) + + coordinator.dismiss() + } + + Task { + // FXIOS-10824 It's strange if the user has to wait a long time to download files that are literally already + // being shown in the webview. + var overrideShareType = shareType + if case ShareType.tab = shareType { + overrideShareType = await tryDownloadingTabFileToShare(shareType: shareType) + } + + await MainActor.run { [weak self, overrideShareType] in + guard let self else { return } + + let shareSheetCoordinator = ShareSheetCoordinator( + alertContainer: toastContainer, + router: router, + profile: profile, + parentCoordinator: self, + tabManager: tabManager + ) + add(child: shareSheetCoordinator) + shareSheetCoordinator.start( + shareType: overrideShareType, + shareMessage: shareMessage, + sourceView: sourceView, + sourceRect: sourceRect, + popoverArrowDirection: popoverArrowDirection + ) + } } - let shareSheetCoordinator = ShareSheetCoordinator( - alertContainer: toastContainer, - router: router, - profile: profile, - parentCoordinator: self, - tabManager: tabManager - ) - add(child: shareSheetCoordinator) - shareSheetCoordinator.start( - url: url, - title: title, - sourceView: sourceView, - sourceRect: sourceRect, - popoverArrowDirection: popoverArrowDirection - ) } func showCreditCardAutofill(creditCard: CreditCard?, @@ -1057,38 +1130,20 @@ class BrowserCoordinator: BaseCoordinator, // MARK: - Private helpers - private func showShareSheet(with url: URL?, title: String?) { - guard let url else { return } - - let showShareSheet = { url in - self.startShareSheetCoordinator( - url: url, - title: title, - sourceView: self.browserViewController.addressToolbarContainer, - sourceRect: nil, - toastContainer: self.browserViewController.contentContainer, - popoverArrowDirection: .any - ) + private func tryDownloadingTabFileToShare(shareType: ShareType) async -> ShareType { + // We can only try to download files for `.tab` type shares that have a TemporaryDocument + guard case let ShareType.tab(_, tab) = shareType, + let temporaryDocument = tab.temporaryDocument else { + return shareType } - // We only care about the temp document for .tab shares right? - // BUT!! A tab might be displaying a downloaded PDF! Then it's file:// url - guard let temporaryDocument = browserViewController.tabManager.selectedTab?.temporaryDocument else { - showShareSheet(url) - return + guard let fileURL = await temporaryDocument.getDownloadedURL() else { + // If no file was downloaded, simply share the tab as usual with a web URL + return shareType } - temporaryDocument.getURL { tempDocURL in - DispatchQueue.main.async { - // If we successfully got a temp file URL, share it like a downloaded file, - // otherwise present the ordinary share menu for the web URL. - if let tempDocURL = tempDocURL, tempDocURL.isFileURL { - showShareSheet(tempDocURL) - } else { - showShareSheet(url) - } - } - } + // If we successfully got a temp file URL, share it like a downloaded file + return .file(url: fileURL) } /// Utility. Performs the supplied action if a coordinator of the indicated type diff --git a/firefox-ios/Client/Coordinators/Browser/BrowserNavigationHandler.swift b/firefox-ios/Client/Coordinators/Browser/BrowserNavigationHandler.swift index a7a15cd5c4c8..da7749cc9058 100644 --- a/firefox-ios/Client/Coordinators/Browser/BrowserNavigationHandler.swift +++ b/firefox-ios/Client/Coordinators/Browser/BrowserNavigationHandler.swift @@ -27,13 +27,14 @@ protocol BrowserNavigationHandler: AnyObject, QRCodeNavigationHandler { /// Shows the share sheet. /// - /// - Parameter url: The url to be shared. + /// - Parameter shareType: The content to be shared. + /// - Parameter shareMessage: An optional plain text message to be shared. /// - Parameter sourceView: The reference view to show the popoverViewController. /// - Parameter sourceRect: An optional rect to use for ipad popover presentation. /// - Parameter toastContainer: The view in which is displayed the toast results from actions in the share extension. /// - Parameter popoverArrowDirection: The arrow direction for the view controller presented as popover. - func showShareSheet(url: URL, - title: String?, + func showShareSheet(shareType: ShareType, + shareMessage: ShareMessage?, sourceView: UIView, sourceRect: CGRect?, toastContainer: UIView, diff --git a/firefox-ios/Client/Coordinators/Library/DownloadsCoordinator.swift b/firefox-ios/Client/Coordinators/Library/DownloadsCoordinator.swift index d3267004fcdd..64f7419e028a 100644 --- a/firefox-ios/Client/Coordinators/Library/DownloadsCoordinator.swift +++ b/firefox-ios/Client/Coordinators/Library/DownloadsCoordinator.swift @@ -49,12 +49,19 @@ class DownloadsCoordinator: BaseCoordinator, } private func startShare(file: DownloadedFile, sourceView: UIView) { - guard !childCoordinators.contains(where: { $0 is ShareSheetCoordinator }) else { return } - let coordinator = makeShareSheetCoordinator() - coordinator.start(url: file.path, sourceView: sourceView) - } + if let coordinator = childCoordinators.first(where: { $0 is ShareSheetCoordinator }) as? ShareSheetCoordinator { + // The share sheet extension coordinator wasn't correctly removed in the last share session. Attempt to recover. + logger.log( + "ShareSheetCoordinator already exists when it shouldn't. Removing and recreating it to access share sheet", + level: .info, + category: .shareSheet, + extra: ["existing ShareSheetCoordinator UUID": "\(coordinator.windowUUID)", + "DownloadsCoordinator windowUUID": "N/A"] + ) + + coordinator.dismiss() + } - private func makeShareSheetCoordinator() -> ShareSheetCoordinator { let coordinator = ShareSheetCoordinator( alertContainer: UIView(), router: router, @@ -63,7 +70,13 @@ class DownloadsCoordinator: BaseCoordinator, tabManager: tabManager ) add(child: coordinator) - return coordinator + coordinator.start( + shareType: .file(url: file.path), + shareMessage: nil, + sourceView: sourceView, + sourceRect: nil, + popoverArrowDirection: .any + ) } func showDocument(file: DownloadedFile) { diff --git a/firefox-ios/Client/Coordinators/Library/LibraryCoordinator.swift b/firefox-ios/Client/Coordinators/Library/LibraryCoordinator.swift index ebbdf1a66692..0c94c37c4f4b 100644 --- a/firefox-ios/Client/Coordinators/Library/LibraryCoordinator.swift +++ b/firefox-ios/Client/Coordinators/Library/LibraryCoordinator.swift @@ -92,9 +92,30 @@ class LibraryCoordinator: BaseCoordinator, } func shareLibraryItem(url: URL, sourceView: UIView) { - guard !childCoordinators.contains(where: { $0 is ShareSheetCoordinator }) else { return } - let coordinator = makeShareSheetCoordinator() - coordinator.start(url: url, sourceView: sourceView) + if let coordinator = childCoordinators.first(where: { $0 is ShareSheetCoordinator }) as? ShareSheetCoordinator { + // The share sheet extension coordinator wasn't correctly removed in the last share session. Attempt to recover. + logger.log( + "ShareSheetCoordinator already exists when it shouldn't. Removing and recreating it to access share sheet", + level: .info, + category: .shareSheet, + extra: ["existing ShareSheetCoordinator UUID": "\(coordinator.windowUUID)", + "LibraryCoordinator windowUUID": "\(windowUUID)"] + ) + + coordinator.dismiss() + } + + let coordinator = ShareSheetCoordinator( + alertContainer: UIView(), + router: router, + profile: profile, + parentCoordinator: self, + tabManager: tabManager + ) + add(child: coordinator) + + // Note: Called from History, Bookmarks, and Reading List long presses > Share from the context menu + coordinator.start(shareType: .site(url: url), shareMessage: nil, sourceView: sourceView) } private func makeBookmarksCoordinator(navigationController: UINavigationController) { @@ -166,18 +187,6 @@ class LibraryCoordinator: BaseCoordinator, remove(child: childCoordinator) } - private func makeShareSheetCoordinator() -> ShareSheetCoordinator { - let coordinator = ShareSheetCoordinator( - alertContainer: UIView(), - router: router, - profile: profile, - parentCoordinator: self, - tabManager: tabManager - ) - add(child: coordinator) - return coordinator - } - // MARK: - LibraryPanelDelegate func libraryPanelDidRequestToOpenInNewTab(_ url: URL, isPrivate: Bool) { diff --git a/firefox-ios/Client/Coordinators/Router/RouteBuilder.swift b/firefox-ios/Client/Coordinators/Router/RouteBuilder.swift index da579d1b0506..aa1f56c30022 100644 --- a/firefox-ios/Client/Coordinators/Router/RouteBuilder.swift +++ b/firefox-ios/Client/Coordinators/Router/RouteBuilder.swift @@ -132,6 +132,7 @@ final class RouteBuilder { // Pass optional share message and subtitle here var shareMessage: ShareMessage? if let titleText = urlScanner.value(query: "title") { + // TODO: FXIOS-10629 Get the optional subtitle parameter shareMessage = ShareMessage(message: titleText, subtitle: nil) } diff --git a/firefox-ios/Client/Coordinators/ShareSheetCoordinator.swift b/firefox-ios/Client/Coordinators/ShareSheetCoordinator.swift index 81de1991ae69..b484002be1df 100644 --- a/firefox-ios/Client/Coordinators/ShareSheetCoordinator.swift +++ b/firefox-ios/Client/Coordinators/ShareSheetCoordinator.swift @@ -18,7 +18,7 @@ class ShareSheetCoordinator: BaseCoordinator, private let profile: Profile private let alertContainer: UIView private weak var parentCoordinator: ParentCoordinatorDelegate? - private var windowUUID: WindowUUID { return tabManager.windowUUID } + var windowUUID: WindowUUID { return tabManager.windowUUID } // MARK: - Initializers @@ -42,30 +42,34 @@ class ShareSheetCoordinator: BaseCoordinator, /// Presents the share sheet from the source view func start( - url: URL, - title: String? = nil, + shareType: ShareType, + shareMessage: ShareMessage?, sourceView: UIView, sourceRect: CGRect? = nil, popoverArrowDirection: UIPopoverArrowDirection = .up ) { - let shareManager = ShareManager( - url: url, - // FXIOS-10669: We only want to pass a non-nil title here for the Info Card Referral experiment. Refactoring is - // needed in the ShareManager to make it properly extensible to multiple share use cases like this. - title: title, - tab: tabManager.selectedTab) - let controller = shareManager.createActivityViewController( - tabManager.selectedTab?.webView - ) { [weak self] completed, activityType in - guard let self = self else { return } - self.handleShareSheetCompletion(activityType: activityType, url: url) - } + let controller = ShareManager.createActivityViewController( + shareType: shareType, + shareMessage: shareMessage, + completionHandler: { [weak self] completed, activityType in + guard let self else { return } + + self.handleShareSheetCompletion( + activityType: activityType, + shareType: shareType, + relatedTab: self.tabManager.selectedTab + ) + } + ) + if let popoverPresentationController = controller.popoverPresentationController { popoverPresentationController.sourceView = sourceView popoverPresentationController.sourceRect = sourceRect ?? sourceView.bounds popoverPresentationController.permittedArrowDirections = popoverArrowDirection } + TelemetryWrapper.recordEvent(category: .action, method: .tap, object: .sharePageWith) + if let presentedViewController = router.navigationController.presentedViewController { presentedViewController.dismiss(animated: true) { [weak self] in self?.router.present(controller) @@ -77,20 +81,46 @@ class ShareSheetCoordinator: BaseCoordinator, private func handleShareSheetCompletion( activityType: UIActivity.ActivityType?, - url: URL + shareType: ShareType, + relatedTab: Tab? ) { + // NOTE: didFinish will be called on each of these code paths to properly dismiss the share sheet. + // FIXME: FXIOS-10334 It's mysterious that we are calling dequeueNotShownJSAlert in the share coordinator. It may not + // be necessary, but this JS alert code is fragile right now so let's not touch it until FXIOS-10334 is underway. switch activityType { case CustomActivityAction.sendToDevice.actionType: - showSendToDevice(url: url) + // Cannot send file:// URLs to another synced device + guard !shareType.wrappedURL.isFileURL else { + dequeueNotShownJSAlert() + return + } + + switch shareType { + case let .tab(_, tab): + showSendToDevice(url: shareType.wrappedURL, relatedTab: tab) + default: + showSendToDevice(url: shareType.wrappedURL, relatedTab: nil) + } + default: dequeueNotShownJSAlert() } } - private func showSendToDevice(url: URL) { - var shareItem: ShareItem! - if let selectedTab = tabManager.selectedTab, let url = selectedTab.canonicalURL?.displayURL { - shareItem = ShareItem(url: url.absoluteString, title: selectedTab.title) + func dismiss() { + parentCoordinator?.didFinish(from: self) + } + + /// Shows the view controller that allows users to send this URL to one of their synced devices running Firefox. + /// - Parameters: + /// - url: The URL to send. + /// - relatedTab: The tab associated with this URL. Will be used to provide the displayURL and displayTitle if set. + /// Should be nil if the url to be sent is NOT related to a specific tab (e.g. a bookmark URL). + private func showSendToDevice(url: URL, relatedTab: (any ShareTab)?) { + // TODO: FXIOS-10831 harden this code path to ensure we don't ever share a reader mode localhost URL + let shareItem: ShareItem + if let relatedTab, let url = relatedTab.canonicalURL?.displayURL { + shareItem = ShareItem(url: url.absoluteString, title: relatedTab.displayTitle) } else { shareItem = ShareItem(url: url.absoluteString, title: nil) } @@ -100,23 +130,25 @@ class ShareSheetCoordinator: BaseCoordinator, textColor: themeColors.textPrimary, iconColor: themeColors.iconDisabled) - let helper = SendToDeviceHelper( + let sendToDeviceHelper = SendToDeviceHelper( shareItem: shareItem, profile: profile, colors: colors, - delegate: self) - let viewController = helper.initialViewController() + delegate: self + ) + let viewController = sendToDeviceHelper.initialViewController() TelemetryWrapper.recordEvent(category: .action, method: .tap, object: .sendToDevice) router.present(viewController) } private func dequeueNotShownJSAlert() { - guard let alertInfo = tabManager.selectedTab?.dequeueJavascriptAlertPrompt() - else { + guard let alertInfo = tabManager.selectedTab?.dequeueJavascriptAlertPrompt() else { parentCoordinator?.didFinish(from: self) return } + + // Will call `dequeueNotShownJSAlert` again when finished via JSPromptAlertControllerDelegate delegate method let alertController = alertInfo.alertController() alertController.delegate = self router.present(alertController) @@ -133,8 +165,7 @@ class ShareSheetCoordinator: BaseCoordinator, _ devicePickerViewController: DevicePickerViewController, didPickDevices devices: [RemoteDevice] ) { - guard let shareItem = devicePickerViewController.shareItem - else { + guard let shareItem = devicePickerViewController.shareItem else { router.dismiss() parentCoordinator?.didFinish(from: self) return @@ -158,6 +189,7 @@ class ShareSheetCoordinator: BaseCoordinator, } return } + profile.sendItem(shareItem, toDevices: devices).uponQueue(.main) { [weak self] _ in guard let self = self else { return } self.router.dismiss() diff --git a/firefox-ios/Client/Frontend/Browser/BrowserViewController/Actions/ActionProviderBuilder.swift b/firefox-ios/Client/Frontend/Browser/BrowserViewController/Actions/ActionProviderBuilder.swift index 71531a4c753c..4f12c97ecf43 100644 --- a/firefox-ios/Client/Frontend/Browser/BrowserViewController/Actions/ActionProviderBuilder.swift +++ b/firefox-ios/Client/Frontend/Browser/BrowserViewController/Actions/ActionProviderBuilder.swift @@ -114,11 +114,13 @@ class ActionProviderBuilder { let helper = tab.getContentScript(name: ContextMenuHelper.name()) as? ContextMenuHelper else { return } - // This is only used on ipad for positioning the popover. On iPhone it is an action sheet. + // The `point` is only used on ipad for positioning the popover. On iPhone it is an bottom sheet. let point = webView.convert(helper.touchPoint, to: view) + + // Shares from long-pressing a link in the webview and tapping Share in the context menu navigationHandler?.showShareSheet( - url: url, - title: nil, + shareType: .site(url: url), // NOT `.tab` share; the link might be to a different domain from the current tab + shareMessage: nil, sourceView: view, sourceRect: CGRect(origin: point, size: CGSize(width: 10.0, height: 10.0)), toastContainer: contentContainer, diff --git a/firefox-ios/Client/Frontend/Browser/BrowserViewController/Extensions/BrowserViewController+WebViewDelegates.swift b/firefox-ios/Client/Frontend/Browser/BrowserViewController/Extensions/BrowserViewController+WebViewDelegates.swift index bd34d03475f1..dba9347acbfd 100644 --- a/firefox-ios/Client/Frontend/Browser/BrowserViewController/Extensions/BrowserViewController+WebViewDelegates.swift +++ b/firefox-ios/Client/Frontend/Browser/BrowserViewController/Extensions/BrowserViewController+WebViewDelegates.swift @@ -636,7 +636,7 @@ extension BrowserViewController: WKNavigationDelegate { let group = DispatchGroup() var url: URL? group.enter() - let temporaryDocument = TemporaryDocument(preflightResponse: response, request: request) + let temporaryDocument = DefaultTemporaryDocument(preflightResponse: response, request: request) temporaryDocument.getURL(completionHandler: { docURL in url = docURL group.leave() @@ -711,7 +711,7 @@ extension BrowserViewController: WKNavigationDelegate { // representative of the contents of the web view. if navigationResponse.isForMainFrame, let tab = tabManager[webView] { if response.mimeType != MIMEType.HTML, let request = request { - tab.temporaryDocument = TemporaryDocument(preflightResponse: response, request: request) + tab.temporaryDocument = DefaultTemporaryDocument(preflightResponse: response, request: request) } else { tab.temporaryDocument = nil } diff --git a/firefox-ios/Client/Frontend/Browser/BrowserViewController/Views/BrowserViewController.swift b/firefox-ios/Client/Frontend/Browser/BrowserViewController/Views/BrowserViewController.swift index 2885604c9a1e..b6f8ad84b9e3 100644 --- a/firefox-ios/Client/Frontend/Browser/BrowserViewController/Views/BrowserViewController.swift +++ b/firefox-ios/Client/Frontend/Browser/BrowserViewController/Views/BrowserViewController.swift @@ -2346,19 +2346,22 @@ class BrowserViewController: UIViewController, extras: nil) } - guard let selectedTab = tabManager.selectedTab, let tabUrl = selectedTab.canonicalURL?.displayURL else { + // We share the tab's displayURL to make sure we don't share reader mode localhost URLs + guard let selectedTab = tabManager.selectedTab, + let tabUrl = selectedTab.canonicalURL?.displayURL else { assertionFailure("Tried to share with no selected tab or URL") return } + /// Note: If the user is viewing a _downloaded_ (not online) PDF in the browser, then the current tab's URL will + /// have a `file://` scheme. navigationHandler?.showShareSheet( - url: tabUrl, - title: nil, + shareType: .tab(url: tabUrl, tab: selectedTab), + shareMessage: nil, sourceView: sourceView, sourceRect: nil, toastContainer: contentContainer, - popoverArrowDirection: isBottomSearchBar ? .down : .up - ) + popoverArrowDirection: isBottomSearchBar ? .down : .up) } func presentTabsLongPressAction(from view: UIView) { diff --git a/firefox-ios/Client/Frontend/Browser/MainMenu/MainMenuCoordinator.swift b/firefox-ios/Client/Frontend/Browser/MainMenu/MainMenuCoordinator.swift index f37f76a56352..44aba5ddb351 100644 --- a/firefox-ios/Client/Frontend/Browser/MainMenu/MainMenuCoordinator.swift +++ b/firefox-ios/Client/Frontend/Browser/MainMenu/MainMenuCoordinator.swift @@ -16,7 +16,9 @@ protocol MainMenuCoordinatorDelegate: AnyObject { func showFindInPage() func showSignInView(fxaParameters: FxASignInViewParameters?) func updateZoomPageBarVisibility() - func showShareSheet(with url: URL?) + + /// Open the share sheet to share the currently selected `Tab`. + func showShareSheetForCurrentlySelectedTab() } class MainMenuCoordinator: BaseCoordinator, FeatureFlaggable { @@ -131,7 +133,7 @@ class MainMenuCoordinator: BaseCoordinator, FeatureFlaggable { ) self.navigationHandler?.showSignInView(fxaParameters: fxaParameters) case .shareSheet: - self.navigationHandler?.showShareSheet(with: destination.url) + self.navigationHandler?.showShareSheetForCurrentlySelectedTab() case .zoom: self.navigationHandler?.updateZoomPageBarVisibility() } diff --git a/firefox-ios/Client/Frontend/Browser/MainMenuActionHelper.swift b/firefox-ios/Client/Frontend/Browser/MainMenuActionHelper.swift index 877a7d4ed0a0..46465ea93f50 100644 --- a/firefox-ios/Client/Frontend/Browser/MainMenuActionHelper.swift +++ b/firefox-ios/Client/Frontend/Browser/MainMenuActionHelper.swift @@ -588,39 +588,20 @@ class MainMenuActionHelper: PhotonActionSheetProtocol, private func getShareAction() -> PhotonRowActions { return SingleActionViewModel(title: .LegacyAppMenu.Share, iconString: StandardImageIdentifiers.Large.share) { _ in - guard let tab = self.selectedTab, let url = tab.canonicalURL?.displayURL else { return } + // We share the tab's displayURL to make sure we don't share reader mode localhost URLs + guard let tab = self.selectedTab, + let url = tab.canonicalURL?.displayURL + else { return } TelemetryWrapper.recordEvent(category: .action, method: .tap, object: .sharePageWith) - - guard let temporaryDocument = tab.temporaryDocument else { - self.navigationHandler?.showShareSheet( - url: url, - title: nil, - sourceView: self.buttonView, - sourceRect: nil, - toastContainer: self.toastContainer, - popoverArrowDirection: .any) - return - } - - temporaryDocument.getURL { tempDocURL in - DispatchQueue.main.async { - // If we successfully got a temp file URL, share it like a downloaded file, - // otherwise present the ordinary share menu for the web URL. - if let tempDocURL = tempDocURL, - tempDocURL.isFileURL { - self.share(fileURL: tempDocURL, buttonView: self.buttonView) - } else { - self.navigationHandler?.showShareSheet( - url: url, - title: nil, - sourceView: self.buttonView, - sourceRect: nil, - toastContainer: self.toastContainer, - popoverArrowDirection: .any) - } - } - } + self.navigationHandler?.showShareSheet( + shareType: .tab(url: url, tab: tab), + shareMessage: nil, + sourceView: self.buttonView, + sourceRect: nil, + toastContainer: self.toastContainer, + popoverArrowDirection: .any + ) }.items } @@ -639,11 +620,12 @@ class MainMenuActionHelper: PhotonActionSheetProtocol, /// Share the URL of a downloaded file (e.g. either a user-downloaded file being viewed in the webView, or a link with a /// non-HTML MIME type currently opened in the webView, such as a PDF). + /// NOTE: Called from getShareFileAction (files in the browser) and getShareAction (websites) private func share(fileURL: URL, buttonView: UIView) { TelemetryWrapper.recordEvent(category: .action, method: .tap, object: .sharePageWith) navigationHandler?.showShareSheet( - url: fileURL, - title: nil, + shareType: .file(url: fileURL), + shareMessage: nil, sourceView: buttonView, sourceRect: nil, toastContainer: toastContainer, diff --git a/firefox-ios/Client/Frontend/Browser/TemporaryDocument.swift b/firefox-ios/Client/Frontend/Browser/TemporaryDocument.swift index 92d9b3c6d7bb..d355f2993e7b 100644 --- a/firefox-ios/Client/Frontend/Browser/TemporaryDocument.swift +++ b/firefox-ios/Client/Frontend/Browser/TemporaryDocument.swift @@ -7,7 +7,12 @@ import Shared private let temporaryDocumentOperationQueue = OperationQueue() -class TemporaryDocument: NSObject { +protocol TemporaryDocument { + func getURL(completionHandler: @escaping ((URL?) -> Void)) + func getDownloadedURL() async -> URL? +} + +class DefaultTemporaryDocument: NSObject, TemporaryDocument { fileprivate let request: URLRequest fileprivate let filename: String @@ -36,6 +41,41 @@ class TemporaryDocument: NSObject { } } + /// Uses modern concurrency to download the file and save to temporary storage. + /// FXIOS-10830 - for iOS 18 we need to avoid calling the old URLSession APIs with continuations. + /// - Returns: The URL of the file within the temporary directory. Returns nil if the file could not be downloaded. + func getDownloadedURL() async -> URL? { + if let url = localFileURL { + return url + } + + do { + let (downloadURL, _) = try await URLSession.shared.download(for: request) + + let tempDirectory = URL(fileURLWithPath: NSTemporaryDirectory()).appendingPathComponent("TempDocs") + let tempFileURL = tempDirectory.appendingPathComponent(filename) + + // Rename and move the downloaded file into the TempDocs directory + try? FileManager.default.createDirectory( + at: tempDirectory, + withIntermediateDirectories: true, + attributes: nil + ) + try? FileManager.default.removeItem(at: tempFileURL) + try FileManager.default.moveItem(at: downloadURL, to: tempFileURL) + + localFileURL = tempFileURL + return tempFileURL + } catch { + return nil + } + } + + /// Downloads this file to temporary storage. + /// + /// CAUTION: Do not call this method from within a continuation. In iOS18, if we call this inside a continuation, is it + /// possible we may have a crash similar to the one fixed in https://github.com/mozilla-mobile/firefox-ios/pull/23596 + /// (related ticket is FXIOS-10811, which tracked an incident elsewhere in the app). func getURL(completionHandler: @escaping ((URL?) -> Void)) { if let url = localFileURL { completionHandler(url) diff --git a/firefox-ios/Client/Frontend/Home/HomepageContextMenuHelper.swift b/firefox-ios/Client/Frontend/Home/HomepageContextMenuHelper.swift index 6d82a65a08e5..3ffde3395987 100644 --- a/firefox-ios/Client/Frontend/Home/HomepageContextMenuHelper.swift +++ b/firefox-ios/Client/Frontend/Home/HomepageContextMenuHelper.swift @@ -249,8 +249,8 @@ class HomepageContextMenuHelper: HomepageContextMenuProtocol { guard let url = URL(string: site.url, invalidCharacters: false) else { return } self.browserNavigationHandler?.showShareSheet( - url: url, - title: nil, + shareType: .site(url: url), + shareMessage: nil, sourceView: sourceView ?? UIView(), sourceRect: nil, toastContainer: self.toastContainer, diff --git a/firefox-ios/Client/Frontend/Library/Downloads/DownloadsPanel.swift b/firefox-ios/Client/Frontend/Library/Downloads/DownloadsPanel.swift index 5e883c319ee2..cc0cb144cc09 100644 --- a/firefox-ios/Client/Frontend/Library/Downloads/DownloadsPanel.swift +++ b/firefox-ios/Client/Frontend/Library/Downloads/DownloadsPanel.swift @@ -156,10 +156,13 @@ class DownloadsPanel: UIViewController, } private func shareDownloadedFile(_ downloadedFile: DownloadedFile, indexPath: IndexPath) { - let helper = ShareManager(url: downloadedFile.path, tab: nil) - let controller = helper.createActivityViewController { _, _ in } + let shareActivityViewController = ShareManager.createActivityViewController( + shareType: .file(url: downloadedFile.path), + shareMessage: nil, + completionHandler: { _, _ in } + ) - if let popoverPresentationController = controller.popoverPresentationController { + if let popoverPresentationController = shareActivityViewController.popoverPresentationController { guard let tableViewCell = tableView.cellForRow(at: indexPath) else { return } popoverPresentationController.sourceView = tableViewCell @@ -167,7 +170,7 @@ class DownloadsPanel: UIViewController, popoverPresentationController.permittedArrowDirections = .any } - present(controller, animated: true, completion: nil) + present(shareActivityViewController, animated: true, completion: nil) } private func iconForFileExtension(_ fileExtension: String) -> UIImage? { diff --git a/firefox-ios/Client/Frontend/Share/ShareManager.swift b/firefox-ios/Client/Frontend/Share/ShareManager.swift index 58756b76fda1..b1c8cb7ed68d 100644 --- a/firefox-ios/Client/Frontend/Share/ShareManager.swift +++ b/firefox-ios/Client/Frontend/Share/ShareManager.swift @@ -9,39 +9,28 @@ import WebKit import UniformTypeIdentifiers class ShareManager: NSObject, FeatureFlaggable { - private weak var selectedTab: Tab? - - private let url: URL - private let title: String? - private var onePasswordExtensionItem: NSExtensionItem! - private let browserFillIdentifier = "org.appextension.fill-browser-action" - private let pocketIconExtension = "com.ideashower.ReadItLaterPro.AddToPocketExtension" - private let pocketActionExtension = "com.ideashower.ReadItLaterPro.Action-Extension" - - private var excludingActivities: [UIActivity.ActivityType] { - return [UIActivity.ActivityType.addToReadingList] + private struct ActivityIdentifiers { + static let pocketIconExtension = "com.ideashower.ReadItLaterPro.AddToPocketExtension" + static let pocketActionExtension = "com.ideashower.ReadItLaterPro.Action-Extension" + static let whatsApp = "net.whatsapp.WhatsApp.ShareExtension" } - // Can be a file:// or http(s):// url - init(url: URL, title: String? = nil, tab: Tab?) { - self.url = url - self.title = title - self.selectedTab = tab - } + // Black list for activities to which we don't want to share + private static let excludingActivities: [UIActivity.ActivityType] = [ + UIActivity.ActivityType.addToReadingList + ] - func createActivityViewController( - _ webView: WKWebView? = nil, + static func createActivityViewController( + shareType: ShareType, + shareMessage: ShareMessage?, completionHandler: @escaping ( _ completed: Bool, _ activityType: UIActivity.ActivityType? ) -> Void ) -> UIActivityViewController { - var activityItems = getActivityItems(url: url) - // Note: webview is required for adding websites to the iOS home screen - if #available(iOS 16.4, *), let webView = webView { - activityItems.append(webView) - } - let appActivities = getApplicationActivities() + let activityItems = getActivityItems(forShareType: shareType, withExplicitShareMessage: shareMessage) + let appActivities = getApplicationActivities(forShareType: shareType) + let activityViewController = UIActivityViewController( activityItems: activityItems, applicationActivities: appActivities @@ -55,14 +44,14 @@ class ShareManager: NSObject, FeatureFlaggable { return } - // Add telemetry for Pocket activityType - if activityType?.rawValue == self.pocketIconExtension { + // Add telemetry for Pocket extension activityTypes + if activityType?.rawValue == ActivityIdentifiers.pocketIconExtension { TelemetryWrapper.recordEvent(category: .action, method: .tap, object: .shareSheet, value: .sharePocketIcon, extras: nil) - } else if activityType?.rawValue == self.pocketActionExtension { + } else if activityType?.rawValue == ActivityIdentifiers.pocketActionExtension { TelemetryWrapper.recordEvent(category: .action, method: .tap, object: .shareSheet, @@ -76,104 +65,70 @@ class ShareManager: NSObject, FeatureFlaggable { return activityViewController } - /// Get the data to be shared if the URL is a file we will share just the url if not we prepare - /// UIPrintInfo to get the option to print the page and tab URL and title - /// - Parameter url: url from the selected tab - /// - Returns: An array of elements to be shared - private func getActivityItems(url: URL) -> [Any] { - // If url is file return only url to be shared - guard !url.isFileURL else { return [url] } + static func getActivityItems( + forShareType shareType: ShareType, + withExplicitShareMessage explicitShareMessage: ShareMessage? + ) -> [Any] { + var activityItems: [Any] = [] - var activityItems = [Any]() + switch shareType { + case .file(let fileURL): + activityItems.append(URLActivityItemProvider(url: fileURL)) - // Add the title (if it exists) - if let title = self.title { - activityItems.append(title) - } + if let explicitShareMessage { + activityItems.append(TitleSubtitleActivityItemProvider(shareMessage: explicitShareMessage)) + } + + case .site(let siteURL): + activityItems.append(URLActivityItemProvider(url: siteURL)) - let printInfo = UIPrintInfo(dictionary: nil) - printInfo.jobName = (url.absoluteString as NSString).lastPathComponent - printInfo.outputType = .general - activityItems.append(printInfo) - - // when tab is not loaded (webView != nil) don't show print activity - if let tab = selectedTab, tab.webView != nil { - activityItems.append( - TabPrintPageRenderer( - tabDisplayTitle: tab.displayTitle, - tabURL: tab.url, - webView: tab.webView + // For websites shared from a place without a webview (e.g. bookmarks), we don't actually have webview to offer + // any advanced information (like title, printing, sent to the iOS home screen, etc.) + if let explicitShareMessage { + activityItems.append(TitleSubtitleActivityItemProvider(shareMessage: explicitShareMessage)) + } + + case .tab(let siteURL, let tab): + // For websites, we also want to offer a few additional activity items besides the URL, like printing the + // webpage or adding a website to the iOS home screen + activityItems.append(URLActivityItemProvider(url: siteURL)) + + // Only show the print activity if the tab's webview is loaded + if tab.webView != nil { + activityItems.append( + TabPrintPageRenderer( + tabDisplayTitle: tab.displayTitle, + tabURL: tab.url, + webView: tab.webView + ) ) - ) - } + } - if let title = selectedTab?.title { - activityItems.append(TitleActivityItemProvider(title: title)) + // Add the webview for an option to add a website to the iOS home screen + if #available(iOS 16.4, *), let webView = tab.webView { + // NOTE: You will not see "Add to Home Screen" option on debug builds. Possibly this is because of how the + // com.apple.developer.web-browser entitlement is applied... + activityItems.append(webView) + } + + if let explicitShareMessage { + activityItems.append(TitleSubtitleActivityItemProvider(shareMessage: explicitShareMessage)) + } else { + // For feature parity with Safari, we use this provider to decide to which apps we should (or should not) + // share a display title and/or subject line + activityItems.append(TitleActivityItemProvider(title: tab.displayTitle)) + } } - activityItems.append(self) return activityItems } - private func getApplicationActivities() -> [UIActivity] { + private static func getApplicationActivities(forShareType shareType: ShareType) -> [UIActivity] { var appActivities = [UIActivity]() - let sendToDeviceActivity = SendToDeviceActivity(activityType: .sendToDevice, url: url) - appActivities.append(sendToDeviceActivity) + // Only acts on non-file URLs to send links to synced devices. Will ignore file URLs it can't handle. + appActivities.append(SendToDeviceActivity(activityType: .sendToDevice, url: shareType.wrappedURL)) return appActivities } } - -extension ShareManager: UIActivityItemSource { - func activityViewControllerPlaceholderItem(_ activityViewController: UIActivityViewController) -> Any { - return url - } - - func activityViewController( - _ activityViewController: UIActivityViewController, - itemForActivityType activityType: UIActivity.ActivityType? - ) -> Any? { - if isPasswordManager(activityType: activityType) { - return onePasswordExtensionItem - } else if isOpenByCopy(activityType: activityType) { - return url - } - - // Return the URL for the selected tab. If we are in reader view then decode - // it so that we copy the original and not the internal localhost one. - return url.isReaderModeURL ? url.decodeReaderModeURL : url - } - - func activityViewController( - _ activityViewController: UIActivityViewController, - dataTypeIdentifierForActivityType activityType: UIActivity.ActivityType? - ) -> String { - if isPasswordManager(activityType: activityType) { - return browserFillIdentifier - } else if isOpenByCopy(activityType: activityType) { - return url.isFileURL ? UTType.fileURL.identifier : UTType.url.identifier - } - - return activityType == nil ? browserFillIdentifier : UTType.url.identifier - } - - private func isPasswordManager(activityType: UIActivity.ActivityType?) -> Bool { - guard let activityType = activityType?.rawValue else { return false } - // A 'password' substring covers the most cases, such as pwsafe and 1Password. - // com.agilebits.onepassword-ios.extension - // com.app77.ios.pwsafe2.find-login-action-password-actionExtension - // If your extension's bundle identifier does not contain "password", simply submit a pull request - // by adding your bundle identifier. - return (activityType.contains("password")) - || (activityType == "com.lastpass.ilastpass.LastPassExt") - || (activityType == "in.sinew.Walletx.WalletxExt") - || (activityType == "com.8bit.bitwarden.find-login-action-extension") - || (activityType == "me.mssun.passforios.find-login-action-extension") - } - - private func isOpenByCopy(activityType: UIActivity.ActivityType?) -> Bool { - guard let activityType = activityType?.rawValue else { return false } - return activityType.lowercased().contains("remoteopeninapplication-bycopy") - } -} diff --git a/firefox-ios/Client/Frontend/Share/ShareMessage.swift b/firefox-ios/Client/Frontend/Share/ShareMessage.swift index cc6b3b4f79f4..8d9a3bdc0a81 100644 --- a/firefox-ios/Client/Frontend/Share/ShareMessage.swift +++ b/firefox-ios/Client/Frontend/Share/ShareMessage.swift @@ -5,7 +5,7 @@ import Foundation /// Additional text to share alongside a `ShareType`. Some apps, like Mail, support an additional subject line (called a -/// subtitle). +/// `subtitle` with respect to `UIActivityItemProvider`s). struct ShareMessage: Equatable { let message: String let subtitle: String? diff --git a/firefox-ios/Client/Frontend/Share/ShareTab.swift b/firefox-ios/Client/Frontend/Share/ShareTab.swift index 1001cb4488bc..8f112f7cf2bb 100644 --- a/firefox-ios/Client/Frontend/Share/ShareTab.swift +++ b/firefox-ios/Client/Frontend/Share/ShareTab.swift @@ -7,5 +7,9 @@ import Foundation protocol ShareTab: Equatable { var displayTitle: String { get } var url: URL? { get } + var canonicalURL: URL? { get } var webView: TabWebView? { get } + + // Tabs displaying content other than HTML mime type can optionally be downloaded and treated as files when shared + var temporaryDocument: TemporaryDocument? { get } } diff --git a/firefox-ios/Client/Frontend/Share/ShareType.swift b/firefox-ios/Client/Frontend/Share/ShareType.swift index b54e570a1eb8..ea3a92705d25 100644 --- a/firefox-ios/Client/Frontend/Share/ShareType.swift +++ b/firefox-ios/Client/Frontend/Share/ShareType.swift @@ -5,11 +5,14 @@ import Foundation /// Preconfigured sharing schemes which the share manager knows how to handle. -/// file: Include a file URL (file://). Best used for sharing downloaded files. -/// site: Include a website URL (http(s)://). Best used for sharing library/bookmarks, etc. without an active tab. +/// file: Include a file URL (`file://`). Best used for sharing downloaded files. +/// site: Include a website URL (`http(s)://`). Best used for sharing library/bookmarks, etc. without an active tab. /// Shares configured using .site will not append a title to Messages but will have a subtitle in Mail. -/// tab: Include a website URL and a tab to share. If sharing a tab with an active webView, then additional sharing -/// options will be configured, including printing the page and adding the webpage to your iOS home screen. +/// tab: Include a URL and a tab to share. If sharing a tab with an active webView, then additional sharing +/// options can be configured, including printing the page and adding the webpage to your iOS home screen. +/// __SPECIAL NOTE__: If you download a PDF, you can view that in a tab. In that case, the URL may have a `file://` +/// scheme instead of `http(s)://`, so certain options, like Send to Device / Add to Home Screen, may not be +/// available. enum ShareType: Equatable { case file(url: URL) case site(url: URL) @@ -27,6 +30,17 @@ enum ShareType: Equatable { return false } } + + var wrappedURL: URL { + switch self { + case let .file(url): + return url + case let .site(url): + return url + case let .tab(url, _): + return url + } + } } extension ShareTab { diff --git a/firefox-ios/Client/Frontend/Share/URLActivityItemProvider.swift b/firefox-ios/Client/Frontend/Share/URLActivityItemProvider.swift index 176914761c4d..acdb682d85fb 100644 --- a/firefox-ios/Client/Frontend/Share/URLActivityItemProvider.swift +++ b/firefox-ios/Client/Frontend/Share/URLActivityItemProvider.swift @@ -14,9 +14,13 @@ class URLActivityItemProvider: UIActivityItemProvider, @unchecked Sendable { ] init(url: URL) { - self.url = url + // If the user is sharing a reader mode URL, we must decode it so we don't share internal localhost URLs + let parsedURL = url.isReaderModeURL + ? url.decodeReaderModeURL ?? url + : url - super.init(placeholderItem: url) + self.url = parsedURL + super.init(placeholderItem: parsedURL) } override var placeholderItem: Any? { diff --git a/firefox-ios/firefox-ios-tests/Tests/ClientTests/Coordinators/BrowserCoordinatorTests.swift b/firefox-ios/firefox-ios-tests/Tests/ClientTests/Coordinators/BrowserCoordinatorTests.swift index 043be088766a..1d905316e573 100644 --- a/firefox-ios/firefox-ios-tests/Tests/ClientTests/Coordinators/BrowserCoordinatorTests.swift +++ b/firefox-ios/firefox-ios-tests/Tests/ClientTests/Coordinators/BrowserCoordinatorTests.swift @@ -7,6 +7,7 @@ import ComponentLibrary import MozillaAppServices import WebKit import XCTest +import GCDWebServers @testable import Client @@ -238,44 +239,75 @@ final class BrowserCoordinatorTests: XCTestCase, FeatureFlaggable { } } - func testShowShareSheet_addsShareSheetCoordinator() { + func testStartShareSheetCoordinator_addsShareSheetCoordinator() { let subject = createSubject() - subject.showShareSheet( - url: URL( - string: "https://www.google.com" - )!, - title: nil, + subject.startShareSheetCoordinator( + shareType: .site(url: URL(string: "https://www.google.com")!), + shareMessage: ShareMessage(message: "Test Message", subtitle: "Test Subtitle"), sourceView: UIView(), sourceRect: CGRect(), toastContainer: UIView(), popoverArrowDirection: .up ) + // NOTE: FXIOS-10824 We are waiting for an async call to complete. Hopefully the temporary document download will be + // improved in the future to make this call synchronous. + let predicate = NSPredicate { _, _ in + return self.mockRouter.presentCalled == 1 + } + let exp = XCTNSPredicateExpectation(predicate: predicate, object: .none) + + wait(for: [exp], timeout: 3.0) + XCTAssertEqual(subject.childCoordinators.count, 1) XCTAssertTrue(subject.childCoordinators.first is ShareSheetCoordinator) - XCTAssertEqual(mockRouter.presentCalled, 1) - XCTAssertTrue(mockRouter.presentedViewController is UIActivityViewController) - } + XCTAssertEqual(self.mockRouter.presentCalled, 1) + XCTAssertTrue(self.mockRouter.presentedViewController is UIActivityViewController) + } + + func testStartShareSheetCoordinator_isSharingTabWithTemporaryDocument_upgradesTabShareToFileShare() throws { + let testWebURL = URL(string: "https://mozilla.org")! + let testFileURL = URL(string: "file://some/file/url")! + let testWebpageDisplayTitle = "Mozilla" + let testShareMessage = ShareMessage(message: "Test Message", subtitle: "Test Subtitle") + let mockTemporaryDocument = MockTemporaryDocument(withFileURL: testFileURL) + let testTab = MockShareTab( + title: testWebpageDisplayTitle, + url: testWebURL, + canonicalURL: testWebURL, + withTemporaryDocument: mockTemporaryDocument + ) + + let mockServerURL = try startMockServer() - func testShowShareSheet_addsShareSheetCoordinatorWithTitle() { let subject = createSubject() - subject.showShareSheet( - url: URL( - string: "https://www.google.com" - )!, - title: "TEST TITLE", + subject.startShareSheetCoordinator( + shareType: .tab(url: mockServerURL, tab: testTab), + shareMessage: testShareMessage, sourceView: UIView(), sourceRect: CGRect(), toastContainer: UIView(), popoverArrowDirection: .up ) + // NOTE: FXIOS-10824 We are waiting for an async call to complete. Hopefully the temporary document download will be + // improved in the future to make this call synchronous. + let predicate = NSPredicate { _, _ in + return self.mockRouter.presentCalled == 1 + } + let exp = XCTNSPredicateExpectation(predicate: predicate, object: .none) + + wait(for: [exp], timeout: 3.0) + XCTAssertEqual(subject.childCoordinators.count, 1) XCTAssertTrue(subject.childCoordinators.first is ShareSheetCoordinator) XCTAssertEqual(mockRouter.presentCalled, 1) XCTAssertTrue(mockRouter.presentedViewController is UIActivityViewController) + // Right now we have no interface to check the ShareType passed in to ShareSheetCoordinator's start() call, but this + // can tell us that there was an attempt to download a TemporaryDocument for a tab type share, which is sufficient. + XCTAssertEqual(mockTemporaryDocument.getDownloadedURLCalled, 1) } func testShowCreditCardAutofill_addsCredentialAutofillCoordinator() { @@ -1174,4 +1206,22 @@ final class BrowserCoordinatorTests: XCTestCase, FeatureFlaggable { subject.handle(route: route) return result } + + // MARK: - Mock Server + + func startMockServer() throws -> URL { + let webServer = GCDWebServer() + + webServer.addHandler(forMethod: "GET", + path: "/", + request: GCDWebServerRequest.self) { (request) -> GCDWebServerResponse? in + return GCDWebServerDataResponse() + } + + if !webServer.start(withPort: 0, bonjourName: nil) { + XCTFail("Can't start the GCDWebServer") + } + + return URL(string: "http://localhost:\(webServer.port)")! + } } diff --git a/firefox-ios/firefox-ios-tests/Tests/ClientTests/Coordinators/Mocks/MockBrowserCoordinator.swift b/firefox-ios/firefox-ios-tests/Tests/ClientTests/Coordinators/Mocks/MockBrowserCoordinator.swift index 59c8b97a3d1d..edce52119b7e 100644 --- a/firefox-ios/firefox-ios-tests/Tests/ClientTests/Coordinators/Mocks/MockBrowserCoordinator.swift +++ b/firefox-ios/firefox-ios-tests/Tests/ClientTests/Coordinators/Mocks/MockBrowserCoordinator.swift @@ -65,14 +65,12 @@ class MockBrowserCoordinator: BrowserNavigationHandler, ParentCoordinatorDelegat showCreditCardAutofillCalled += 1 } - func showShareSheet( - url: URL, - title: String?, - sourceView: UIView, - sourceRect: CGRect?, - toastContainer: UIView, - popoverArrowDirection: UIPopoverArrowDirection - ) { + func showShareSheet(shareType: ShareType, + shareMessage: ShareMessage?, + sourceView: UIView, + sourceRect: CGRect?, + toastContainer: UIView, + popoverArrowDirection: UIPopoverArrowDirection) { showShareSheetCalled += 1 } diff --git a/firefox-ios/firefox-ios-tests/Tests/ClientTests/Coordinators/ShareSheetCoordinatorTests.swift b/firefox-ios/firefox-ios-tests/Tests/ClientTests/Coordinators/ShareSheetCoordinatorTests.swift index 885e7e2e2f95..08920af4d0a1 100644 --- a/firefox-ios/firefox-ios-tests/Tests/ClientTests/Coordinators/ShareSheetCoordinatorTests.swift +++ b/firefox-ios/firefox-ios-tests/Tests/ClientTests/Coordinators/ShareSheetCoordinatorTests.swift @@ -24,9 +24,10 @@ final class ShareSheetCoordinatorTests: XCTestCase { } func testStart_presentUIActivityViewController() { + let testURL = URL(string: "https://www.google.com")! let subject = createSubject() - subject.start(url: URL(string: "https://www.google.com")!, sourceView: UIView()) + subject.start(shareType: .site(url: testURL), shareMessage: nil, sourceView: UIView()) XCTAssertEqual(mockRouter.presentCalled, 1) XCTAssertTrue(mockRouter.presentedViewController is UIActivityViewController) diff --git a/firefox-ios/firefox-ios-tests/Tests/ClientTests/Mocks/MockShareTab.swift b/firefox-ios/firefox-ios-tests/Tests/ClientTests/Mocks/MockShareTab.swift index a3bf8a9b990d..4c708e7887b6 100644 --- a/firefox-ios/firefox-ios-tests/Tests/ClientTests/Mocks/MockShareTab.swift +++ b/firefox-ios/firefox-ios-tests/Tests/ClientTests/Mocks/MockShareTab.swift @@ -7,19 +7,35 @@ import Foundation @testable import Client class MockShareTab: ShareTab { + var canonicalURL: URL? var displayTitle: String var url: URL? var webView: TabWebView? + var temporaryDocument: TemporaryDocument? - init(title: String, url: URL?, withActiveWebView: Bool = true) { + init( + title: String, + url: URL?, + canonicalURL: URL?, + withActiveWebView: Bool = true, + withTemporaryDocument: TemporaryDocument? = nil + ) { self.displayTitle = title self.url = url + self.canonicalURL = canonicalURL self.webView = TabWebView(frame: CGRect.zero, configuration: .init(), windowUUID: .XCTestDefaultUUID) + self.temporaryDocument = withTemporaryDocument } static func == (lhs: MockShareTab, rhs: MockShareTab) -> Bool { + guard let lhsTempDoc = lhs.temporaryDocument as? MockTemporaryDocument, + let rhsTempDoc = rhs.temporaryDocument as? MockTemporaryDocument else { + return false + } + return lhs.displayTitle == rhs.displayTitle && lhs.url == rhs.url && lhs.webView === rhs.webView + && lhsTempDoc === rhsTempDoc } } diff --git a/firefox-ios/firefox-ios-tests/Tests/ClientTests/Mocks/MockTemporaryDocument.swift b/firefox-ios/firefox-ios-tests/Tests/ClientTests/Mocks/MockTemporaryDocument.swift new file mode 100644 index 000000000000..9a68e6cab3bc --- /dev/null +++ b/firefox-ios/firefox-ios-tests/Tests/ClientTests/Mocks/MockTemporaryDocument.swift @@ -0,0 +1,27 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/ + +import Foundation + +@testable import Client + +class MockTemporaryDocument: TemporaryDocument { + var fileURL: URL + var getURLCalled = 0 + var getDownloadedURLCalled = 0 + + init(withFileURL fileURL: URL) { + self.fileURL = fileURL + } + + func getURL(completionHandler: @escaping ((URL?) -> Void)) { + getURLCalled += 1 + completionHandler(fileURL) + } + + func getDownloadedURL() async -> URL? { + getDownloadedURLCalled += 1 + return fileURL + } +} diff --git a/firefox-ios/firefox-ios-tests/Tests/ClientTests/ShareManagerTests.swift b/firefox-ios/firefox-ios-tests/Tests/ClientTests/ShareManagerTests.swift new file mode 100644 index 000000000000..06c382d7e5bf --- /dev/null +++ b/firefox-ios/firefox-ios-tests/Tests/ClientTests/ShareManagerTests.swift @@ -0,0 +1,205 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/ + +import UniformTypeIdentifiers +import XCTest +@testable import Client + +final class ShareManagerTests: XCTestCase { + let testMessage = "Test message" + let testSubtitle = "Test subtitle" + let testFileURL = URL(string: "file://some/file/url")! + let testWebURL = URL(string: "https://mozilla.org")! + let testWebpageDisplayTitle = "Mozilla" + var testTab: (any ShareTab)! + + override func setUp() { + super.setUp() + testTab = MockShareTab(title: testWebpageDisplayTitle, url: testWebURL, canonicalURL: testWebURL) + } + + override func tearDown() { + testTab = nil + super.tearDown() + } + + // MARK: - Test sharing a file + + func testGetActivityItems_forFileURL_withNoShareText() throws { + let activityItems = ShareManager.getActivityItems( + forShareType: .file(url: testFileURL), + withExplicitShareMessage: nil + ) + + XCTAssertEqual(activityItems.count, 1) + + let urlActivityItemProvider = try XCTUnwrap(activityItems.first as? URLActivityItemProvider) + XCTAssertEqual(urlActivityItemProvider.item as? URL, testFileURL) + } + + func testGetActivityItems_forFileURL_withShareText() throws { + let testMessage = "Test message" + let testSubtitle = "Test subtitle" + let stubActivityViewController = UIActivityViewController( + activityItems: [], + applicationActivities: [] + ) + let activityItems = ShareManager.getActivityItems( + forShareType: .file(url: testFileURL), + withExplicitShareMessage: ShareMessage(message: testMessage, subtitle: testSubtitle) + ) + + // Test that we get back a URL provider and a title and subject provider with the correct content: + XCTAssertEqual(activityItems.count, 2) + + let urlActivityItemProvider = try XCTUnwrap(activityItems[safe: 0] as? URLActivityItemProvider) + let urlDataIdentifier = urlActivityItemProvider.activityViewController( + stubActivityViewController, + dataTypeIdentifierForActivityType: .mail + ) + + XCTAssertEqual(urlActivityItemProvider.item as? URL, testFileURL) + XCTAssertEqual(urlDataIdentifier, UTType.fileURL.identifier) + + let titleSubjectActivityItemProvider = try XCTUnwrap( + activityItems[safe: 1] as? TitleSubtitleActivityItemProvider + ) + let titleSubjectSubject = titleSubjectActivityItemProvider.activityViewController( + stubActivityViewController, + subjectForActivityType: .mail + ) + let titleSubjectDataIdentifier = titleSubjectActivityItemProvider.activityViewController( + stubActivityViewController, + dataTypeIdentifierForActivityType: .mail + ) + + XCTAssertEqual(titleSubjectActivityItemProvider.item as? String, testMessage) + XCTAssertEqual(titleSubjectSubject, testSubtitle) + XCTAssertEqual(titleSubjectDataIdentifier, UTType.text.identifier) + } + + // MARK: - Test sharing a website + + func testGetActivityItems_forWebsiteURL_withNoShareText() throws { + let activityItems = ShareManager.getActivityItems( + forShareType: .file(url: testWebURL), + withExplicitShareMessage: nil + ) + + XCTAssertEqual(activityItems.count, 1) + + let urlActivityItemProvider = try XCTUnwrap(activityItems.first as? URLActivityItemProvider) + XCTAssertEqual(urlActivityItemProvider.item as? URL, testWebURL) + } + + func testGetActivityItems_forWebsiteURL_withShareText() throws { + let testMessage = "Test message" + let testSubtitle = "Test subtitle" + let stubActivityViewController = UIActivityViewController( + activityItems: [], + applicationActivities: [] + ) + let activityItems = ShareManager.getActivityItems( + forShareType: .file(url: testWebURL), + withExplicitShareMessage: ShareMessage(message: testMessage, subtitle: testSubtitle) + ) + + // Test that we get back a URL provider and a title and subject provider with the correct content: + XCTAssertEqual(activityItems.count, 2) + + let urlActivityItemProvider = try XCTUnwrap(activityItems[safe: 0] as? URLActivityItemProvider) + let urlDataIdentifier = urlActivityItemProvider.activityViewController( + stubActivityViewController, + dataTypeIdentifierForActivityType: .mail + ) + + XCTAssertEqual(urlActivityItemProvider.item as? URL, testWebURL) + XCTAssertEqual(urlDataIdentifier, UTType.url.identifier) + + let titleSubjectActivityItemProvider = try XCTUnwrap( + activityItems[safe: 1] as? TitleSubtitleActivityItemProvider + ) + let titleSubjectSubject = titleSubjectActivityItemProvider.activityViewController( + stubActivityViewController, + subjectForActivityType: .mail + ) + let titleSubjectDataIdentifier = titleSubjectActivityItemProvider.activityViewController( + stubActivityViewController, + dataTypeIdentifierForActivityType: .mail + ) + + XCTAssertEqual(titleSubjectActivityItemProvider.item as? String, testMessage) + XCTAssertEqual(titleSubjectSubject, testSubtitle) + XCTAssertEqual(titleSubjectDataIdentifier, UTType.text.identifier) + } + + // MARK: - Test sharing a website with a related Tab and webview + + func testGetActivityItems_forTab_withNoShareText() throws { + let stubActivityViewController = UIActivityViewController( + activityItems: [], + applicationActivities: [] + ) + + let activityItems = ShareManager.getActivityItems( + forShareType: .tab(url: testWebURL, tab: testTab), + withExplicitShareMessage: nil + ) + + // Check we get all 4 types of share items for tabs below + XCTAssertEqual(activityItems.count, 4) + + let urlActivityItemProvider = try XCTUnwrap(activityItems[safe: 0] as? URLActivityItemProvider) + let urlDataIdentifier = urlActivityItemProvider.activityViewController( + stubActivityViewController, + dataTypeIdentifierForActivityType: .mail + ) + XCTAssertEqual(urlActivityItemProvider.item as? URL, testWebURL) + XCTAssertEqual(urlDataIdentifier, UTType.url.identifier) + + _ = try XCTUnwrap(activityItems[safe: 1] as? TabPrintPageRenderer) + + _ = try XCTUnwrap(activityItems[safe: 2] as? TabWebView) + + let titleActivityItemProvider = try XCTUnwrap(activityItems[safe: 3] as? TitleActivityItemProvider) + XCTAssertEqual( + titleActivityItemProvider.item as? String, + testWebpageDisplayTitle, + "When no explicit share message is set, we expect to see the webpage's title." + ) + } + + func testGetActivityItems_forTab_withShareText() throws { + let stubActivityViewController = UIActivityViewController( + activityItems: [], + applicationActivities: [] + ) + + let activityItems = ShareManager.getActivityItems( + forShareType: .tab(url: testWebURL, tab: testTab), + withExplicitShareMessage: ShareMessage(message: testMessage, subtitle: testSubtitle) + ) + + // Check we get all 4 types of share items for tabs below + XCTAssertEqual(activityItems.count, 4) + + let urlActivityItemProvider = try XCTUnwrap(activityItems[safe: 0] as? URLActivityItemProvider) + let urlDataIdentifier = urlActivityItemProvider.activityViewController( + stubActivityViewController, + dataTypeIdentifierForActivityType: .mail + ) + XCTAssertEqual(urlActivityItemProvider.item as? URL, testWebURL) + XCTAssertEqual(urlDataIdentifier, UTType.url.identifier) + _ = try XCTUnwrap(activityItems[safe: 1] as? TabPrintPageRenderer) + + _ = try XCTUnwrap(activityItems[safe: 2] as? TabWebView) + + let titleActivityItemProvider = try XCTUnwrap(activityItems[safe: 3] as? TitleSubtitleActivityItemProvider) + XCTAssertEqual( + titleActivityItemProvider.item as? String, + testMessage, + "When an explicit share message is set, we expect to see that message, not the webpage's title." + ) + } +}