Skip to content

Commit

Permalink
Reload address bar deactivation (#2370)
Browse files Browse the repository at this point in the history 
Task/Issue URL: https://app.asana.com/0/1177771139624306/1206795665613520/f
  • Loading branch information
@mallexxx
mallexxx authored Mar 11, 2024
1 parent 86823fc commit 20ff0d3
Show file tree
Hide file tree
Showing 14 changed files with 106 additions and 92 deletions.
4 changes: 0 additions & 4 deletions DuckDuckGo.xcodeproj/project.pbxproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -916,7 +916,6 @@
3707C72A294B5D2900682A9F /* URLExtension.swift in Sources */ = {isa = PBXBuildFile; fileRef = AA8EDF2324923E980071C2E8 /* URLExtension.swift */; };
3707C72D294B5D4100682A9F /* EmptyAttributionRulesProver.swift in Sources */ = {isa = PBXBuildFile; fileRef = B6AE39F029373AF200C37AA4 /* EmptyAttributionRulesProver.swift */; };
3707C72F294B5D4F00682A9F /* WebViewTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3783F92229432E1800BCA897 /* WebViewTests.swift */; };
3707C730294B5D5C00682A9F /* DuckPlayerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3714B1E828EDBAAB0056C57A /* DuckPlayerTests.swift */; };
370A34B12AB24E3700C77F7C /* SyncDebugMenu.swift in Sources */ = {isa = PBXBuildFile; fileRef = 370A34B02AB24E3700C77F7C /* SyncDebugMenu.swift */; };
370A34B22AB24E3700C77F7C /* SyncDebugMenu.swift in Sources */ = {isa = PBXBuildFile; fileRef = 370A34B02AB24E3700C77F7C /* SyncDebugMenu.swift */; };
3714B1E728EDB7FA0056C57A /* DuckPlayerPreferencesTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3714B1E628EDB7FA0056C57A /* DuckPlayerPreferencesTests.swift */; };
Expand Down Expand Up @@ -2931,7 +2930,6 @@
B6AE39F129373AF200C37AA4 /* EmptyAttributionRulesProver.swift in Sources */ = {isa = PBXBuildFile; fileRef = B6AE39F029373AF200C37AA4 /* EmptyAttributionRulesProver.swift */; };
B6AE39F329374AEC00C37AA4 /* OHHTTPStubs in Frameworks */ = {isa = PBXBuildFile; productRef = B6AE39F229374AEC00C37AA4 /* OHHTTPStubs */; };
B6AE39F529374AEC00C37AA4 /* OHHTTPStubsSwift in Frameworks */ = {isa = PBXBuildFile; productRef = B6AE39F429374AEC00C37AA4 /* OHHTTPStubsSwift */; };
B6AE39F729374B9900C37AA4 /* DuckPlayerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3714B1E828EDBAAB0056C57A /* DuckPlayerTests.swift */; };
B6AE74342609AFCE005B9B1A /* ProgressEstimationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = B6AE74332609AFCE005B9B1A /* ProgressEstimationTests.swift */; };
B6AFE6BC29A5D3F8002FF962 /* PrivacyDashboardTabExtension.swift in Sources */ = {isa = PBXBuildFile; fileRef = B6BF5D842946FFDA006742B1 /* PrivacyDashboardTabExtension.swift */; };
B6AFE6BD29A5D621002FF962 /* HTTPSUpgradeTabExtension.swift in Sources */ = {isa = PBXBuildFile; fileRef = B6BF5D8829470BC4006742B1 /* HTTPSUpgradeTabExtension.swift */; };
Expand Down Expand Up @@ -10756,7 +10754,6 @@
3706FEA0293F662100E42796 /* EncryptionMocks.swift in Sources */,
B62A233D29C322BC00D22475 /* NavigationProtectionIntegrationTests.swift in Sources */,
3706FEA1293F662100E42796 /* TestNavigationDelegate.swift in Sources */,
3707C730294B5D5C00682A9F /* DuckPlayerTests.swift in Sources */,
B6EC37DF29B5D05A001ACE79 /* DownloadsIntegrationTests.swift in Sources */,
B6EC37FD29B83E99001ACE79 /* TestsURLExtension.swift in Sources */,
B603973529BEF86200902A34 /* HTTPSUpgradeIntegrationTests.swift in Sources */,
Expand Down Expand Up @@ -10796,7 +10793,6 @@
files = (
B603971129B9D67E00902A34 /* PublishersExtensions.swift in Sources */,
B662D3DF275616FF0035D4D6 /* EncryptionKeyStoreMock.swift in Sources */,
B6AE39F729374B9900C37AA4 /* DuckPlayerTests.swift in Sources */,
B62A233C29C322BC00D22475 /* NavigationProtectionIntegrationTests.swift in Sources */,
B6EC37DE29B5D05A001ACE79 /* DownloadsIntegrationTests.swift in Sources */,
4B1AD8E225FC390B00261379 /* EncryptionMocks.swift in Sources */,
Expand Down
12 changes: 0 additions & 12 deletions DuckDuckGo/Assets.xcassets/Images/Tracker Icons/Xaxis.imageset/Contents.json
View file

This file was deleted.

4 changes: 0 additions & 4 deletions DuckDuckGo/Assets.xcassets/Images/Tracker Icons/Xaxis.imageset/Xaxis.svg
View file

This file was deleted.

10 changes: 7 additions & 3 deletions DuckDuckGo/DataImport/Bookmarks/Firefox/FirefoxBookmarksReader.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,13 @@ final class FirefoxBookmarksReader {

private let firefoxPlacesDatabaseURL: URL
private var currentOperationType: ImportError.OperationType = .copyTemporaryFile
private let otherBookmarksFolderTitle: String
private let mobileBookmarksFolderTitle: String

init(firefoxDataDirectoryURL: URL) {
init(firefoxDataDirectoryURL: URL, otherBookmarksFolderTitle: String = UserText.otherBookmarksImportedFolderTitle, mobileBookmarksFolderTitle: String = UserText.mobileBookmarksImportedFolderTitle) {
self.firefoxPlacesDatabaseURL = firefoxDataDirectoryURL.appendingPathComponent(Constants.placesDatabaseName)
self.otherBookmarksFolderTitle = otherBookmarksFolderTitle
self.mobileBookmarksFolderTitle = mobileBookmarksFolderTitle
}

func readBookmarks() -> DataImportResult<ImportedBookmarks> {
Expand Down Expand Up @@ -181,8 +185,8 @@ final class FirefoxBookmarksReader {
urlString: nil,
children: toolbarBookmarksAndFolders + menuBookmarksAndFolders)

let unfiledFolder = ImportedBookmarks.BookmarkOrFolder(name: UserText.otherBookmarksImportedFolderTitle, type: .folder, urlString: nil, children: unfiledBookmarksAndFolders)
let syncedFolder = ImportedBookmarks.BookmarkOrFolder(name: UserText.mobileBookmarksImportedFolderTitle, type: .folder, urlString: nil, children: syncedBookmarksAndFolders)
let unfiledFolder = ImportedBookmarks.BookmarkOrFolder(name: self.otherBookmarksFolderTitle, type: .folder, urlString: nil, children: unfiledBookmarksAndFolders)
let syncedFolder = ImportedBookmarks.BookmarkOrFolder(name: self.mobileBookmarksFolderTitle, type: .folder, urlString: nil, children: syncedBookmarksAndFolders)
let folders = ImportedBookmarks.TopLevelFolders(bookmarkBar: toolbarFolder, otherBookmarks: unfiledFolder, syncedBookmarks: syncedFolder)

return ImportedBookmarks(topLevelFolders: folders)
Expand Down
6 changes: 4 additions & 2 deletions DuckDuckGo/DataImport/Bookmarks/HTML/BookmarkHTMLReader.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,11 @@ final class BookmarkHTMLReader {
}

private var currentOperationType: ImportError.OperationType = .parseXml
private let otherBookmarksFolderTitle: String

init(bookmarksFileURL: URL) {
init(bookmarksFileURL: URL, otherBookmarksFolderTitle: String = UserText.otherBookmarksImportedFolderTitle) {
self.bookmarksFileURL = bookmarksFileURL
self.otherBookmarksFolderTitle = otherBookmarksFolderTitle
}

func readBookmarks() -> DataImportResult<HTMLImportedBookmarks> {
Expand Down Expand Up @@ -114,7 +116,7 @@ final class BookmarkHTMLReader {
bookmarkBar = firstFolder
}

let otherBookmarks = ImportedBookmarks.BookmarkOrFolder.folder(name: UserText.otherBookmarksImportedFolderTitle, children: other)
let otherBookmarks = ImportedBookmarks.BookmarkOrFolder.folder(name: otherBookmarksFolderTitle, children: other)
let allBookmarks = ImportedBookmarks(topLevelFolders: .init(bookmarkBar: bookmarkBar, otherBookmarks: otherBookmarks, syncedBookmarks: nil))
let result = HTMLImportedBookmarks(source: importSource, bookmarks: allBookmarks)

Expand Down
6 changes: 4 additions & 2 deletions DuckDuckGo/DataImport/Bookmarks/Safari/SafariBookmarksReader.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,11 @@ final class SafariBookmarksReader {

private let safariBookmarksFileURL: URL
private var currentOperationType: ImportError.OperationType = .readPlist
private let otherBookmarksFolderTitle: String

init(safariBookmarksFileURL: URL) {
init(safariBookmarksFileURL: URL, otherBookmarksFolderTitle: String = UserText.otherBookmarksImportedFolderTitle) {
self.safariBookmarksFileURL = safariBookmarksFileURL
self.otherBookmarksFolderTitle = otherBookmarksFolderTitle
}

func readBookmarks() -> DataImportResult<ImportedBookmarks> {
Expand Down Expand Up @@ -118,7 +120,7 @@ final class SafariBookmarksReader {

}

let otherBookmarksFolder = ImportedBookmarks.BookmarkOrFolder(name: UserText.otherBookmarksImportedFolderTitle, type: .folder, urlString: nil, children: otherBookmarks)
let otherBookmarksFolder = ImportedBookmarks.BookmarkOrFolder(name: self.otherBookmarksFolderTitle, type: .folder, urlString: nil, children: otherBookmarks)
let emptyFolder = ImportedBookmarks.BookmarkOrFolder(name: "bar", type: .folder, urlString: nil, children: [])

let topLevelFolder = ImportedBookmarks.TopLevelFolders(bookmarkBar: bookmarksBar ?? emptyFolder, otherBookmarks: otherBookmarksFolder, syncedBookmarks: nil)
Expand Down
112 changes: 60 additions & 52 deletions DuckDuckGo/Tab/Model/Tab.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,16 @@ protocol NewWindowPolicyDecisionMaker {
}
}

var source: URLSource {
switch self {
case .url(_, _, source: let source):
return source
case .newtab, .settings, .bookmarks, .onboarding, .dataBrokerProtection,
.subscription, .identityTheftRestoration, .none:
return .ui
}
}

var isUrl: Bool {
switch self {
case .url, .subscription, .identityTheftRestoration:
Expand Down Expand Up @@ -681,7 +691,7 @@ protocol NewWindowPolicyDecisionMaker {
self.title = title
}

return reloadIfNeeded(shouldLoadInBackground: true)
return reloadIfNeeded(source: .contentUpdated)
}

@discardableResult
Expand Down Expand Up @@ -987,10 +997,10 @@ protocol NewWindowPolicyDecisionMaker {
return nil
}

self.content = content.forceReload()
if webView.url == nil, content.isUrl {
self.content = content.forceReload()
// load from cache or interactionStateData when called by lazy loader
return reloadIfNeeded(shouldLoadInBackground: true)
return reloadIfNeeded(source: .lazyLoad)
} else {
return webView.navigator(distributedNavigationDelegate: navigationDelegate).reload(withExpectedNavigationType: .reload)
}
Expand All @@ -1004,30 +1014,17 @@ protocol NewWindowPolicyDecisionMaker {
audioState = webView.audioState()
}

private func tabContentReloadInfo(for content: TabContent, shouldLoadInBackground: Bool) -> (url: URL, source: TabContent.URLSource, forceReload: Bool)? {
switch content {
case .url(let url, _, source: let source):
let forceReload = url.absoluteString == source.userEnteredValue ? shouldLoadInBackground : (source == .reload)
return (url, source, forceReload: forceReload)

case .subscription(let url), .identityTheftRestoration(let url):
return (url, .ui, forceReload: false)

case .newtab, .bookmarks, .onboarding, .dataBrokerProtection, .settings:
guard let contentUrl = content.urlForWebView, webView.url != contentUrl else { return nil }

return (contentUrl, .ui, forceReload: true) // always navigate built-in ui (duck://) urls

case .none:
return nil
}
private enum ReloadIfNeededSource {
case contentUpdated
case webViewDisplayed
case loadInBackgroundIfNeeded(shouldLoadInBackground: Bool)
case lazyLoad
}

@MainActor(unsafe)
@discardableResult
private func reloadIfNeeded(shouldLoadInBackground: Bool = false) -> ExpectedNavigation? {
guard let (url, source, forceReload) = tabContentReloadInfo(for: content, shouldLoadInBackground: shouldLoadInBackground),
forceReload || shouldReload(url, shouldLoadInBackground: shouldLoadInBackground) else { return nil }
private func reloadIfNeeded(source reloadIfNeededSource: ReloadIfNeededSource) -> ExpectedNavigation? {
guard let url = content.urlForWebView,
shouldReload(url, source: reloadIfNeededSource) else { return nil }

if case .settings = content, case .settings = webView.url.flatMap({ TabContent.contentFromURL($0, source: .ui) }) {
// replace WebView URL without adding a new history item if switching settings panes
Expand All @@ -1041,6 +1038,7 @@ protocol NewWindowPolicyDecisionMaker {
if restoreInteractionStateIfNeeded() { return nil /* session restored */ }
invalidateInteractionStateData()

let source = content.source
if url.isFileURL {
return webView.navigator(distributedNavigationDelegate: navigationDelegate)
.loadFileURL(url, allowingReadAccessTo: URL(fileURLWithPath: "/"), withExpectedNavigationType: source.navigationType)
Expand All @@ -1056,23 +1054,43 @@ protocol NewWindowPolicyDecisionMaker {
}

@MainActor
private func shouldReload(_ url: URL, shouldLoadInBackground: Bool) -> Bool {
// don‘t reload in background unless shouldLoadInBackground
guard url.isValid,
webView.superview != nil || shouldLoadInBackground,
// don‘t reload when already loaded
webView.url != url || error != nil else { return false }

// if content not loaded inspect error
switch error {
case .none, // no error
// error due to connection failure
.some(URLError.notConnectedToInternet),
.some(URLError.networkConnectionLost):
private func shouldReload(_ url: URL, source: ReloadIfNeededSource) -> Bool {
guard url.isValid else { return false }

switch source {
// should load when Web View is displayed?
case .webViewDisplayed:
// yes if not loaded yet
if webView.url == nil {
return true
}

switch error {
case .some(URLError.notConnectedToInternet),
.some(URLError.networkConnectionLost):
// reload when showing error due to connection failure
return true
default:
// don‘t autoreload on other kinds of errors
return false
}

// should load on Web View instantiation?
case .loadInBackgroundIfNeeded(shouldLoadInBackground: let shouldLoadInBackground):
switch content {
case .newtab, .bookmarks, .settings:
return webView.url == nil // navigate to empty pages loaded for duck:// urls
default:
return shouldLoadInBackground
}

// lazy loading triggered
case .lazyLoad:
return webView.url == nil

// `.setContent()` called - always load
case .contentUpdated:
return true
case .some:
// don‘t autoreload on other kinds of errors
return false
}
}

Expand Down Expand Up @@ -1107,13 +1125,6 @@ protocol NewWindowPolicyDecisionMaker {
webView.interactionState = interactionStateData
}

private func addHomePageToWebViewIfNeeded() {
guard NSApp.runType.requiresEnvironment else { return }
if content == .newtab && webView.url == nil {
webView.load(URLRequest(url: .newtab))
}
}

func stopLoading() {
webView.stopLoading()
}
Expand Down Expand Up @@ -1147,7 +1158,7 @@ protocol NewWindowPolicyDecisionMaker {
webView.observe(\.superview, options: .old) { [weak self] _, change in
// if the webView is being added to superview - reload if needed
if case .some(.none) = change.oldValue {
self?.reloadIfNeeded()
self?.reloadIfNeeded(source: .webViewDisplayed)
}
}.store(in: &webViewCancellables)

Expand Down Expand Up @@ -1190,10 +1201,7 @@ protocol NewWindowPolicyDecisionMaker {

// background tab loading should start immediately
DispatchQueue.main.async {
self.reloadIfNeeded(shouldLoadInBackground: shouldLoadInBackground)
if !shouldLoadInBackground {
self.addHomePageToWebViewIfNeeded()
}
self.reloadIfNeeded(source: .loadInBackgroundIfNeeded(shouldLoadInBackground: shouldLoadInBackground))
}
}

Expand Down
1 change: 0 additions & 1 deletion IntegrationTests/Tab/AddressBarTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,6 @@ class AddressBarTests: XCTestCase {
}

func testWhenTabReloaded_addressBarIsDeactivated() async throws {
throw XCTSkip("https://app.asana.com/0/0/1206791095409241/1206794776988117/f")
let tab = Tab(content: .url(.duckDuckGo, credential: nil, source: .webViewUpdated), webViewConfiguration: webViewConfiguration, privacyFeatures: privacyFeaturesMock)
let viewModel = TabCollectionViewModel(tabCollection: TabCollection(tabs: [tab]))
window = WindowsManager.openNewWindow(with: viewModel)!
Expand Down
2 changes: 1 addition & 1 deletion UnitTests/Bookmarks/Services/LocalBookmarkStoreTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1068,7 +1068,7 @@ final class LocalBookmarkStoreTests: XCTestCase {
case .success(let entities):
XCTAssert(entities.contains(where: { $0.title == "DuckDuckGo" }))
XCTAssert(entities.contains(where: { $0.title == "Folder" }))
XCTAssert(entities.contains(where: { $0.title.contains("Imported from") }))
XCTAssert(entities.contains(where: { $0.title.contains(source.importSourceName) }))
case .failure:
XCTFail("Did not expect failure when checking topLevelEntitiesResult")
}
Expand Down
2 changes: 1 addition & 1 deletion UnitTests/DataImport/BookmarksHTMLReaderTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ final class BookmarksHTMLReaderTests: XCTestCase {
for fileName in try FileManager.default.contentsOfDirectory(atPath: bookmarksHTMLReaderTestFilesURL.path) {
let fileNameWithoutExtension = fileName.dropping(suffix: "html")
let fileURL = bookmarksHTMLReaderTestFilesURL.appendingPathComponent(fileName)
let reader = BookmarkHTMLReader(bookmarksFileURL: fileURL)
let reader = BookmarkHTMLReader(bookmarksFileURL: fileURL, otherBookmarksFolderTitle: "Other bookmarks")
let result = reader.readBookmarks()

if expectedToThrow.contains(fileName) {
Expand Down
6 changes: 3 additions & 3 deletions UnitTests/JSAlert/JSAlertViewModelTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,9 @@ final class JSAlertViewModelTests: XCTestCase {

func testTitleText() {
let params: [JSAlertQuery.TestParameters] = [
.init(type: .testAlert(parameters: .testData(domain: "duckduckgo.com")), result: "A message from duckduckgo.com"),
.init(type: .testConfirm(parameters: .testData(domain: "wikipedia.com")), result: "A message from wikipedia.com"),
.init(type: .testTextInput(parameters: .testData(domain: "example.com")), result: "A message from example.com")
.init(type: .testAlert(parameters: .testData(domain: "duckduckgo.com")), result: UserText.alertTitle(from: "duckduckgo.com")),
.init(type: .testConfirm(parameters: .testData(domain: "wikipedia.com")), result: UserText.alertTitle(from: "wikipedia.com")),
.init(type: .testTextInput(parameters: .testData(domain: "example.com")), result: UserText.alertTitle(from: "example.com"))
]

for param in params {
Expand Down
Loading

0 comments on commit 20ff0d3

Please sign in to comment.