From 9aa93166b70a97c0645636e0ff1b8c06d688b136 Mon Sep 17 00:00:00 2001 From: Brent Simmons Date: Fri, 9 May 2025 09:22:34 -0700 Subject: [PATCH] Revert more changes since 6193 in hopes of fixing the scrolling bug. --- .../Timeline/TimelineViewController.swift | 2 +- .../Sources/RSCore/Shared/String+RSCore.swift | 17 +-- Modules/RSWeb/Sources/RSWeb/Downloader.swift | 110 ++---------------- .../RSWeb/HTMLMetadataDownloader.swift | 40 +++---- .../ArticleThemeDownloader.swift | 4 - Shared/Favicons/SingleFaviconDownloader.swift | 30 +++-- Shared/Images/ImageDownloader.swift | 36 +++--- Shared/Timeline/ArticleArray.swift | 8 +- iOS/Article/ImageViewController.swift | 10 -- iOS/Article/WebViewController.swift | 40 +++---- iOS/MainFeed/MainFeedViewController.swift | 1 - .../MainTimelineViewController.swift | 1 - 12 files changed, 84 insertions(+), 215 deletions(-) diff --git a/Mac/MainWindow/Timeline/TimelineViewController.swift b/Mac/MainWindow/Timeline/TimelineViewController.swift index f19276b9f..ce37ebc9f 100644 --- a/Mac/MainWindow/Timeline/TimelineViewController.swift +++ b/Mac/MainWindow/Timeline/TimelineViewController.swift @@ -85,7 +85,7 @@ final class TimelineViewController: NSViewController, UndoableCommandRunner, Unr var showsSearchResults = false var selectedArticles: [Article] { - return articles.articlesForIndexes(tableView.selectedRowIndexes) + return Array(articles.articlesForIndexes(tableView.selectedRowIndexes)) } var hasAtLeastOneSelectedArticle: Bool { diff --git a/Modules/RSCore/Sources/RSCore/Shared/String+RSCore.swift b/Modules/RSCore/Sources/RSCore/Shared/String+RSCore.swift index 237a268d0..612f4fe60 100644 --- a/Modules/RSCore/Sources/RSCore/Shared/String+RSCore.swift +++ b/Modules/RSCore/Sources/RSCore/Shared/String+RSCore.swift @@ -189,28 +189,13 @@ public extension String { /// Removes an HTML tag and everything between its start and end tags. /// - /// The regex pattern `[\\s\\S]*?` explanation: - /// - `<` matches the literal `<` character. - /// - `tag` matches the literal parameter provided to the function, e.g., `style`. - /// - `>` matches the literal `>` character. - /// - `[\\s\\S]*?` - /// - `[\\s\\S]` matches _any_ character, including new lines. - /// - `*` will match zero or more of the preceeding character, in this case _any_ - /// character. - /// - `?` switches the matching mode to [lazy](https://javascript.info/regexp-greedy-and-lazy) - /// so it will match as few as characters as possible before satisfying the rest of the pattern. - /// - `<` matches the literal `<` character. - /// - `/` matches the literal `/` character. - /// - `tag` matches the literal parameter provided to the function, e.g., `style`. - /// - `>` matches the literal `>` character. - /// /// - Parameter tag: The tag to remove. /// /// - Returns: A new copy of `self` with the tag removed. /// /// - Note: Doesn't work correctly with nested tags of the same name. private func removingTagAndContents(_ tag: String) -> String { - return self.replacingOccurrences(of: "<\(tag)>[\\s\\S]*?", with: "", options: [.regularExpression, .caseInsensitive]) + return self.replacingOccurrences(of: "<\(tag).+?", with: "", options: [.regularExpression, .caseInsensitive]) } /// Strips HTML from a string. diff --git a/Modules/RSWeb/Sources/RSWeb/Downloader.swift b/Modules/RSWeb/Sources/RSWeb/Downloader.swift index 2588eaa9f..7b052a134 100755 --- a/Modules/RSWeb/Sources/RSWeb/Downloader.swift +++ b/Modules/RSWeb/Sources/RSWeb/Downloader.swift @@ -7,34 +7,25 @@ // import Foundation -import os -import RSCore -public typealias DownloadCallback = @MainActor (Data?, URLResponse?, Error?) -> Swift.Void +public typealias DownloadCallback = (Data?, URLResponse?, Error?) -> Swift.Void /// Simple downloader, for a one-shot download like an image /// or a web page. For a download-feeds session, see DownloadSession. -/// Caches response for a short time for GET requests. May return cached response. -@MainActor public final class Downloader { +public final class Downloader { public static let shared = Downloader() private let urlSession: URLSession - private var callbacks = [URL: [DownloadCallback]]() - - // Cache — short-lived - private let cache = Cache(timeToLive: 60 * 3, timeBetweenCleanups: 60 * 2) - - nonisolated private static let logger = Logger(subsystem: Bundle.main.bundleIdentifier!, category: "Downloader") - nonisolated private static let debugLoggingEnabled = false private init() { + let sessionConfiguration = URLSessionConfiguration.ephemeral sessionConfiguration.requestCachePolicy = .reloadIgnoringLocalCacheData sessionConfiguration.httpShouldSetCookies = false sessionConfiguration.httpCookieAcceptPolicy = .never sessionConfiguration.httpMaximumConnectionsPerHost = 1 sessionConfiguration.httpCookieStorage = nil - + if let userAgentHeaders = UserAgent.headers() { sessionConfiguration.httpAdditionalHeaders = userAgentHeaders } @@ -46,103 +37,20 @@ public typealias DownloadCallback = @MainActor (Data?, URLResponse?, Error?) -> urlSession.invalidateAndCancel() } - public func download(_ url: URL, _ callback: @escaping DownloadCallback) { - assert(Thread.isMainThread) - download(URLRequest(url: url), callback) + public func download(_ url: URL, _ completion: DownloadCallback? = nil) { + download(URLRequest(url: url), completion) } - public func download(_ urlRequest: URLRequest, _ callback: @escaping DownloadCallback) { - assert(Thread.isMainThread) - - guard let url = urlRequest.url else { - Self.logger.fault("Downloader: skipping download for URLRequest without a URL") - return - } - - let isCacheableRequest = urlRequest.httpMethod == HTTPMethod.get - - // Return cached record if available. - if isCacheableRequest { - if let cachedRecord = cache[url.absoluteString] { - if Self.debugLoggingEnabled { - Self.logger.debug("Downloader: returning cached record for \(url)") - } - callback(cachedRecord.data, cachedRecord.response, cachedRecord.error) - return - } - } - - // Add callback. If there is already a download in progress for this URL, return early. - if callbacks[url] == nil { - if Self.debugLoggingEnabled { - Self.logger.debug("Downloader: downloading \(url)") - } - callbacks[url] = [callback] - } else { - // A download is already be in progress for this URL. Don’t start a separate download. - // Add the callback to the callbacks array for this URL. - if Self.debugLoggingEnabled { - Self.logger.debug("Downloader: download in progress for \(url) — adding callback") - } - callbacks[url]?.append(callback) - return - } + public func download(_ urlRequest: URLRequest, _ completion: DownloadCallback? = nil) { var urlRequestToUse = urlRequest urlRequestToUse.addSpecialCaseUserAgentIfNeeded() let task = urlSession.dataTask(with: urlRequestToUse) { (data, response, error) in - - if isCacheableRequest { - if Self.debugLoggingEnabled { - Self.logger.debug("Downloader: caching record for \(url)") - } - let cachedRecord = DownloaderRecord(data: data, response: response, error: error) - self.cache[url.absoluteString] = cachedRecord - } - - Task { @MainActor in - self.callAndReleaseCallbacks(url, data, response, error) + DispatchQueue.main.async() { + completion?(data, response, error) } } task.resume() } } - -private extension Downloader { - - func callAndReleaseCallbacks(_ url: URL, _ data: Data? = nil, _ response: URLResponse? = nil, _ error: Error? = nil) { - assert(Thread.isMainThread) - - defer { - callbacks[url] = nil - } - - guard let callbacksForURL = callbacks[url] else { - assertionFailure("Downloader: downloaded URL \(url) but no callbacks found") - Self.logger.fault("Downloader: downloaded URL \(url) but no callbacks found") - return - } - - if Self.debugLoggingEnabled { - let count = callbacksForURL.count - if count == 1 { - Self.logger.debug("Downloader: calling 1 callback for URL \(url)") - } else { - Self.logger.debug("Downloader: calling \(count) callbacks for URL \(url)") - } - } - - for callback in callbacksForURL { - callback(data, response, error) - } - } -} - -struct DownloaderRecord: CacheRecord, Sendable { - - let dateCreated = Date() - let data: Data? - let response: URLResponse? - let error: Error? -} diff --git a/Modules/RSWeb/Sources/RSWeb/HTMLMetadataDownloader.swift b/Modules/RSWeb/Sources/RSWeb/HTMLMetadataDownloader.swift index 6d8a0eea4..96090e89d 100644 --- a/Modules/RSWeb/Sources/RSWeb/HTMLMetadataDownloader.swift +++ b/Modules/RSWeb/Sources/RSWeb/HTMLMetadataDownloader.swift @@ -75,38 +75,36 @@ private extension HTMLMetadataDownloader { } func downloadMetadata(_ url: String) { - + guard let actualURL = URL(string: url) else { if Self.debugLoggingEnabled { Self.logger.debug("HTMLMetadataDownloader skipping download for \(url) because it couldn’t construct a URL.") } return } - + if Self.debugLoggingEnabled { Self.logger.debug("HTMLMetadataDownloader downloading for \(url)") } - - Task { @MainActor in - Downloader.shared.download(actualURL) { data, response, error in - if let data, !data.isEmpty, let response, response.statusIsOK { - let urlToUse = response.url ?? actualURL - let parserData = ParserData(url: urlToUse.absoluteString, data: data) - let htmlMetadata = RSHTMLMetadataParser.htmlMetadata(with: parserData) - if Self.debugLoggingEnabled { - Self.logger.debug("HTMLMetadataDownloader caching parsed metadata for \(url)") - } - self.cache[url] = htmlMetadata - return - } - - if let statusCode = response?.forcedStatusCode, (400...499).contains(statusCode) { - self.noteURLDidReturn4xx(url) - } - + + Downloader.shared.download(actualURL) { data, response, error in + if let data, !data.isEmpty, let response, response.statusIsOK { + let urlToUse = response.url ?? actualURL + let parserData = ParserData(url: urlToUse.absoluteString, data: data) + let htmlMetadata = RSHTMLMetadataParser.htmlMetadata(with: parserData) if Self.debugLoggingEnabled { - Self.logger.debug("HTMLMetadataDownloader failed download for \(url)") + Self.logger.debug("HTMLMetadataDownloader caching parsed metadata for \(url)") } + self.cache[url] = htmlMetadata + return + } + + if let statusCode = response?.forcedStatusCode, (400...499).contains(statusCode) { + self.noteURLDidReturn4xx(url) + } + + if Self.debugLoggingEnabled { + Self.logger.debug("HTMLMetadataDownloader failed download for \(url)") } } } diff --git a/Shared/ArticleStyles/ArticleThemeDownloader.swift b/Shared/ArticleStyles/ArticleThemeDownloader.swift index 43ee04567..d6714e169 100644 --- a/Shared/ArticleStyles/ArticleThemeDownloader.swift +++ b/Shared/ArticleStyles/ArticleThemeDownloader.swift @@ -75,10 +75,6 @@ public class ArticleThemeDownloader { private func findThemeFile(in searchPath: String) -> String? { if let directoryContents = FileManager.default.enumerator(atPath: searchPath) { while let file = directoryContents.nextObject() as? String { - if file.hasPrefix("__MACOSX/") { - //logger.debug("Ignoring theme file in __MACOSX folder.") - continue - } if file.hasSuffix(".nnwtheme") { return file } diff --git a/Shared/Favicons/SingleFaviconDownloader.swift b/Shared/Favicons/SingleFaviconDownloader.swift index bc952bdf4..1d3a8e035 100644 --- a/Shared/Favicons/SingleFaviconDownloader.swift +++ b/Shared/Favicons/SingleFaviconDownloader.swift @@ -133,27 +133,25 @@ private extension SingleFaviconDownloader { } func downloadFavicon(_ completion: @escaping (RSImage?) -> Void) { - + guard let url = URL(string: faviconURL) else { completion(nil) return } - - Task { @MainActor in - Downloader.shared.download(url) { (data, response, error) in - - if let data = data, !data.isEmpty, let response = response, response.statusIsOK, error == nil { - self.saveToDisk(data) - RSImage.image(with: data, imageResultBlock: completion) - return - } - - if let error = error { - os_log(.info, log: self.log, "Error downloading image at %@: %@.", url.absoluteString, error.localizedDescription) - } - - completion(nil) + + Downloader.shared.download(url) { (data, response, error) in + + if let data = data, !data.isEmpty, let response = response, response.statusIsOK, error == nil { + self.saveToDisk(data) + RSImage.image(with: data, imageResultBlock: completion) + return } + + if let error = error { + os_log(.info, log: self.log, "Error downloading image at %@: %@.", url.absoluteString, error.localizedDescription) + } + + completion(nil) } } diff --git a/Shared/Images/ImageDownloader.swift b/Shared/Images/ImageDownloader.swift index b5233ae55..f794e61a9 100644 --- a/Shared/Images/ImageDownloader.swift +++ b/Shared/Images/ImageDownloader.swift @@ -99,30 +99,28 @@ private extension ImageDownloader { } func downloadImage(_ url: String, _ completion: @escaping (Data?) -> Void) { - + guard let imageURL = URL(string: url) else { completion(nil) return } - - Task { @MainActor in - Downloader.shared.download(imageURL) { (data, response, error) in - - if let data = data, !data.isEmpty, let response = response, response.statusIsOK, error == nil { - self.saveToDisk(url, data) - completion(data) - return - } - - if let response = response as? HTTPURLResponse, response.statusCode >= HTTPResponseCode.badRequest && response.statusCode <= HTTPResponseCode.notAcceptable { - self.badURLs.insert(url) - } - if let error = error { - os_log(.info, log: self.log, "Error downloading image at %@: %@.", url, error.localizedDescription) - } - - completion(nil) + + Downloader.shared.download(imageURL) { (data, response, error) in + + if let data = data, !data.isEmpty, let response = response, response.statusIsOK, error == nil { + self.saveToDisk(url, data) + completion(data) + return } + + if let response = response as? HTTPURLResponse, response.statusCode >= HTTPResponseCode.badRequest && response.statusCode <= HTTPResponseCode.notAcceptable { + self.badURLs.insert(url) + } + if let error = error { + os_log(.info, log: self.log, "Error downloading image at %@: %@.", url, error.localizedDescription) + } + + completion(nil) } } diff --git a/Shared/Timeline/ArticleArray.swift b/Shared/Timeline/ArticleArray.swift index 2f8d69e06..5ac8da789 100644 --- a/Shared/Timeline/ArticleArray.swift +++ b/Shared/Timeline/ArticleArray.swift @@ -44,12 +44,12 @@ extension Array where Element == Article { return nil } - func articlesForIndexes(_ indexes: IndexSet) -> [Article] { - return indexes.compactMap{ (oneIndex) -> Article? in + func articlesForIndexes(_ indexes: IndexSet) -> Set
{ + return Set(indexes.compactMap{ (oneIndex) -> Article? in return articleAtRow(oneIndex) - } + }) } - + func sortedByDate(_ sortDirection: ComparisonResult, groupByFeed: Bool = false) -> ArticleArray { return ArticleSorter.sortedByDate(articles: self, sortDirection: sortDirection, groupByFeed: groupByFeed) } diff --git a/iOS/Article/ImageViewController.swift b/iOS/Article/ImageViewController.swift index fd1f17d79..91d8a17d9 100644 --- a/iOS/Article/ImageViewController.swift +++ b/iOS/Article/ImageViewController.swift @@ -25,16 +25,6 @@ class ImageViewController: UIViewController { return imageScrollView.zoomedFrame } - override var keyCommands: [UIKeyCommand]? { - return [ - UIKeyCommand( - title: NSLocalizedString("Close Image", comment: "Close Image"), - action: #selector(done(_:)), - input: " " - ) - ] - } - override func viewDidLoad() { super.viewDidLoad() diff --git a/iOS/Article/WebViewController.swift b/iOS/Article/WebViewController.swift index 5f6666edd..c5048bf09 100644 --- a/iOS/Article/WebViewController.swift +++ b/iOS/Article/WebViewController.swift @@ -34,12 +34,11 @@ class WebViewController: UIViewController { private var webView: PreloadedWebView? { return view.subviews[0] as? PreloadedWebView } - + private lazy var contextMenuInteraction = UIContextMenuInteraction(delegate: self) private var isFullScreenAvailable: Bool { return AppDefaults.shared.articleFullscreenAvailable && traitCollection.userInterfaceIdiom == .phone && coordinator.isRootSplitCollapsed } - private lazy var articleIconSchemeHandler = ArticleIconSchemeHandler(coordinator: coordinator); private lazy var transition = ImageTransition(controller: self) private var clickedImageCompletion: (() -> Void)? @@ -359,15 +358,15 @@ extension WebViewController: WKNavigationDelegate { } } } - + func webView(_ webView: WKWebView, decidePolicyFor navigationAction: WKNavigationAction, decisionHandler: @escaping (WKNavigationActionPolicy) -> Void) { - + if navigationAction.navigationType == .linkActivated { guard let url = navigationAction.request.url else { decisionHandler(.allow) return } - + let components = URLComponents(url: url, resolvingAgainstBaseURL: false) if components?.scheme == "http" || components?.scheme == "https" { decisionHandler(.cancel) @@ -381,14 +380,14 @@ extension WebViewController: WKNavigationDelegate { self.openURLInSafariViewController(url) } } - + } else if components?.scheme == "mailto" { decisionHandler(.cancel) - + guard let emailAddress = url.percentEncodedEmailAddress else { return } - + if UIApplication.shared.canOpenURL(emailAddress) { UIApplication.shared.open(emailAddress, options: [.universalLinksOnly : false], completionHandler: nil) } else { @@ -398,11 +397,11 @@ extension WebViewController: WKNavigationDelegate { } } else if components?.scheme == "tel" { decisionHandler(.cancel) - + if UIApplication.shared.canOpenURL(url) { UIApplication.shared.open(url, options: [.universalLinksOnly : false], completionHandler: nil) } - + } else { decisionHandler(.allow) } @@ -414,7 +413,7 @@ extension WebViewController: WKNavigationDelegate { func webViewWebContentProcessDidTerminate(_ webView: WKWebView) { fullReload() } - + } // MARK: WKUIDelegate @@ -514,16 +513,16 @@ private extension WebViewController { func loadWebView(replaceExistingWebView: Bool = false) { guard isViewLoaded else { return } - + if !replaceExistingWebView, let webView = webView { self.renderPage(webView) return } - + coordinator.webViewProvider.dequeueWebView() { webView in - + webView.ready { - + // Add the webview webView.translatesAutoresizingMaskIntoConstraints = false self.view.insertSubview(webView, at: 0) @@ -533,7 +532,7 @@ private extension WebViewController { self.view.topAnchor.constraint(equalTo: webView.topAnchor), self.view.bottomAnchor.constraint(equalTo: webView.bottomAnchor) ]) - + // UISplitViewController reports the wrong size to WKWebView which can cause horizontal // rubberbanding on the iPad. This interferes with our UIPageViewController preventing // us from easily swiping between WKWebViews. This hack fixes that. @@ -555,11 +554,11 @@ private extension WebViewController { webView.configuration.userContentController.add(WrapperScriptMessageHandler(self), name: MessageName.showFeedInspector) self.renderPage(webView) - + } - + } - + } func renderPage(_ webView: PreloadedWebView?) { @@ -592,8 +591,7 @@ private extension WebViewController { "windowScrollY": String(windowScrollY) ] - var html = try! MacroProcessor.renderedText(withTemplate: ArticleRenderer.page.html, substitutions: substitutions) - html = ArticleRenderingSpecialCases.filterHTMLIfNeeded(baseURL: rendering.baseURL, html: html) + let html = try! MacroProcessor.renderedText(withTemplate: ArticleRenderer.page.html, substitutions: substitutions) webView.loadHTMLString(html, baseURL: ArticleRenderer.page.baseURL) } diff --git a/iOS/MainFeed/MainFeedViewController.swift b/iOS/MainFeed/MainFeedViewController.swift index 4fc8afb16..db4af7b9e 100644 --- a/iOS/MainFeed/MainFeedViewController.swift +++ b/iOS/MainFeed/MainFeedViewController.swift @@ -7,7 +7,6 @@ // import UIKit -import WebKit import Account import Articles import RSCore diff --git a/iOS/MainTimeline/MainTimelineViewController.swift b/iOS/MainTimeline/MainTimelineViewController.swift index ad7af3551..1f190923b 100644 --- a/iOS/MainTimeline/MainTimelineViewController.swift +++ b/iOS/MainTimeline/MainTimelineViewController.swift @@ -7,7 +7,6 @@ // import UIKit -import WebKit import RSCore import RSWeb import Account