Fixed address bar cleared/losing focus on Navigation

DuckDuckGo/Common/Extensions/NSViewExtension.swift

@@ -84,6 +84,8 @@ extension NSView {
             os_log("%s: Window not available", type: .error, className)
             return
         }
+        // prevent all text selection on repeated Address Bar activation
+        guard window.firstResponder !== (self as? NSControl)?.currentEditor() ?? self else { return }
 
         window.makeFirstResponder(self)
     }

DuckDuckGo/MainWindow/MainViewController.swift

@@ -141,6 +141,7 @@ final class MainViewController: NSViewController {
         }
 
         updateDividerColor()
+        adjustFirstResponder()
     }
 
     override func viewDidLayout() {

DuckDuckGo/NavigationBar/View/AddressBarTextField.swift

@@ -47,6 +47,10 @@
         tabCollectionViewModel.isBurner
     }
 
+    var isFirstResponder: Bool {
+        window?.firstResponder == currentEditor()
+    }
+
     private var suggestionResultCancellable: AnyCancellable?
     private var selectedSuggestionViewModelCancellable: AnyCancellable?
     private var selectedTabViewModelCancellable: AnyCancellable?
@@ -104,41 +108,36 @@
 
     private func subscribeToSelectedTabViewModel() {
         selectedTabViewModelCancellable = tabCollectionViewModel.$selectedTabViewModel
-            .receive(on: DispatchQueue.main)
-            .sink { [weak self] _ in
-                self?.restoreValueIfPossible()
-                self?.subscribeToAddressBarString()
-                self?.subscribeToContentType()
+            .compactMap { $0 }
+            .sink { [weak self] selectedTabViewModel in
+                self?.restoreValueIfPossible(newSelectedTabViewModel: selectedTabViewModel)
+                self?.subscribeToAddressBarString(selectedTabViewModel: selectedTabViewModel)
+                self?.subscribeToContentType(selectedTabViewModel: selectedTabViewModel)
             }
     }
 
-    private func subscribeToContentType() {
-        contentTypeCancellable = tabCollectionViewModel.selectedTabViewModel?.tab.$content
-            .receive(on: DispatchQueue.main)
+    private func subscribeToContentType(selectedTabViewModel: TabViewModel) {
+        contentTypeCancellable = selectedTabViewModel.tab.$content
             .sink { [weak self] contentType in
                 self?.font = .systemFont(ofSize: contentType == .newtab ? 15 : 13)
             }
     }
 
-    private func subscribeToAddressBarString() {
-        addressBarStringCancellable?.cancel()
-
-        guard let selectedTabViewModel = tabCollectionViewModel.selectedTabViewModel else {
-            clearValue()
-            return
-        }
-        addressBarStringCancellable = selectedTabViewModel.$addressBarString.dropFirst().receive(on: DispatchQueue.main).sink { [weak self] _ in
-            self?.updateValue()
-        }
+    private func subscribeToAddressBarString(selectedTabViewModel: TabViewModel) {
+        addressBarStringCancellable = selectedTabViewModel.$addressBarString
+            .sink { [weak self, weak selectedTabViewModel] addressBarString in
+                guard let self, let selectedTabViewModel else { return }
+                updateValueIfNeeded(selectedTabViewModel: selectedTabViewModel, addressBarString: addressBarString)
+            }
     }
 
     // MARK: - Value
 
-    @Published private(set) var value: Value = .text("") {
+    @Published private(set) var value: Value = .text("", userTyped: false) {
         didSet {
             guard value != oldValue else { return }
 
-            saveValue(oldValue: oldValue)
+            saveUndoValue(oldValue: oldValue)
             updateAttributedStringValue()
 
             if let editor, case .suggestion(let suggestion) = value {
@@ -191,9 +190,7 @@
         }
     }
 
-    private func saveValue(oldValue: Value) {
-        tabCollectionViewModel.selectedTabViewModel?.lastAddressBarTextFieldValue = value
-
+    private func saveUndoValue(oldValue: Value) {
         guard let undoManager else { return }
         // disable recording undo Value when iterating through suggestions
         if oldValue.isSuggestion && value.isSuggestion { return }
@@ -209,35 +206,38 @@
         }
     }
 
-    private func restoreValueIfPossible() {
-        guard let selectedTabViewModel = tabCollectionViewModel.selectedTabViewModel else {
-            clearValue()
-            return
+    private func restoreValueIfPossible(newSelectedTabViewModel: TabViewModel) {
+        // save current (possibly modified) value into the old TabViewModel when selecting another Tab
+        if let oldSelectedTabViewModel = tabCollectionViewModel.selectedTabViewModel {
+            guard oldSelectedTabViewModel !== newSelectedTabViewModel else {
+                updateValue(selectedTabViewModel: newSelectedTabViewModel, addressBarString: nil)
+                return
+            }
+            oldSelectedTabViewModel.lastAddressBarTextFieldValue = value
         }
-
-        let lastAddressBarTextFieldValue = selectedTabViewModel.lastAddressBarTextFieldValue
+        let lastAddressBarTextFieldValue = newSelectedTabViewModel.lastAddressBarTextFieldValue
 
         switch lastAddressBarTextFieldValue {
-        case .text(let text):
+        case .text(let text, userTyped: let userTyped):
             if !text.isEmpty {
-                restoreValue(.text(text))
+                restoreValue(.text(text, userTyped: userTyped))
             } else {
-                updateValue()
+                updateValue(selectedTabViewModel: newSelectedTabViewModel, addressBarString: nil)
             }
         case .suggestion(let suggestionViewModel):
             let suggestion = suggestionViewModel.suggestion
             switch suggestion {
             case .website, .bookmark, .historyEntry:
                 restoreValue(Value(stringValue: suggestionViewModel.autocompletionString, userTyped: true))
             case .phrase(phrase: let phase):
-                restoreValue(Value.text(phase))
-            default:
-                updateValue()
+                restoreValue(Value.text(phase, userTyped: false))
+            case .unknown:
+                updateValue(selectedTabViewModel: newSelectedTabViewModel, addressBarString: nil)
             }
         case .url(urlString: let urlString, url: _, userTyped: true):
             restoreValue(Value(stringValue: urlString, userTyped: true))
-        default:
-            updateValue()
+        case .url, .none:
+            updateValue(selectedTabViewModel: newSelectedTabViewModel, addressBarString: nil)
         }
     }
 
@@ -247,17 +247,35 @@
         clearUndoManager()
     }
 
-    private func updateValue() {
-        guard let selectedTabViewModel = tabCollectionViewModel.selectedTabViewModel else { return }
+    /// don‘t update value when the addressisis
+    private func updateValueIfNeeded(selectedTabViewModel: TabViewModel?, addressBarString: String) {
+        var shouldUpdateValue: Bool {
+            switch self.value {
+            case .suggestion(let suggestionViewModel):
+                switch suggestionViewModel.suggestion {
+                case .phrase, .website, .bookmark, .historyEntry: return false
+                case .unknown(let value): return true
+                }
+            case .text(_, userTyped: true), .url(_, _, userTyped: true): return false
+            case .text, .url: return true
+            }
+        }
+        if !self.isFirstResponder || shouldUpdateValue {
+            updateValue(selectedTabViewModel: selectedTabViewModel, addressBarString: addressBarString)
+        }
+    }
+
+    private func updateValue(selectedTabViewModel: TabViewModel?, addressBarString: String?) {
+        guard let selectedTabViewModel = selectedTabViewModel ?? tabCollectionViewModel.selectedTabViewModel else { return }
 
-        let addressBarString = selectedTabViewModel.addressBarString
+        let addressBarString = addressBarString ?? selectedTabViewModel.addressBarString
         let isSearch = selectedTabViewModel.tab.content.url?.isDuckDuckGoSearch ?? false
         self.value = Value(stringValue: addressBarString, userTyped: false, isSearch: isSearch)
         clearUndoManager()
     }
 
     func clearValue() {
-        self.value = .text("")
+        self.value = .text("", userTyped: false)
         suggestionContainerViewModel?.clearSelection()
         suggestionContainerViewModel?.clearUserStringValue()
         hideSuggestionWindow()
@@ -291,7 +309,7 @@
         }
 
         clearValue()
-        updateValue()
+        updateValue(selectedTabViewModel: nil, addressBarString: nil)
     }
 
     private func updateTabUrlWithUrl(_ providedUrl: URL, userEnteredValue: String, suggestion: Suggestion?) {
@@ -503,7 +521,7 @@
             return
         }
 
-        guard !suggestionWindow.isVisible, window.firstResponder == currentEditor() else { return }
+        guard !suggestionWindow.isVisible, isFirstResponder else { return }
 
         window.addChildWindow(suggestionWindow, ordered: .above)
         layoutSuggestionWindow()
@@ -632,7 +650,7 @@
 extension AddressBarTextField {
 
     enum Value: Equatable {
-        case text(_ text: String)
+        case text(_ text: String, userTyped: Bool)
         case url(urlString: String, url: URL, userTyped: Bool)
         case suggestion(_ suggestionViewModel: SuggestionViewModel)
 
@@ -646,13 +664,13 @@
                 }
                 self = .url(urlString: stringValue, url: url, userTyped: userTyped)
             } else {
-                self = .text(stringValue)
+                self = .text(stringValue, userTyped: userTyped)
             }
         }
 
         var string: String {
             switch self {
-            case .text(let text):
+            case .text(let text, _):
                 return text
             case .url(urlString: let urlString, url: _, userTyped: _):
                 return urlString
@@ -674,7 +692,7 @@
 
         var isEmpty: Bool {
             switch self {
-            case .text(let text):
+            case .text(let text, _):
                 return text.isEmpty
             case .url(urlString: let urlString, url: _, userTyped: _):
                 return urlString.isEmpty
@@ -723,7 +741,7 @@
 
     enum Suffix {
         init?(value: Value) {
-            if case .text("") = value {
+            if case .text("", _) = value {
                 return nil
             }
 

DuckDuckGo/Tab/View/BrowserTabViewController.swift

@@ -16,12 +16,12 @@
 //  limitations under the License.
 //
 
+import BrowserServicesKit
 import Cocoa
-import WebKit
 import Combine
 import Common
 import SwiftUI
-import BrowserServicesKit
+import WebKit
 
 final class BrowserTabViewController: NSViewController {
     @IBOutlet var errorView: NSView!
@@ -412,7 +412,25 @@ final class BrowserTabViewController: NSViewController {
 
     private var setFirstResponderAfterAdding = false
 
+    private var shouldMakeWebViewFirstResponder: Bool {
+        if !(view.window?.firstResponder is NSText) {
+            return true
+        } else if case .url(_, _, source: let source) = tabViewModel?.tab.content {
+            switch source {
+            case .pendingStateRestoration, .loadedByStateRestoration, .webViewUpdated:
+                // prevent Address Bar deactivation when the WebView is restoring state or updates url on redirect
+                return false
+            case .userEntered, .historyEntry, .bookmark, .ui, .link, .appOpenUrl, .reload:
+                return true
+            }
+        } else {
+            return true
+        }
+    }
+
     private func setFirstResponderIfNeeded() {
+
+        guard shouldMakeWebViewFirstResponder else { return }
         guard let webView else {
             setFirstResponderAfterAdding = true
             return
@@ -422,7 +440,8 @@ final class BrowserTabViewController: NSViewController {
         }
 
         DispatchQueue.main.async { [weak self] in
-            self?.makeWebViewFirstResponder()
+            guard let self, shouldMakeWebViewFirstResponder else { return }
+            self.makeWebViewFirstResponder()
         }
     }
 

DuckDuckGo/Tab/ViewModel/TabViewModel.swift

@@ -66,7 +66,6 @@ final class TabViewModel {
     @Published private(set) var addressBarString: String = ""
     @Published private(set) var passiveAddressBarString: String = ""
     var lastAddressBarTextFieldValue: AddressBarTextField.Value?
-    private(set) var addressBarHasUpdated: Bool = false
 
     @Published private(set) var title: String = UserText.tabHomeTitle
     @Published private(set) var favicon: NSImage?
@@ -101,30 +100,25 @@ final class TabViewModel {
         tab.$loadingProgress
             .assign(to: \.progress, onWeaklyHeld: self)
             .store(in: &cancellables)
+        if case .url(_, credential: _, source: .pendingStateRestoration) = tab.content {
+            updateAddressBarStrings()
+        }
     }
 
     private func subscribeToUrl() {
         tab.$content
-            .receive(on: DispatchQueue.main)
-            .sink { [weak self] content in
-                self?.waitToUpdateAddressBar(content)
+            .map { [tab] _ in
+                // Update the address bar only after the tab did commit navigation
+                // to prevent Address Bar Spoofing
+                tab.webViewDidCommitNavigationPublisher
             }
-            .store(in: &cancellables)
-    }
+            .switchToLatest()
+            .sink { [weak self] _ in
+                guard let self else { return }
 
-    private func waitToUpdateAddressBar(_ content: Published<Tab.TabContent>.Publisher.Output) {
-        self.addressBarHasUpdated = false
-        tab.$loadingProgress
-            .receive(on: DispatchQueue.main)
-            .sink { [weak self] progress in
-                // Update the address bar only after the tab has reached 10% loading
-                // to prevent Address Bar Spoofing
-                if progress > 0.1 && !(self?.addressBarHasUpdated ?? true) {
-                    self?.updateAddressBarStrings()
-                    self?.updateCanBeBookmarked()
-                    self?.updateFavicon()
-                    self?.addressBarHasUpdated = true
-                }
+                updateAddressBarStrings()
+                updateCanBeBookmarked()
+                updateFavicon()
             }
             .store(in: &cancellables)
     }