Navigation failure handled in navigation bar buttons and their menus

DuckDuckGo/Browser Tab/Model/Tab.swift

@@ -284,23 +284,28 @@ final class Tab: NSObject {
     }
 
     func goForward() {
-        guard self.canGoForward else { return }
+        guard canGoForward else { return }
         shouldStoreNextVisit = false
         webView.goForward()
     }
 
     var canGoBack: Bool {
-        webView.canGoBack
+        webView.canGoBack || error != nil
     }
 
     func goBack() {
-        guard self.canGoBack else {
+        guard canGoBack else {
             if canBeClosedWithBack {
                 delegate?.closeTab(self)
             }
             return
         }
 
+        guard error == nil else {
+            webView.reload()
+            return
+        }
+
         shouldStoreNextVisit = false
         webView.goBack()
     }

DuckDuckGo/Browser Tab/ViewModel/TabViewModel.swift

@@ -114,6 +114,7 @@ final class TabViewModel {
             guard let self = self else { return }
             self.errorViewState.isVisible = self.tab.error != nil
             self.errorViewState.message = self.tab.error?.localizedDescription
+            self.updateCanGoBack()
         } .store(in: &cancellables)
     }
 
@@ -135,7 +136,7 @@ final class TabViewModel {
     }
 
     func updateCanGoBack() {
-        canGoBack = tab.canGoBack || tab.canBeClosedWithBack
+        canGoBack = tab.canGoBack || tab.canBeClosedWithBack || tab.error != nil
     }
 
     private func updateCanBeBookmarked() {

DuckDuckGo/Navigation Bar/View/NavigationButtonMenuDelegate.swift

@@ -40,53 +40,45 @@ final class NavigationButtonMenuDelegate: NSObject {
 extension NavigationButtonMenuDelegate: NSMenuDelegate {
 
     func numberOfItems(in menu: NSMenu) -> Int {
-        if listItems.count > 1 {
-            return listItems.count
-        } else if tabCollectionViewModel.selectedTabViewModel?.tab.canBeClosedWithBack == true {
-            return 1
-        }
-        return 0
+        let listItems = listItems
+
+        // Don't show menu if there is just the current item
+        if listItems.items.count == 0 || (listItems.items.count == 1 && listItems.currentIndex == 0) { return 0 }
+
+        return listItems.items.count
     }
 
     func menu(_ menu: NSMenu, update item: NSMenuItem, at index: Int, shouldCancel: Bool) -> Bool {
-        let listItems = self.listItems
-        let listItem: BackForwardListItem
-        if listItems.count > 1,
-           let item = listItems[safe: index] {
-            listItem = .backForwardListItem(item)
-        } else if let parentTab = tabCollectionViewModel.selectedTabViewModel?.tab.parentTab {
-            listItem = .goBackToCloseItem(parentTab: parentTab)
-        } else {
+        let (listItems, currentIndex) = self.listItems
+        guard let listItem = listItems[safe: index] else {
             os_log("%s: Index out of bounds", type: .error, className)
             return true
         }
 
         let listItemViewModel = WKBackForwardListItemViewModel(backForwardListItem: listItem,
                                                                faviconManagement: FaviconManager.shared,
                                                                historyCoordinating: HistoryCoordinator.shared,
-                                                               isCurrentItem: listItems[safe: index] === currentListItem)
+                                                               isCurrentItem: index == currentIndex)
 
         item.title = listItemViewModel.title
         item.image = listItemViewModel.image
         item.state =  listItemViewModel.state
 
         item.target = self
-        item.action = listItemViewModel.isGoBackToCloseItem ? #selector(goBackAction(_:)) : #selector(menuItemAction(_:))
+        item.action = #selector(menuItemAction(_:))
         item.tag = index
         return true
     }
 
     @objc func menuItemAction(_ sender: NSMenuItem) {
         let index = sender.tag
-        let listItems = self.listItems
-
-        guard index < listItems.count else {
+        let (listItems, currentIndex) = self.listItems
+        guard let listItem = listItems[safe: index] else {
             os_log("%s: Index out of bounds", type: .error, className)
             return
         }
-        let listItem = listItems[index]
 
-        guard listItem !== currentListItem else {
+        guard currentIndex != index else {
             // current item selected: do nothing
             return
         }
@@ -95,37 +87,45 @@ extension NavigationButtonMenuDelegate: NSMenuDelegate {
             return
         }
 
-        selectedTabViewModel.tab.go(to: listItem)
-    }
-
-    @objc func goBackAction(_: NSMenuItem) {
-        tabCollectionViewModel.selectedTabViewModel?.tab.goBack()
+        switch listItem {
+        case .backForwardListItem(let wkListItem):
+            selectedTabViewModel.tab.go(to: wkListItem)
+        case .goBackToCloseItem(parentTab:):
+            tabCollectionViewModel.selectedTabViewModel?.tab.goBack()
+        case .error:
+            break
+        }
     }
 
-    private var listItems: [WKBackForwardListItem] {
+    private var listItems: (items: [BackForwardListItem], currentIndex: Int?) {
         guard let selectedTabViewModel = tabCollectionViewModel.selectedTabViewModel else {
             os_log("%s: Selected tab view model is nil", type: .error, className)
-            return []
+            return ([], nil)
         }
 
         let backForwardList = selectedTabViewModel.tab.webView.backForwardList
-        var list = buttonType == .back ? backForwardList.backList.reversed() : backForwardList.forwardList
+        let wkList = buttonType == .back ? backForwardList.backList.reversed() : backForwardList.forwardList
+        var list = wkList.map { BackForwardListItem.backForwardListItem($0) }
+        var currentIndex: Int?
 
-        guard let currentItem = selectedTabViewModel.tab.webView.backForwardList.currentItem else {
-            return list
+        // Add closing with back button to the list
+        if list.count == 0, let parentTab = selectedTabViewModel.tab.parentTab {
+            list.insert(.goBackToCloseItem(parentTab: parentTab), at: 0)
         }
-        list.insert(currentItem, at: 0)
 
-        return list
-    }
+        // Add current item to the list
+        if let currentItem = selectedTabViewModel.tab.webView.backForwardList.currentItem {
+            list.insert(.backForwardListItem(currentItem), at: 0)
+            currentIndex = 0
+        }
 
-    private var currentListItem: WKBackForwardListItem? {
-        guard let selectedTabViewModel = tabCollectionViewModel.selectedTabViewModel else {
-            os_log("%s: Selected tab view model is nil", type: .error, className)
-            return nil
+        // Add error to the list
+        if selectedTabViewModel.tab.error != nil {
+            list.insert(.error, at: 0)
+            currentIndex = 0
         }
 
-        return selectedTabViewModel.tab.webView.backForwardList.currentItem
+        return (list, currentIndex)
     }
 
 }

DuckDuckGo/Navigation Bar/ViewModel/BackForwardListItem.swift

@@ -18,16 +18,19 @@
 
 import WebKit
 
-enum BackForwardListItem {
+enum BackForwardListItem: Equatable {
     case backForwardListItem(WKBackForwardListItem)
     case goBackToCloseItem(parentTab: Tab)
+    case error
 
     var url: URL? {
         switch self {
         case .backForwardListItem(let item):
             return item.url
         case .goBackToCloseItem(parentTab: let tab):
             return tab.content.url
+        case .error:
+            return nil
         }
     }
 

DuckDuckGo/Navigation Bar/ViewModel/WKBackForwardListItemViewModel.swift

@@ -60,10 +60,16 @@ final class WKBackForwardListItemViewModel {
             } else {
                 return UserText.closeAndReturnToParent
             }
+        case .error:
+            return UserText.tabErrorTitle
         }
     }
 
     var image: NSImage? {
+        if case .error = backForwardListItem {
+            return nil
+        }
+
         if backForwardListItem.url == .homePage {
             return NSImage(named: "HomeFavicon")
         }
@@ -78,16 +84,18 @@ final class WKBackForwardListItemViewModel {
     }
 
     var state: NSControl.StateValue {
-        if case .backForwardListItem = backForwardListItem {
-            return isCurrentItem ? .on : .off
+        if case .goBackToCloseItem = backForwardListItem {
+            return .off
         }
-        return .off
+
+        return isCurrentItem ? .on : .off
     }
 
     var isGoBackToCloseItem: Bool {
         if case .goBackToCloseItem = backForwardListItem {
             return true
         }
+
         return false
     }