From 201bc03685e97adf4e659ccb3d4c32f69a043a7e Mon Sep 17 00:00:00 2001 From: Juan Manuel Pereira Date: Thu, 21 Nov 2024 15:17:38 -0300 Subject: [PATCH 1/2] Show error message inside downloads popup when download fails because OS issue (#3576) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Task/Issue URL: https://app.asana.com/0/1204006570077678/1208780258680374/f Tech Design URL: CC: **Description**: **AC1** Given a user on macOS 14.7.1 When the user fails to download a fail with error code 256 and type NSFileReadUnknownError Then we show an error banner with some guidance on how to update **AC2** Given a user on macOS 15.0.1 When the user fails to download a fail with error code 256 and type NSFileReadUnknownError Then we show an error banner with some guidance on how to update **Notes:** - For the App Store version we should show a `How To Update` button that will take the user to this page: https://support.apple.com/guide/mac-help/get-macos-updates-and-apps-mh35618/mac For Sparkle users: We will show a `Open Settings` button that it will take them to System Settings → Software Update ![non-sandboxed](https://github.com/user-attachments/assets/0bccc3a7-0d5b-4129-8c73-b6862718ccb0) ![sandboxed](https://github.com/user-attachments/assets/acddc4ad-8966-4068-baf4-1fb73240b50c) **Steps to test this PR**: To test this, you first need to mimic the error. To force this, go to `WebKitDownloadTask`, and after line 330, throw the following exception: ```swift throw NSError.init(domain: NSCocoaErrorDomain, code: NSFileReadUnknownError) ``` Then, if you are not on macOS 14.7.1 or 15.0.1, you need to add your current OS to the `isSpecificMacOSVersion` function in the `DownloadListViewModel`. If you want to test Sandbox vs Non-sandbox, you can change how the view is initialized in the `DownloadsViewController` line 140. **Definition of Done**: * [x] Does this PR satisfy our [Definition of Done](https://app.asana.com/0/1202500774821704/1207634633537039/f)? --- ###### Internal references: [Pull Request Review Checklist](https://app.asana.com/0/1202500774821704/1203764234894239/f) [Software Engineering Expectations](https://app.asana.com/0/59792373528535/199064865822552) [Technical Design Template](https://app.asana.com/0/59792373528535/184709971311943) [Pull Request Documentation](https://app.asana.com/0/1202500774821704/1204012835277482/f) --- DuckDuckGo/Common/Localizables/UserText.swift | 3 + .../Model/DownloadListViewModel.swift | 37 +++++- .../Model/FileDownloadError.swift | 12 ++ .../View/DownloadsViewController.swift | 114 ++++++++++++++++-- DuckDuckGo/Localizable.xcstrings | 38 +++++- 5 files changed, 191 insertions(+), 13 deletions(-) diff --git a/DuckDuckGo/Common/Localizables/UserText.swift b/DuckDuckGo/Common/Localizables/UserText.swift index e2683d67c6..14fa14eb82 100644 --- a/DuckDuckGo/Common/Localizables/UserText.swift +++ b/DuckDuckGo/Common/Localizables/UserText.swift @@ -825,6 +825,9 @@ struct UserText { static let downloadFailed = NSLocalizedString("downloads.error.other", value: "Error", comment: "Short error description when Download failed") static let downloadBytesLoadedFormat = NSLocalizedString("downloads.bytes.format", value: "%@ of %@", comment: "Number of bytes out of total bytes downloaded (1Mb of 2Mb)") static let downloadSpeedFormat = NSLocalizedString("downloads.speed.format", value: "%@/s", comment: "Download speed format (1Mb/sec)") + static let downloadsErrorMessage = NSLocalizedString("downloads.error.message.for.specific.os", value: "The download failed because of a known issue on macOS 14.7.1 and 15.0.1. Update to macOS 15.1 and try downloading again.", comment: "This error message will appear in an error banner when users cannot download files on macOS 14.7.1 or 15.0.1") + static let downloadsErrorSandboxCallToAction = NSLocalizedString("downloads.error.cta.sandbox", value: "How To Update", comment: "Call to action for the OS specific downloads issue") + static let downloadsErrorNonSandboxCallToAction = NSLocalizedString("downloads.error.cta.non-sandbox", value: "Open Settings", comment: "Call to action for the OS specific downloads issue") static let cancelDownloadToolTip = NSLocalizedString("downloads.tooltip.cancel", value: "Cancel Download", comment: "Mouse-over tooltip for Cancel Download button") static let restartDownloadToolTip = NSLocalizedString("downloads.tooltip.restart", value: "Restart Download", comment: "Mouse-over tooltip for Restart Download button") diff --git a/DuckDuckGo/FileDownload/Model/DownloadListViewModel.swift b/DuckDuckGo/FileDownload/Model/DownloadListViewModel.swift index fd77921622..79ae958060 100644 --- a/DuckDuckGo/FileDownload/Model/DownloadListViewModel.swift +++ b/DuckDuckGo/FileDownload/Model/DownloadListViewModel.swift @@ -27,9 +27,10 @@ final class DownloadListViewModel { private let fireWindowSession: FireWindowSessionRef? private let coordinator: DownloadListCoordinator private var viewModels: [UUID: DownloadViewModel] - private var cancellable: AnyCancellable? + private var cancellables = Set() @Published private(set) var items: [DownloadViewModel] + @Published private(set) var shouldShowErrorBanner: Bool = false init(fireWindowSession: FireWindowSessionRef?, coordinator: DownloadListCoordinator = DownloadListCoordinator.shared) { self.fireWindowSession = fireWindowSession @@ -40,9 +41,10 @@ final class DownloadListViewModel { .map(DownloadViewModel.init) self.items = items self.viewModels = items.reduce(into: [:]) { $0[$1.id] = $1 } - cancellable = coordinator.updates.receive(on: DispatchQueue.main).sink { [weak self] update in + coordinator.updates.receive(on: DispatchQueue.main).sink { [weak self] update in self?.handleDownloadsUpdate(of: update.kind, item: update.item) - } + }.store(in: &cancellables) + self.setupErrorBannerBinding() } private func handleDownloadsUpdate(of kind: DownloadListCoordinator.UpdateKind, item: DownloadListItem) { @@ -65,6 +67,35 @@ final class DownloadListViewModel { } } + private func setupErrorBannerBinding() { + $items.flatMap { items in + Publishers.MergeMany(items.map { $0.$state }) + }.map { state in + if case .failed(let error) = state { + if error.isNSFileReadUnknownError && self.isAffectedMacOSVersion() { + return true + } + } + + return false + } + .removeDuplicates() + .receive(on: DispatchQueue.main) + .sink { [weak self] showError in + self?.shouldShowErrorBanner = showError + } + .store(in: &cancellables) + } + + /// macOS 15.0.1 and 14.7.1 have a bug that affects downloads. Apple fixed the issue on macOS 15.1 + /// For more information: https://app.asana.com/0/1204006570077678/1208522448255790/f + private func isAffectedMacOSVersion() -> Bool { + let currentVersion = AppVersion.shared.osVersion + let targetVersions = ["15.0.1", "14.7.1"] + + return targetVersions.contains(currentVersion) + } + func cleanupInactiveDownloads() { coordinator.cleanupInactiveDownloads(for: fireWindowSession) } diff --git a/DuckDuckGo/FileDownload/Model/FileDownloadError.swift b/DuckDuckGo/FileDownload/Model/FileDownloadError.swift index 50146e7ef6..55467874d8 100644 --- a/DuckDuckGo/FileDownload/Model/FileDownloadError.swift +++ b/DuckDuckGo/FileDownload/Model/FileDownloadError.swift @@ -21,6 +21,18 @@ import Foundation enum FileDownloadError: Error { case failedToMoveFileToDownloads case failedToCompleteDownloadTask(underlyingError: Error?, resumeData: Data?, isRetryable: Bool) + + var isNSFileReadUnknownError: Bool { + switch self { + case .failedToMoveFileToDownloads: + return false + case .failedToCompleteDownloadTask(let underlyingError, _, _): + guard let underlyingError else { return false } + + let nsError = underlyingError as NSError + return nsError.domain == NSCocoaErrorDomain && nsError.code == NSFileReadUnknownError + } + } } extension FileDownloadError: LocalizedError { diff --git a/DuckDuckGo/FileDownload/View/DownloadsViewController.swift b/DuckDuckGo/FileDownload/View/DownloadsViewController.swift index 89ba57e1e7..44c3513c20 100644 --- a/DuckDuckGo/FileDownload/View/DownloadsViewController.swift +++ b/DuckDuckGo/FileDownload/View/DownloadsViewController.swift @@ -18,6 +18,7 @@ import Cocoa import Combine +import SwiftUI protocol DownloadsViewControllerDelegate: AnyObject { func clearDownloadsActionTriggered() @@ -34,13 +35,18 @@ final class DownloadsViewController: NSViewController { private lazy var scrollView = NSScrollView() private lazy var tableView = NSTableView() + private var hostingViewConstraints: [NSLayoutConstraint] = [] private var tableViewHeightConstraint: NSLayoutConstraint! + private var errorBannerTopAnchorConstraint: NSLayoutConstraint! private var cellIndexToUnselect: Int? weak var delegate: DownloadsViewControllerDelegate? + private let separator = NSBox() private let viewModel: DownloadListViewModel private var downloadsCancellable: AnyCancellable? + private var errorBannerCancellable: AnyCancellable? + private var errorBannerHostingView: NSHostingView? init(viewModel: DownloadListViewModel) { self.viewModel = viewModel @@ -127,16 +133,31 @@ final class DownloadsViewController: NSViewController { scrollView.contentView = clipView - let separator = NSBox() separator.boxType = .separator separator.translatesAutoresizingMaskIntoConstraints = false view.addSubview(separator) - setupLayout(separator: separator) + let swiftUIView = DownloadsErrorBannerView(dismiss: { self.dismiss() }, + errorType: NSApp.isSandboxed ? .openHelpURL : .openSystemSettings) + let hostingView = NSHostingView(rootView: swiftUIView) + hostingView.translatesAutoresizingMaskIntoConstraints = false + hostingView.isHidden = true + view.addSubview(hostingView) + errorBannerHostingView = hostingView + + setupLayout(separator: separator, hostingView: hostingView) } - private func setupLayout(separator: NSBox) { + private func setupLayout(separator: NSBox, hostingView: NSHostingView) { tableViewHeightConstraint = scrollView.heightAnchor.constraint(equalToConstant: 440) + errorBannerTopAnchorConstraint = scrollView.topAnchor.constraint(equalTo: separator.bottomAnchor, constant: 12) + + hostingViewConstraints = [ + hostingView.leadingAnchor.constraint(equalTo: view.leadingAnchor, constant: 12), + hostingView.trailingAnchor.constraint(equalTo: view.trailingAnchor, constant: -12), + hostingView.topAnchor.constraint(equalTo: separator.bottomAnchor, constant: 12), + scrollView.topAnchor.constraint(equalTo: hostingView.bottomAnchor, constant: 12) + ] NSLayoutConstraint.activate([ titleLabel.leadingAnchor.constraint(equalTo: view.leadingAnchor, constant: 12), @@ -153,19 +174,33 @@ final class DownloadsViewController: NSViewController { view.trailingAnchor.constraint(equalTo: clearDownloadsButton.trailingAnchor, constant: 11), clearDownloadsButton.centerYAnchor.constraint(equalTo: openDownloadsFolderButton.centerYAnchor), - view.trailingAnchor.constraint(equalTo: scrollView.trailingAnchor), - view.bottomAnchor.constraint(equalTo: scrollView.bottomAnchor), - scrollView.topAnchor.constraint(equalTo: view.topAnchor, constant: 44), - scrollView.leadingAnchor.constraint(equalTo: view.leadingAnchor), - separator.centerXAnchor.constraint(equalTo: view.centerXAnchor), separator.widthAnchor.constraint(equalTo: view.widthAnchor, constant: -2), separator.topAnchor.constraint(equalTo: view.topAnchor, constant: 43), + scrollView.leadingAnchor.constraint(equalTo: view.leadingAnchor), + scrollView.trailingAnchor.constraint(equalTo: view.trailingAnchor), + view.bottomAnchor.constraint(equalTo: scrollView.bottomAnchor, constant: 12), + + errorBannerTopAnchorConstraint, tableViewHeightConstraint ]) } + private func showErrorBanner() { + errorBannerHostingView?.isHidden = false + NSLayoutConstraint.deactivate([errorBannerTopAnchorConstraint]) + NSLayoutConstraint.activate(hostingViewConstraints) + view.layoutSubtreeIfNeeded() + } + + private func hideErrorBanner() { + errorBannerHostingView?.isHidden = true + NSLayoutConstraint.deactivate(hostingViewConstraints) + NSLayoutConstraint.activate([errorBannerTopAnchorConstraint]) + view.layoutSubtreeIfNeeded() + } + override func viewDidLoad() { super.viewDidLoad() @@ -204,6 +239,17 @@ final class DownloadsViewController: NSViewController { } } + errorBannerCancellable = viewModel.$shouldShowErrorBanner + .sink { [weak self] shouldShowErrorBanner in + guard let self = self else { return } + + if shouldShowErrorBanner { + self.showErrorBanner() + } else { + self.hideErrorBanner() + } + } + for item in viewModel.items { item.didAppear() // initial table appearance should have no progress animations } @@ -213,6 +259,7 @@ final class DownloadsViewController: NSViewController { override func viewWillDisappear() { downloadsCancellable = nil + errorBannerCancellable = nil } private func setUpContextMenu() -> NSMenu { @@ -384,7 +431,7 @@ extension DownloadsViewController: NSMenuDelegate { for menuItem in menu.items { switch menuItem.action { case #selector(openDownloadAction(_:)), - #selector(revealDownloadAction(_:)): + #selector(revealDownloadAction(_:)): if case .complete(.some(let url)) = item.state, FileManager.default.fileExists(atPath: url.path) { menuItem.isHidden = false @@ -479,6 +526,55 @@ extension DownloadsViewController: NSTableViewDataSource, NSTableViewDelegate { } +enum DownloadsErrorViewType { + case openHelpURL + case openSystemSettings + + var errorMessage: String { + return UserText.downloadsErrorMessage + } + + var title: String { + switch self { + case .openHelpURL: return UserText.downloadsErrorSandboxCallToAction + case .openSystemSettings: return UserText.downloadsErrorNonSandboxCallToAction + } + } + + @MainActor func onAction() { + switch self { + case .openHelpURL: + let updateHelpURL = URL(string: "https://support.apple.com/guide/mac-help/get-macos-updates-and-apps-mh35618/mac")! + WindowControllersManager.shared.show(url: updateHelpURL, source: .ui, newTab: true) + case .openSystemSettings: + let softwareUpdateURL = URL(string: "x-apple.systempreferences:com.apple.Software-Update-Settings.extension")! + NSWorkspace.shared.open(softwareUpdateURL) + } + } +} + +struct DownloadsErrorBannerView: View { + var dismiss: () -> Void + let errorType: DownloadsErrorViewType + + var body: some View { + HStack { + Image("Clear-Recolorable-16") + Text(errorType.errorMessage) + .font(.body) + Button(errorType.title) { + errorType.onAction() + dismiss() + } + } + .padding() + .background(Color.gray.opacity(0.1)) + .cornerRadius(8) + .frame(width: 420) + .frame(minHeight: 84.0) + } +} + #if DEBUG @available(macOS 14.0, *) #Preview(traits: DownloadsViewController.preferredContentSize.fixedLayout) { { diff --git a/DuckDuckGo/Localizable.xcstrings b/DuckDuckGo/Localizable.xcstrings index 86503b711c..a96916fab8 100644 --- a/DuckDuckGo/Localizable.xcstrings +++ b/DuckDuckGo/Localizable.xcstrings @@ -19452,6 +19452,42 @@ } } }, + "downloads.error.cta.non-sandbox" : { + "comment" : "Call to action for the OS specific downloads issue", + "extractionState" : "extracted_with_value", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "new", + "value" : "Open Settings" + } + } + } + }, + "downloads.error.cta.sandbox" : { + "comment" : "Call to action for the OS specific downloads issue", + "extractionState" : "extracted_with_value", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "new", + "value" : "How To Update" + } + } + } + }, + "downloads.error.message.for.specific.os" : { + "comment" : "This error message will appear in an error banner when users cannot download files on macOS 14.7.1 or 15.0.1", + "extractionState" : "extracted_with_value", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "new", + "value" : "The download failed because of a known issue on macOS 14.7.1 and 15.0.1. Update to macOS 15.1 and try downloading again." + } + } + } + }, "downloads.error.move.failed" : { "comment" : "Short error description when could not move downloaded file to the Downloads folder", "extractionState" : "extracted_with_value", @@ -64570,4 +64606,4 @@ } }, "version" : "1.0" -} +} \ No newline at end of file From 55246c9cc7abe9f97658ea2db85759117d43c75e Mon Sep 17 00:00:00 2001 From: Anka Date: Fri, 22 Nov 2024 05:17:32 +0000 Subject: [PATCH 2/2] Bump version to 1.115.0 (313) --- Configuration/BuildNumber.xcconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Configuration/BuildNumber.xcconfig b/Configuration/BuildNumber.xcconfig index 206ff962a3..74c36f48b9 100644 --- a/Configuration/BuildNumber.xcconfig +++ b/Configuration/BuildNumber.xcconfig @@ -1 +1 @@ -CURRENT_PROJECT_VERSION = 312 +CURRENT_PROJECT_VERSION = 313