From 97757a567f8b947408be60ebce595fd1ee6670f0 Mon Sep 17 00:00:00 2001 From: Brent Simmons Date: Mon, 20 Jan 2025 14:28:23 -0800 Subject: [PATCH] Unify DetailIconSchemeHandler and ArticleIconSchemeHandler into one shared ArticleIconSchemeHandler. --- .../Detail/DetailIconSchemeHandler.swift | 46 ----------- .../Detail/DetailWebViewController.swift | 27 +++++-- .../ArticleIconSchemeHandler.swift | 79 +++++++++++++++++++ iOS/Article/ArticleIconSchemeHandler.swift | 60 -------------- iOS/Article/WebViewController.swift | 26 +++++- 5 files changed, 123 insertions(+), 115 deletions(-) delete mode 100644 Mac/MainWindow/Detail/DetailIconSchemeHandler.swift create mode 100644 Shared/Article Rendering/ArticleIconSchemeHandler.swift delete mode 100644 iOS/Article/ArticleIconSchemeHandler.swift diff --git a/Mac/MainWindow/Detail/DetailIconSchemeHandler.swift b/Mac/MainWindow/Detail/DetailIconSchemeHandler.swift deleted file mode 100644 index 4aee30c11..000000000 --- a/Mac/MainWindow/Detail/DetailIconSchemeHandler.swift +++ /dev/null @@ -1,46 +0,0 @@ -// -// DetailIconSchemeHandler.swift -// NetNewsWire-iOS -// -// Created by Maurice Parker on 11/7/19. -// Copyright © 2019 Ranchero Software. All rights reserved. -// - -import Foundation -import WebKit -import Articles - -class DetailIconSchemeHandler: NSObject, WKURLSchemeHandler { - - var currentArticle: Article? - - func webView(_ webView: WKWebView, start urlSchemeTask: WKURLSchemeTask) { - - guard let responseURL = urlSchemeTask.request.url, let iconImage = self.currentArticle?.iconImage() else { - urlSchemeTask.didFailWithError(URLError(.fileDoesNotExist)) - return - } - - let iconView = IconView(frame: CGRect(x: 0, y: 0, width: 48, height: 48)) - iconView.iconImage = iconImage - let renderedImage = iconView.asImage() - - guard let data = renderedImage.dataRepresentation() else { - urlSchemeTask.didFailWithError(URLError(.fileDoesNotExist)) - return - } - - let headerFields = ["Cache-Control": "no-cache"] - if let response = HTTPURLResponse(url: responseURL, statusCode: 200, httpVersion: nil, headerFields: headerFields) { - urlSchemeTask.didReceive(response) - urlSchemeTask.didReceive(data) - urlSchemeTask.didFinish() - } - - } - - func webView(_ webView: WKWebView, stop urlSchemeTask: WKURLSchemeTask) { - urlSchemeTask.didFailWithError(URLError(.unknown)) - } - -} diff --git a/Mac/MainWindow/Detail/DetailWebViewController.swift b/Mac/MainWindow/Detail/DetailWebViewController.swift index 37a3cfb25..1c99691af 100644 --- a/Mac/MainWindow/Detail/DetailWebViewController.swift +++ b/Mac/MainWindow/Detail/DetailWebViewController.swift @@ -56,8 +56,8 @@ final class DetailWebViewController: NSViewController { webView.configuration.preferences._developerExtrasEnabled = newValue } } - - private let detailIconSchemeHandler = DetailIconSchemeHandler() + + private lazy var articleIconSchemeHandler = ArticleIconSchemeHandler(delegate: self) private var waitingForFirstReload = false private let keyboardDelegate = DetailKeyboardDelegate() private var windowScrollY: CGFloat? @@ -79,7 +79,7 @@ final class DetailWebViewController: NSViewController { override func loadView() { - let configuration = WebViewConfiguration.configuration(with: detailIconSchemeHandler) + let configuration = WebViewConfiguration.configuration(with: articleIconSchemeHandler) webView = DetailWebView(frame: NSRect.zero, configuration: configuration) webView.uiDelegate = self @@ -171,6 +171,25 @@ final class DetailWebViewController: NSViewController { } +// MARK: - ArticleIconSchemeHandlerDelegate + +extension DetailWebViewController: ArticleIconSchemeHandlerDelegate { + + func articleIconSchemeHandler(_: ArticleIconSchemeHandler, imageForArticleID articleID: String) -> IconImage? { + + guard let article else { + assertionFailure("Did not expect request for article image when there is no current article.") + return nil + } + guard articleID == article.articleID else { + assertionFailure("Expected articleID to match current articleID.") + return nil + } + + return article.iconImage() // May be nil — not a programming error + } +} + // MARK: - WKScriptMessageHandler extension DetailWebViewController: WKScriptMessageHandler { @@ -282,10 +301,8 @@ private extension DetailWebViewController { case .loading: rendering = ArticleRenderer.loadingHTML(theme: theme) case .article(let article, _): - detailIconSchemeHandler.currentArticle = article rendering = ArticleRenderer.articleHTML(article: article, theme: theme) case .extracted(let article, let extractedArticle, _): - detailIconSchemeHandler.currentArticle = article rendering = ArticleRenderer.articleHTML(article: article, extractedArticle: extractedArticle, theme: theme) } diff --git a/Shared/Article Rendering/ArticleIconSchemeHandler.swift b/Shared/Article Rendering/ArticleIconSchemeHandler.swift new file mode 100644 index 000000000..fbfbef6dc --- /dev/null +++ b/Shared/Article Rendering/ArticleIconSchemeHandler.swift @@ -0,0 +1,79 @@ +// +// ArticleIconSchemeHandler.swift +// NetNewsWire +// +// Created by Brent Simmons on 1/20/25. +// Copyright © 2025 Ranchero Software. All rights reserved. +// + +import Foundation +import WebKit + +protocol ArticleIconSchemeHandlerDelegate: AnyObject { + func articleIconSchemeHandler(_: ArticleIconSchemeHandler, imageForArticleID: String) -> IconImage? +} + +final class ArticleIconSchemeHandler: NSObject, WKURLSchemeHandler { + + private weak var delegate: ArticleIconSchemeHandlerDelegate? + private static let headerFields = ["Cache-Control": "no-cache"] + + init(delegate: ArticleIconSchemeHandlerDelegate) { + self.delegate = delegate + } + + // WKURLSchemeHandler is @MainActor, so this is @MainActor. + + func webView(_ webView: WKWebView, start urlSchemeTask: WKURLSchemeTask) { + + guard let url = urlSchemeTask.request.url, + let components = URLComponents(url: url, resolvingAgainstBaseURL: false) else { + assertionFailure("Expected URL and components in ArticleIconSchemeHandler.") + urlSchemeTask.didFailWithError(URLError(.fileDoesNotExist)) + return + } + + let articleID = components.path + guard !articleID.isEmpty else { + assertionFailure("Expected non-empty articleID in ArticleIconSchemeHandler.") + urlSchemeTask.didFailWithError(URLError(.fileDoesNotExist)) + return + } + + guard let iconImage = delegate?.articleIconSchemeHandler(self, imageForArticleID: articleID) else { + // There may not be an image — this is not a programming error. + urlSchemeTask.didFailWithError(URLError(.fileDoesNotExist)) + return + } + + let iconView = IconView(frame: CGRect(x: 0, y: 0, width: 48, height: 48)) + iconView.iconImage = iconImage + let renderedImage = iconView.asImage() + + guard let data = renderedImage.dataRepresentation() else { + assertionFailure("Expected non-empty image data ArticleIconSchemeHandler.") + urlSchemeTask.didFailWithError(URLError(.fileDoesNotExist)) + return + } + + guard let response = HTTPURLResponse(url: url, statusCode: 200, httpVersion: nil, headerFields: Self.headerFields) else { + assertionFailure("Expected to create HTTPURLResponse but failed.") + urlSchemeTask.didFailWithError(URLError(.unknown)) + return + } + + urlSchemeTask.didReceive(response) + urlSchemeTask.didReceive(data) + urlSchemeTask.didFinish() + } + + func webView(_ webView: WKWebView, stop urlSchemeTask: WKURLSchemeTask) { + urlSchemeTask.didFailWithError(URLError(.unknown)) + } +} + +// TODO: The above code is re-rendering images multiple times, which is not good +// for performance. Fixing this will probably require some refactoring +// of the entire system for images, so that the code gets some kind of identifier — +// probably a URL — so that it has a key for a cache, so it doesn’t have to +// re-render images. diff --git a/iOS/Article/ArticleIconSchemeHandler.swift b/iOS/Article/ArticleIconSchemeHandler.swift deleted file mode 100644 index 62fe0ee62..000000000 --- a/iOS/Article/ArticleIconSchemeHandler.swift +++ /dev/null @@ -1,60 +0,0 @@ -// -// ArticleIconSchemeHandler.swift -// NetNewsWire-iOS -// -// Created by Maurice Parker on 1/27/20. -// Copyright © 2020 Ranchero Software. All rights reserved. -// - -import Foundation -import WebKit -import Articles - -class ArticleIconSchemeHandler: NSObject, WKURLSchemeHandler { - - weak var coordinator: SceneCoordinator? - - init(coordinator: SceneCoordinator) { - self.coordinator = coordinator - } - - func webView(_ webView: WKWebView, start urlSchemeTask: WKURLSchemeTask) { - - guard let url = urlSchemeTask.request.url, let coordinator = coordinator else { - urlSchemeTask.didFailWithError(URLError(.fileDoesNotExist)) - return - } - - guard let components = URLComponents(url: url, resolvingAgainstBaseURL: false) else { - return - } - let articleID = components.path - guard let iconImage = coordinator.articleFor(articleID)?.iconImage() else { - urlSchemeTask.didFailWithError(URLError(.fileDoesNotExist)) - return - } - - let iconView = IconView(frame: CGRect(x: 0, y: 0, width: 48, height: 48)) - iconView.iconImage = iconImage - let renderedImage = iconView.asImage() - - guard let data = renderedImage.dataRepresentation() else { - urlSchemeTask.didFailWithError(URLError(.fileDoesNotExist)) - return - } - - let headerFields = ["Cache-Control": "no-cache"] - if let response = HTTPURLResponse(url: url, statusCode: 200, httpVersion: nil, headerFields: headerFields) { - urlSchemeTask.didReceive(response) - urlSchemeTask.didReceive(data) - urlSchemeTask.didFinish() - } - - } - - func webView(_ webView: WKWebView, stop urlSchemeTask: WKURLSchemeTask) { - urlSchemeTask.didFailWithError(URLError(.unknown)) - } - -} - diff --git a/iOS/Article/WebViewController.swift b/iOS/Article/WebViewController.swift index 21c36ca93..a6275831e 100644 --- a/iOS/Article/WebViewController.swift +++ b/iOS/Article/WebViewController.swift @@ -18,8 +18,8 @@ protocol WebViewControllerDelegate: AnyObject { func webViewController(_: WebViewController, articleExtractorButtonStateDidUpdate: ArticleExtractorButtonState) } -class WebViewController: UIViewController { - +final class WebViewController: UIViewController { + private struct MessageName { static let imageWasClicked = "imageWasClicked" static let imageWasShown = "imageWasShown" @@ -39,7 +39,7 @@ class WebViewController: UIViewController { private var isFullScreenAvailable: Bool { return AppDefaults.shared.articleFullscreenAvailable && traitCollection.userInterfaceIdiom == .phone && coordinator.isRootSplitCollapsed } - private lazy var articleIconSchemeHandler = ArticleIconSchemeHandler(coordinator: coordinator); + private lazy var articleIconSchemeHandler = ArticleIconSchemeHandler(delegate: self) private lazy var transition = ImageTransition(controller: self) private var clickedImageCompletion: (() -> Void)? @@ -279,6 +279,25 @@ class WebViewController: UIViewController { } } +// MARK: - ArticleIconSchemeHandlerDelegate + +extension WebViewController: ArticleIconSchemeHandlerDelegate { + + func articleIconSchemeHandler(_: ArticleIconSchemeHandler, imageForArticleID articleID: String) -> IconImage? { + + guard let article else { + assertionFailure("Did not expect request for article image when there is no current article.") + return nil + } + guard articleID == article.articleID else { + assertionFailure("Expected articleID to match current articleID.") + return nil + } + + return article.iconImage() // May be nil — not a programming error + } +} + // MARK: ArticleExtractorDelegate extension WebViewController: ArticleExtractorDelegate { @@ -300,7 +319,6 @@ extension WebViewController: ArticleExtractorDelegate { articleExtractorButtonState = .on } } - } // MARK: UIContextMenuInteractionDelegate