Fixes #1596 & Fixes #1599: Swipe to go back removes search history in…

… url bar (#1597) * Fixes #1596: Swipe to go back removes search history in url bar * Fixes #1599: Reload pushes to SearchHistoryUtils
mozilla-mobile · Nov 29, 2018 · d13794b · d13794b
1 parent b08043e
commit d13794b
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 6 deletions.
diff --git a/Blockzilla/BrowserViewController.swift b/Blockzilla/BrowserViewController.swift
@@ -1017,24 +1017,33 @@ extension BrowserViewController: BrowserToolsetDelegate {
     }
 
     func browserToolsetDidPressBack(_ browserToolset: BrowserToolset) {
-        guard let navigatingFromAmpSite = urlBar.url?.absoluteString.hasPrefix(UIConstants.strings.googleAmpURLPrefix) else { return }
+        webViewController.goBack()
+    }
+
+    private func handleNavigationBack() {
+        // Check if the previous site we were on was AMP
+        guard let navigatingFromAmpSite = SearchHistoryUtils.pullSearchFromStack()?.hasPrefix(UIConstants.strings.googleAmpURLPrefix) else {
+            return
+        }
 
         // Make sure our navigation is not pushed to the SearchHistoryUtils stack (since it already exists there)
         SearchHistoryUtils.isFromURLBar = true
 
+        // This function is now getting called after our new url is set!!
         if !navigatingFromAmpSite {
             SearchHistoryUtils.goBack()
         }
-
-        webViewController.goBack()
     }
 
     func browserToolsetDidPressForward(_ browserToolset: BrowserToolset) {
+        webViewController.goForward()
+    }
+
+    private func handleNavigationForward() {
         // Make sure our navigation is not pushed to the SearchHistoryUtils stack (since it already exists there)
         SearchHistoryUtils.isFromURLBar = true
-        webViewController.goForward()
 
-        // Check if we're navigating to an AMP site *after* the URL bar is updated. This is intentionally after the goForward call.
+        // Check if we're navigating to an AMP site *after* the URL bar is updated. This is intentionally grabbing the NEW url
         guard let navigatingToAmpSite = urlBar.url?.absoluteString.hasPrefix(UIConstants.strings.googleAmpURLPrefix) else { return }
 
         if !navigatingToAmpSite {
@@ -1187,10 +1196,15 @@ extension BrowserViewController: WebControllerDelegate {
         }
     }
 
+    func webControllerDidReload(_ controller: WebController) {
+        SearchHistoryUtils.isReload = true
+    }
+
     func webControllerDidStartNavigation(_ controller: WebController) {
-        if !SearchHistoryUtils.isFromURLBar && !SearchHistoryUtils.isNavigating {
+        if !SearchHistoryUtils.isFromURLBar && !SearchHistoryUtils.isNavigating && !SearchHistoryUtils.isReload {
             SearchHistoryUtils.pushSearchToStack(with: (urlBar.url?.absoluteString)!)
         }
+        SearchHistoryUtils.isReload = false
         SearchHistoryUtils.isNavigating = false
         SearchHistoryUtils.isFromURLBar = false
         urlBar.isLoading = true
@@ -1310,6 +1324,14 @@ extension BrowserViewController: WebControllerDelegate {
         return true
     }
 
+    func webControllerDidNavigateBack(_ controller: WebController) {
+        handleNavigationBack()
+    }
+
+    func webControllerDidNavigateForward(_ controller: WebController) {
+        handleNavigationForward()
+    }
+
     func webController(_ controller: WebController, stateDidChange state: BrowserState) {}
 
     func webController(_ controller: WebController, didUpdateTrackingProtectionStatus trackingStatus: TrackingProtectionStatus) {

diff --git a/Blockzilla/Modules/WebView/WebViewController.swift b/Blockzilla/Modules/WebView/WebViewController.swift
@@ -27,6 +27,9 @@ protocol WebControllerDelegate: class {
     func webControllerDidStartProvisionalNavigation(_ controller: WebController)
     func webControllerDidStartNavigation(_ controller: WebController)
     func webControllerDidFinishNavigation(_ controller: WebController)
+    func webControllerDidNavigateBack(_ controller: WebController)
+    func webControllerDidNavigateForward(_ controller: WebController)
+    func webControllerDidReload(_ controller: WebController)
     func webControllerURLDidChange(_ controller: WebController, url: URL)
     func webController(_ controller: WebController, didFailNavigationWithError error: Error)
     func webController(_ controller: WebController, didUpdateCanGoBack canGoBack: Bool)
@@ -62,6 +65,7 @@ class WebViewController: UIViewController, WebController {
     var onePasswordExtensionItem: NSExtensionItem!
     private var progressObserver: NSKeyValueObservation?
     private var urlObserver: NSKeyValueObservation?
+    private var currentBackForwardItem: WKBackForwardListItem?
     private var userAgent: UserAgent?
     private var trackingProtectionStatus = TrackingProtectionStatus.on(TPPageStats()) {
         didSet {
@@ -280,6 +284,22 @@ extension WebViewController: WKNavigationDelegate {
     func webView(_ webView: WKWebView, decidePolicyFor navigationAction: WKNavigationAction, decisionHandler: @escaping (WKNavigationActionPolicy) -> Void) {
         let present: (UIViewController) -> Void = { self.present($0, animated: true, completion: nil) }
 
+        switch navigationAction.navigationType {
+            case .backForward:
+                let navigatingBack = webView.backForwardList.backList.filter { $0 == currentBackForwardItem }.count == 0
+                if navigatingBack {
+                    delegate?.webControllerDidNavigateBack(self)
+                } else {
+                    delegate?.webControllerDidNavigateForward(self)
+                }
+            case .reload:
+                delegate?.webControllerDidReload(self)
+            default:
+                break
+        }
+
+        currentBackForwardItem = webView.backForwardList.currentItem
+
         // prevent Focus from opening universal links
         // https://stackoverflow.com/questions/38450586/prevent-universal-links-from-opening-in-wkwebview-uiwebview
         let allowDecision = WKNavigationActionPolicy(rawValue: WKNavigationActionPolicy.allow.rawValue + 2) ?? .allow

diff --git a/Blockzilla/SearchHistoryUtils.swift b/Blockzilla/SearchHistoryUtils.swift
@@ -28,6 +28,7 @@ class SearchHistoryUtils {
 
     static var isFromURLBar = false
     static var isNavigating = false
+    static var isReload = false
 
     static func pushSearchToStack(with searchedText: String) {
         var currentStack = [textSearched]()