From 45df6b6aa4212a0f81ce1c81612741b2f28efb4b Mon Sep 17 00:00:00 2001 From: Brandon T Date: Wed, 29 Nov 2023 10:17:38 -0500 Subject: [PATCH] Fix URLs not opening in active-tab Fix spamming error alerts Block synthetic clicks. Also do not allow the error alert to show above the suppression alert in Z-Order. Get rid of main-frame check. Universal links can come from any frame. Use tab visibility check so that in-active or backgrounded tabs cannot show alerts or open universal links. --- ...rViewController+WKNavigationDelegate.swift | 272 ++++++++++-------- 1 file changed, 148 insertions(+), 124 deletions(-) diff --git a/Sources/Brave/Frontend/Browser/BrowserViewController/BrowserViewController+WKNavigationDelegate.swift b/Sources/Brave/Frontend/Browser/BrowserViewController/BrowserViewController+WKNavigationDelegate.swift index cceefc7ee83..e68bc9c1ff5 100644 --- a/Sources/Brave/Frontend/Browser/BrowserViewController/BrowserViewController+WKNavigationDelegate.swift +++ b/Sources/Brave/Frontend/Browser/BrowserViewController/BrowserViewController+WKNavigationDelegate.swift @@ -179,9 +179,8 @@ extension BrowserViewController: WKNavigationDelegate { let tab = tab(for: webView) if ["sms", "tel", "facetime", "facetime-audio"].contains(requestURL.scheme) { - // Do not allow opening external URLs from child tabs - handleExternalURL(requestURL, tab: tab, navigationAction: navigationAction) - return (.cancel, preferences) + let shouldOpen = await handleExternalURL(requestURL, tab: tab, navigationAction: navigationAction) + return (shouldOpen ? .allow : .cancel, preferences) } // Second special case are a set of URLs that look like regular http links, but should be handed over to iOS @@ -189,22 +188,19 @@ extension BrowserViewController: WKNavigationDelegate { // In addition we are exchaging actual scheme with "maps" scheme // So the Apple maps URLs will open properly if let mapsURL = isAppleMapsURL(requestURL), mapsURL.enabled { - // Do not allow opening external URLs from child tabs - handleExternalURL(mapsURL.url, tab: tab, navigationAction: navigationAction) - return (.cancel, preferences) + let shouldOpen = await handleExternalURL(mapsURL.url, tab: tab, navigationAction: navigationAction) + return (shouldOpen ? .allow : .cancel, preferences) } if isStoreURL(requestURL) { - // Do not allow opening external URLs from child tabs - handleExternalURL(requestURL, tab: tab, navigationAction: navigationAction) - return (.cancel, preferences) + let shouldOpen = await handleExternalURL(requestURL, tab: tab, navigationAction: navigationAction) + return (shouldOpen ? .allow : .cancel, preferences) } // Handles custom mailto URL schemes. if requestURL.scheme == "mailto" { - // Do not allow opening external URLs from child tabs - handleExternalURL(requestURL, tab: tab, navigationAction: navigationAction) - return (.cancel, preferences) + let shouldOpen = await handleExternalURL(requestURL, tab: tab, navigationAction: navigationAction) + return (shouldOpen ? .allow : .cancel, preferences) } // handles IPFS URL schemes @@ -406,15 +402,29 @@ extension BrowserViewController: WKNavigationDelegate { // Our own 'brave' scheme does not require the switch-app prompt. if requestURL.scheme?.contains("brave") == false { // Do not allow opening external URLs from child tabs - handleExternalURL(requestURL, tab: tab, navigationAction: navigationAction) { didOpenURL in - // Do not show error message for JS navigated links or redirect - // as it's not the result of a user action. - if !didOpenURL, navigationAction.navigationType == .linkActivated { - let alert = UIAlertController(title: Strings.unableToOpenURLErrorTitle, message: Strings.unableToOpenURLError, preferredStyle: .alert) - alert.addAction(UIAlertAction(title: Strings.OKString, style: .default, handler: nil)) - self.present(alert, animated: true, completion: nil) + let shouldOpen = await handleExternalURL(requestURL, tab: tab, navigationAction: navigationAction) + let isSyntheticClick = navigationAction.responds(to: Selector(("_syntheticClickType"))) && + navigationAction.value(forKey: "syntheticClickType") as? Int == 0 + + // Do not show error message for JS navigated links or redirect + // as it's not the result of a user action. + if !shouldOpen, navigationAction.navigationType == .linkActivated && !isSyntheticClick { + if self.presentedViewController == nil && + self.presentingViewController == nil && + tab?.isExternalAppAlertPresented == false && + tab?.isExternalAppAlertSuppressed == false { + + return await withCheckedContinuation { continuation in + let alert = UIAlertController(title: Strings.unableToOpenURLErrorTitle, message: Strings.unableToOpenURLError, preferredStyle: .alert) + alert.addAction(UIAlertAction(title: Strings.OKString, style: .default, handler: nil)) + self.present(alert, animated: true) { + continuation.resume(returning: (shouldOpen ? .allow : .cancel, preferences)) + } + } } } + + return (shouldOpen ? .allow : .cancel, preferences) } return (.cancel, preferences) @@ -797,125 +807,139 @@ extension BrowserViewController { private func handleExternalURL( _ url: URL, tab: Tab?, - navigationAction: WKNavigationAction, - openedURLCompletionHandler: ((Bool) -> Void)? = nil - ) { - // Do not open external links for child tabs automatically - // The user must tap on the link to open it. - if tab?.parent != nil && navigationAction.navigationType != .linkActivated { - return - } - - // Check if the current url of the caller has changed - if let domain = tab?.url?.baseDomain, - domain != tab?.externalAppURLDomain { - tab?.externalAppAlertCounter = 0 - tab?.isExternalAppAlertSuppressed = false - } - tab?.externalAppURLDomain = tab?.url?.baseDomain - - // Do not try to present over existing warning - if tab?.isExternalAppAlertPresented == true || tab?.isExternalAppAlertSuppressed == true { - return - } - - // We do not schemes to be opened externally when called from subframes. - // And external dialog should not be shown for non-active tabs #6687 - #7835 - - // Check navigation action is from main frame as first step - let isMainFrame = navigationAction.targetFrame?.isMainFrame == true - - // Check user trying to open on NTP like external link browsing - var isAboutHome = false - if let url = tab?.url { - isAboutHome = InternalURL(url)?.isAboutHomeURL == true - } - - // Finally check non-active tab - let isNonActiveTab = isAboutHome ? false : tab?.url?.host != topToolbar.currentURL?.host - - if !isMainFrame || isNonActiveTab { - return - } - - var alertTitle = Strings.openExternalAppURLGenericTitle - - if let displayHost = tab?.url?.withoutWWW.host { - alertTitle = String(format: Strings.openExternalAppURLTitle, displayHost) - } - - // Handling condition when Tab is empty when handling an external URL we should remove the tab once the user decides - let removeTabIfEmpty = { [weak self] in - if let tab = tab, tab.url == nil { - self?.tabManager.removeTab(tab) + navigationAction: WKNavigationAction) async -> Bool { + // Do not open external links for child tabs automatically + // The user must tap on the link to open it. + if tab?.parent != nil && navigationAction.navigationType != .linkActivated { + return false } - } - - // Show the external sceheme invoke alert - func showExternalSchemeAlert(isSuppressActive: Bool) { - // Check if active controller is bvc otherwise do not show show external sceheme alerts - guard shouldShowExternalSchemeAlert() else { return } - view.endEditing(true) - tab?.isExternalAppAlertPresented = true - - let popup = AlertPopupView( - imageView: nil, - title: alertTitle, - message: String(format: Strings.openExternalAppURLMessage, url.relativeString), - titleWeight: .semibold, - titleSize: 21 - ) - if isSuppressActive { - popup.addButton(title: Strings.suppressAlertsActionTitle, type: .destructive) { [weak tab] () -> PopupViewDismissType in - tab?.isExternalAppAlertSuppressed = true - return .flyDown + // Check if the current url of the caller has changed + if let domain = tab?.url?.baseDomain, + domain != tab?.externalAppURLDomain { + tab?.externalAppAlertCounter = 0 + tab?.isExternalAppAlertSuppressed = false + } + + tab?.externalAppURLDomain = tab?.url?.baseDomain + + // Do not try to present over existing warning + if tab?.isExternalAppAlertPresented == true || tab?.isExternalAppAlertSuppressed == true { + return false + } + + // External dialog should not be shown for non-active tabs #6687 - #7835 + let isVisibleTab = tab?.isTabVisible() == true + + // Check user trying to open on NTP like external link browsing + var isAboutHome = false + if let url = tab?.url { + isAboutHome = InternalURL(url)?.isAboutHomeURL == true + } + + // Finally check non-active tab + let isNonActiveTab = isAboutHome ? false : tab?.url?.host != topToolbar.currentURL?.host + + if !isVisibleTab || isNonActiveTab { + return false + } + + var alertTitle = Strings.openExternalAppURLGenericTitle + + if let displayHost = tab?.url?.withoutWWW.host { + alertTitle = String(format: Strings.openExternalAppURLTitle, displayHost) + } + + // Handling condition when Tab is empty when handling an external URL we should remove the tab once the user decides + let removeTabIfEmpty = { [weak self] in + if let tab = tab, tab.url == nil { + self?.tabManager.removeTab(tab) } - } else { - popup.addButton(title: Strings.openExternalAppURLDontAllow) { [weak tab] () -> PopupViewDismissType in + } + + // Show the external sceheme invoke alert + @MainActor + func showExternalSchemeAlert(isSuppressActive: Bool, openedURLCompletionHandler: @escaping (Bool) -> Void) { + // Check if active controller is bvc otherwise do not show show external sceheme alerts + guard shouldShowExternalSchemeAlert() else { + openedURLCompletionHandler(false) + return + } + + view.endEditing(true) + tab?.isExternalAppAlertPresented = true + + let popup = AlertPopupView( + imageView: nil, + title: alertTitle, + message: String(format: Strings.openExternalAppURLMessage, url.relativeString), + titleWeight: .semibold, + titleSize: 21 + ) + + if isSuppressActive { + popup.addButton(title: Strings.suppressAlertsActionTitle, type: .destructive) { [weak tab] () -> PopupViewDismissType in + openedURLCompletionHandler(false) + tab?.isExternalAppAlertSuppressed = true + return .flyDown + } + } else { + popup.addButton(title: Strings.openExternalAppURLDontAllow) { [weak tab] () -> PopupViewDismissType in + openedURLCompletionHandler(false) + removeTabIfEmpty() + tab?.isExternalAppAlertPresented = false + return .flyDown + } + } + popup.addButton(title: Strings.openExternalAppURLAllow, type: .primary) { [weak tab] () -> PopupViewDismissType in + if UIApplication.shared.canOpenURL(url) { + UIApplication.shared.open(url, options: [:]) { didOpen in + openedURLCompletionHandler(!didOpen) + } + } else { + openedURLCompletionHandler(true) + } removeTabIfEmpty() tab?.isExternalAppAlertPresented = false return .flyDown } - } - popup.addButton(title: Strings.openExternalAppURLAllow, type: .primary) { [weak tab] () -> PopupViewDismissType in - UIApplication.shared.open(url, options: [:], completionHandler: openedURLCompletionHandler) - removeTabIfEmpty() - tab?.isExternalAppAlertPresented = false - return .flyDown - } - popup.showWithType(showType: .flyUp) - } - - func shouldShowExternalSchemeAlert() -> Bool { - guard let rootVC = currentScene?.browserViewController else { - return false + popup.showWithType(showType: .flyUp) } - func topViewController(startingFrom viewController: UIViewController) -> UIViewController { - var top = viewController - if let navigationController = top as? UINavigationController, - let vc = navigationController.visibleViewController { - return topViewController(startingFrom: vc) - } - if let tabController = top as? UITabBarController, - let vc = tabController.selectedViewController { - return topViewController(startingFrom: vc) + func shouldShowExternalSchemeAlert() -> Bool { + guard let rootVC = currentScene?.browserViewController else { + return false } - while let next = top.presentedViewController { - top = next + + func topViewController(startingFrom viewController: UIViewController) -> UIViewController { + var top = viewController + if let navigationController = top as? UINavigationController, + let vc = navigationController.visibleViewController { + return topViewController(startingFrom: vc) + } + if let tabController = top as? UITabBarController, + let vc = tabController.selectedViewController { + return topViewController(startingFrom: vc) + } + while let next = top.presentedViewController { + top = next + } + return top } - return top + + let isTopController = self == topViewController(startingFrom: rootVC) + let isTopWindow = view.window?.isKeyWindow == true + return isTopController && isTopWindow } - let isTopController = self == topViewController(startingFrom: rootVC) - let isTopWindow = view.window?.isKeyWindow == true - return isTopController && isTopWindow + tab?.externalAppAlertCounter += 1 + + return await withCheckedContinuation { continuation in + showExternalSchemeAlert(isSuppressActive: tab?.externalAppAlertCounter ?? 0 > 2) { + continuation.resume(with: .success($0)) + } + } } - - tab?.externalAppAlertCounter += 1 - showExternalSchemeAlert(isSuppressActive: tab?.externalAppAlertCounter ?? 0 > 2) - } } // MARK: WKUIDelegate