Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AddressBar opt+return to download, cmd+opt to open new window #2262

Merged
merged 24 commits into from
Mar 1, 2024
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
03f9bb3
Fixed address bar cleared/losing focus on Navigation
mallexxx Feb 5, 2024
375cbb1
Merge remote-tracking branch 'origin/main' into alex/fix-address-bar-…
mallexxx Feb 7, 2024
c5cdd45
Merge remote-tracking branch 'origin/main' into alex/fix-address-bar-…
mallexxx Feb 7, 2024
7b699bf
fixed suggestion window not disappearing on tab switch
mallexxx Feb 7, 2024
3aa45ff
Merge remote-tracking branch 'origin/main' into alex/fix-address-bar-…
mallexxx Feb 19, 2024
8f37cd8
refactor first responder setting and fix address bar blinking issues
mallexxx Feb 19, 2024
7ebe607
fix tests
mallexxx Feb 19, 2024
543f5ba
Merge remote-tracking branch 'origin/main' into alex/fix-address-bar-…
mallexxx Feb 19, 2024
535f92e
Merge remote-tracking branch 'origin/main' into alex/fix-address-bar-…
mallexxx Feb 21, 2024
41ae4cf
fix issues, add tests
mallexxx Feb 22, 2024
eeb4e1c
Merge remote-tracking branch 'origin/main' into alex/fix-address-bar-…
mallexxx Feb 22, 2024
d0c5643
Merge remote-tracking branch 'origin/main' into alex/fix-address-bar-…
mallexxx Feb 22, 2024
808c5bd
fix comment; fixing tests
mallexxx Feb 22, 2024
3b634cb
Merge remote-tracking branch 'origin/main' into alex/fix-address-bar-…
mallexxx Feb 26, 2024
5684bb5
fix divider shown on the Home Page
mallexxx Feb 26, 2024
c0051dc
Download a URL on Option+Return
mallexxx Feb 26, 2024
f87062e
fix opening in new window; position inactive window below key window
mallexxx Feb 26, 2024
5054033
fix html downloads with wrong extension
mallexxx Feb 26, 2024
ced53ba
Merge remote-tracking branch 'origin/main' into alex/addressbar-opt-d…
mallexxx Feb 27, 2024
ed7d2c5
Merge remote-tracking branch 'origin/main' into alex/addressbar-opt-d…
mallexxx Feb 29, 2024
5df9a97
Merge branch 'main' into alex/addressbar-opt-download
mallexxx Mar 1, 2024
b82c902
Merge remote-tracking branch 'origin/main' into alex/addressbar-opt-d…
mallexxx Mar 1, 2024
e658f82
MainViewController: only handle Return key when the address bar is th…
mallexxx Mar 1, 2024
1024737
fix linter issue
mallexxx Mar 1, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,5 @@ extension NavigationAction {
extension CustomNavigationType {
static let userEnteredUrl = CustomNavigationType(rawValue: "userEnteredUrl")
static let tabContentUpdate = CustomNavigationType(rawValue: "tabContentUpdate")
static let userRequestedPageDownload = CustomNavigationType(rawValue: "userRequestedPageDownload")
}
2 changes: 1 addition & 1 deletion DuckDuckGo/FileDownload/Model/WebKitDownloadTask.swift
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ extension WebKitDownloadTask: WKDownloadDelegate {
// sometimes suggesteFilename has an extension appended to already present URL file extension
// e.g. feed.xml.rss for www.domain.com/rss.xml
if let urlSuggestedFilename = response.url?.suggestedFilename,
!urlSuggestedFilename.pathExtension.isEmpty,
!(urlSuggestedFilename.pathExtension.isEmpty || (self.suggestedFileType == .html && urlSuggestedFilename.pathExtension == "html")),
suggestedFilename.hasPrefix(urlSuggestedFilename) {
suggestedFilename = urlSuggestedFilename
}
Expand Down
9 changes: 9 additions & 0 deletions DuckDuckGo/MainWindow/MainViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -474,6 +474,7 @@ extension MainViewController {
}.store(in: &eventMonitorCancellables)
}

// swiftlint:disable:next cyclomatic_complexity
func customKeyDown(with event: NSEvent) -> Bool {
guard let locWindow = self.view.window,
NSApplication.shared.keyWindow === locWindow else { return false }
Expand All @@ -482,6 +483,14 @@ extension MainViewController {
.subtracting(.capsLock)

switch Int(event.keyCode) {
case kVK_Return:
if let addressBarTextField = navigationBarViewController.addressBarViewController?.addressBarTextField,
addressBarTextField.isFirstResponder {
addressBarTextField.addressBarEnterPressed()
return true
}
return false

case kVK_Escape:
var isHandled = false
if !mainView.findInPageContainerView.isHidden {
Expand Down
6 changes: 5 additions & 1 deletion DuckDuckGo/MainWindow/MainWindowController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,11 @@ final class MainWindowController: NSWindowController {
}

func orderWindowBack(_ sender: Any?) {
window?.orderBack(sender)
if let lastKeyWindow = WindowControllersManager.shared.lastKeyMainWindowController?.window {
window?.order(.below, relativeTo: lastKeyWindow.windowNumber)
} else {
window?.orderFront(sender)
}
register()
}

Expand Down
76 changes: 40 additions & 36 deletions DuckDuckGo/NavigationBar/View/AddressBarTextField.swift
Original file line number Diff line number Diff line change
Expand Up @@ -285,22 +285,22 @@ final class AddressBarTextField: NSTextField {
clearUndoManager()
}

private func addressBarEnterPressed() {
func addressBarEnterPressed() {
suggestionContainerViewModel?.clearUserStringValue()

let suggestion = suggestionContainerViewModel?.selectedSuggestionViewModel?.suggestion
if NSApp.isCommandPressed {
openNewTab(selected: NSApp.isShiftPressed, suggestion: suggestion)
} else {
navigate(suggestion: suggestion)
}
navigate(suggestion: suggestion)

hideSuggestionWindow()
}

private func navigate(suggestion: Suggestion?) {
hideSuggestionWindow()
updateTabUrl(suggestion: suggestion)
if NSApp.isCommandPressed {
openNew(NSApp.isOptionPressed ? .window : .tab, selected: NSApp.isShiftPressed, suggestion: suggestion)
} else {
hideSuggestionWindow()
updateTabUrl(suggestion: suggestion, downloadRequested: NSApp.isOptionPressed && !NSApp.isShiftPressed)
}

currentEditor()?.selectAll(self)
}
Expand All @@ -322,7 +322,7 @@ final class AddressBarTextField: NSTextField {
}
}

private func updateTabUrlWithUrl(_ providedUrl: URL, userEnteredValue: String, suggestion: Suggestion?) {
private func updateTabUrlWithUrl(_ providedUrl: URL, userEnteredValue: String, downloadRequested: Bool, suggestion: Suggestion?) {
guard let selectedTabViewModel = tabCollectionViewModel.selectedTabViewModel else {
os_log("%s: Selected tab view model is nil", type: .error, className)
return
Expand Down Expand Up @@ -353,44 +353,52 @@ final class AddressBarTextField: NSTextField {
#endif

self.window?.makeFirstResponder(nil)
selectedTabViewModel.tab.setUrl(providedUrl, source: .userEntered(userEnteredValue))
selectedTabViewModel.tab.setUrl(providedUrl, source: .userEntered(userEnteredValue, downloadRequested: downloadRequested))
if downloadRequested {
updateValue(selectedTabViewModel: nil, addressBarString: nil)
}
}

private func updateTabUrl(suggestion: Suggestion?) {
private func updateTabUrl(suggestion: Suggestion?, downloadRequested: Bool) {
makeUrl(suggestion: suggestion,
stringValueWithoutSuffix: stringValueWithoutSuffix,
completion: { [weak self] url, userEnteredValue, isUpgraded in
guard let url = url else { return }

if isUpgraded { self?.updateTabUpgradedToUrl(url) }
self?.updateTabUrlWithUrl(url, userEnteredValue: userEnteredValue, suggestion: suggestion)
if isUpgraded {
self?.updateTab(self?.tabCollectionViewModel.selectedTabViewModel?.tab, upgradedTo: url)
}
self?.updateTabUrlWithUrl(url, userEnteredValue: userEnteredValue, downloadRequested: downloadRequested, suggestion: suggestion)
})
}

private func updateTabUpgradedToUrl(_ url: URL?) {
if url == nil { return }
let tab = tabCollectionViewModel.selectedTabViewModel?.tab
tab?.setMainFrameConnectionUpgradedTo(url)
}

private func openNewTabWithUrl(_ providedUrl: URL?, userEnteredValue: String, selected: Bool, suggestion: Suggestion?) {
guard let url = providedUrl else {
os_log("%s: Making url from address bar string failed", type: .error, className)
return
}

let tab = Tab(content: .url(url, source: .userEntered(userEnteredValue)),
shouldLoadInBackground: true,
burnerMode: tabCollectionViewModel.burnerMode)
tabCollectionViewModel.append(tab: tab, selected: selected)
private func updateTab(_ tab: Tab?, upgradedTo url: URL?) {
guard let tab, let url else { return }
tab.setMainFrameConnectionUpgradedTo(url)
}

private func openNewTab(selected: Bool, suggestion: Suggestion?) {
enum TabOrWindow { case tab, window }
private func openNew(_ tabOrWindow: TabOrWindow, selected: Bool, suggestion: Suggestion?) {
makeUrl(suggestion: suggestion,
stringValueWithoutSuffix: stringValueWithoutSuffix) { [weak self] url, userEnteredValue, isUpgraded in
guard let self, let url else {
os_log("%s: Making url from address bar string failed", type: .error)
return
}
let tab = Tab(content: .url(url, source: .userEntered(userEnteredValue)),
shouldLoadInBackground: true,
burnerMode: tabCollectionViewModel.burnerMode)

if isUpgraded { self?.updateTabUpgradedToUrl(url) }
self?.openNewTabWithUrl(url, userEnteredValue: userEnteredValue, selected: selected, suggestion: suggestion)
if isUpgraded {
updateTab(tab, upgradedTo: url)
}

switch tabOrWindow {
case .tab:
tabCollectionViewModel.append(tab: tab, selected: selected)
case .window:
WindowsManager.openNewWindow(with: tab, showWindow: selected, popUp: false)
}
}
}

Expand Down Expand Up @@ -1120,10 +1128,6 @@ extension AddressBarTextField: SuggestionViewControllerDelegate {

func suggestionViewControllerDidConfirmSelection(_ suggestionViewController: SuggestionViewController) {
let suggestion = suggestionContainerViewModel?.selectedSuggestionViewModel?.suggestion
if NSApp.isCommandPressed {
openNewTab(selected: NSApp.isShiftPressed, suggestion: suggestion)
return
}
navigate(suggestion: suggestion)
}

Expand Down
16 changes: 13 additions & 3 deletions DuckDuckGo/Tab/Model/Tab.swift
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ protocol NewWindowPolicyDecisionMaker {
enum URLSource: Equatable {
case pendingStateRestoration
case loadedByStateRestoration
case userEntered(String)
case userEntered(String, downloadRequested: Bool = false)
case historyEntry
case bookmark
case ui
Expand All @@ -77,7 +77,7 @@ protocol NewWindowPolicyDecisionMaker {
case webViewUpdated

var userEnteredValue: String? {
if case .userEntered(let userEnteredValue) = self {
if case .userEntered(let userEnteredValue, _) = self {
userEnteredValue
} else {
nil
Expand All @@ -90,6 +90,8 @@ protocol NewWindowPolicyDecisionMaker {

var navigationType: NavigationType {
switch self {
case .userEntered(_, downloadRequested: true):
.custom(.userRequestedPageDownload)
case .userEntered:
.custom(.userEnteredUrl)
case .pendingStateRestoration:
Expand Down Expand Up @@ -259,6 +261,14 @@ protocol NewWindowPolicyDecisionMaker {
userEnteredValue != nil
}

var isUserRequestedPageDownload: Bool {
if case .url(_, credential: _, source: .userEntered(_, downloadRequested: true)) = self {
return true
} else {
return false
}
}

var displaysContentInWebView: Bool {
isUrl
}
Expand Down Expand Up @@ -1001,7 +1011,7 @@ protocol NewWindowPolicyDecisionMaker {
return nil
}

if webView.url == url, webView.backForwardList.currentItem?.url == url, !webView.isLoading {
if webView.url == url, webView.backForwardList.currentItem?.url == url, !webView.isLoading, !content.isUserRequestedPageDownload {
return reload()
}
if restoreInteractionStateDataIfNeeded() { return nil /* session restored */ }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ final class SearchNonexistentDomainNavigationResponder {
self.setContent = setContent

cancellable = contentPublisher.sink { [weak self] tabContent in
if case .url(_, credential: .none, source: .userEntered(let userEnteredValue)) = tabContent {
if case .url(_, credential: .none, source: .userEntered(let userEnteredValue, _)) = tabContent {
self?.lastUserEnteredValue = userEnteredValue
}
}
Expand Down
2 changes: 2 additions & 0 deletions DuckDuckGo/Tab/TabExtensions/DownloadsTabExtension.swift
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,8 @@ extension DownloadsTabExtension: NavigationResponder {
func decidePolicy(for navigationResponse: NavigationResponse) async -> NavigationResponsePolicy? {
guard navigationResponse.httpResponse?.isSuccessful == true,
!navigationResponse.canShowMIMEType || navigationResponse.shouldDownload
// if user pressed Opt+Enter in the Address bar to download from a URL
|| (navigationResponse.mainFrameNavigation?.redirectHistory.last ?? navigationResponse.mainFrameNavigation?.navigationAction)?.navigationType == .custom(.userRequestedPageDownload)
else {
return .next
}
Expand Down
4 changes: 4 additions & 0 deletions DuckDuckGo/Tab/ViewModel/TabViewModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,10 @@ final class TabViewModel {
// Update the address bar only after the tab did commit navigation to prevent Address Bar Spoofing
return tab.webViewDidCommitNavigationPublisher.map { .didCommit }.eraseToAnyPublisher()

case .url(_, _, source: .userEntered(_, downloadRequested: true)):
// don‘t update the address bar for download navigations
return Empty().eraseToAnyPublisher().eraseToAnyPublisher()

case .url(_, _, source: .pendingStateRestoration),
.url(_, _, source: .loadedByStateRestoration),
.url(_, _, source: .userEntered),
Expand Down
Loading