Skip to content
This repository was archived by the owner on Feb 24, 2025. It is now read-only.
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
16 changes: 16 additions & 0 deletions DuckDuckGo/Common/Extensions/StringExtension.swift
Original file line number Diff line number Diff line change
Expand Up @@ -92,4 +92,20 @@ extension String {
return hasPrefix(other) || other.hasPrefix(self)
}

// MARK: - Search with bang

private static let searchWithBangPattern = "^.+\\?q=%21[a-zA-Z]+(?:%20[a-zA-Z]+)*$"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this needs some comment describing what the regex does as it‘s non-trivial


private static var compiledSearchWithBangRegex: NSRegularExpression? = {
if let newRegex = try? NSRegularExpression(pattern: searchWithBangPattern, options: .caseInsensitive) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check BSK.Common.StringExtension -> RegEx for regex instantiation helper (to be updated with compile-time checked macro with xc15)

return newRegex
}
return nil
}()

var isSearchWithBang: Bool {
guard let regex = Self.compiledSearchWithBangRegex else { return false }
return self.url?.isDuckDuckGoSearch ?? false && regex.firstMatch(in: self, options: [], range: NSRange(location: 0, length: self.utf16.count)) != nil
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,13 @@ extension NavigationButtonMenuDelegate: NSMenuDelegate {
}
}

// Remove duckduckgo search with bang
for (index, item) in list.enumerated().reversed() {
if let url = item.url, url.absoluteString.isSearchWithBang {
list.remove(at: index)
}
}

return (list, currentIndex)
}

Expand Down
19 changes: 18 additions & 1 deletion DuckDuckGo/Tab/Model/Tab.swift
Original file line number Diff line number Diff line change
Expand Up @@ -670,7 +670,8 @@ protocol NewWindowPolicyDecisionMaker {
return
}

let canGoBack = webView.canGoBack || self.error != nil
let isJustOneRidirectGoingBack = webView.backForwardList.backList.count == 1 && webView.backForwardList.backItem?.url.absoluteString.contains("%21") ?? false
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems error-prone since it‘s checking for "%21" for every url

let canGoBack = (webView.canGoBack && !isJustOneRidirectGoingBack) || self.error != nil
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the meaning of the check is non-straightforward and would be good if it had some comment on its purpose

let canGoForward = webView.canGoForward && self.error == nil
let canReload = (self.content.urlForWebView?.scheme ?? URL.NavigationalScheme.about.rawValue) != URL.NavigationalScheme.about.rawValue

Expand Down Expand Up @@ -700,13 +701,29 @@ protocol NewWindowPolicyDecisionMaker {
}

userInteractionDialog = nil

// If search with bang go back of 2 spaces (to avoid redirect)
if let urlString = webView.backForwardList.backItem?.url.absoluteString, urlString.isSearchWithBang {
if let item = webView.backForwardList.item(at: -2) {
return webView.navigator()?.go(to: item)
}
}
Comment on lines +706 to +710
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this check doesn‘t belong here as we were struggling to reduce navigation policy decision logics and move it away to TabExtensions


return webView.navigator()?.goBack(withExpectedNavigationType: .backForward(distance: -1))
}

@MainActor
@discardableResult
func goForward() -> ExpectedNavigation? {
guard canGoForward else { return nil }

// If search with bang go forward of 2 spaces (to avoid redirect)
if let urlString = webView.backForwardList.forwardItem?.url.absoluteString, urlString.isSearchWithBang {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these won‘t work for gesture (2-finger swipe) back/forw navigations

if let item = webView.backForwardList.item(at: +2) {
return webView.navigator()?.go(to: item)
}
}

return webView.navigator()?.goForward(withExpectedNavigationType: .backForward(distance: 1))
}

Expand Down
46 changes: 46 additions & 0 deletions UnitTests/Tab/TabStringExtensionTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
//
// TabUrlExtensionTests.swift
//
// Copyright © 2023 DuckDuckGo. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//

import XCTest
@testable import DuckDuckGo_Privacy_Browser

final class TabStringExtensionTests: XCTestCase {

func testSearchWithBangDetection() {
let searchWithBang: [String] = [
"https://duckduckgo.com/?q=%21Hello",
"https://duckduckgo.com/?q=%21Hello%20World",
"https://duckduckgo.com/?q=%21Search%20With%20Bang"
]

let nonSearchWithBang: [String] = [
"https://duckduckgo.com/?q=%21",
"https://duckduckgo.com/?q=%21%20",
"https://duckduckgo.com/?q=%21%20test",
"https://duckduckgo.com/?q=test%21test",
]

for url in searchWithBang {
XCTAssertTrue(url.isSearchWithBang, "\(url) should be detected as a search with bang")
}

for url in nonSearchWithBang {
XCTAssertFalse(url.isSearchWithBang, "\(url) should not be detected as search with bang")
}
}
}