From 9092d075a00f1465fa4124667df282d149a3c624 Mon Sep 17 00:00:00 2001 From: Duncan Babbage Date: Mon, 26 Apr 2021 09:28:19 +1200 Subject: [PATCH] Handle links with unencoded spaces. Fixes #3069 --- .../Timeline/ArticlePasteboardWriter.swift | 2 +- .../Article/ArticleToolbarModifier.swift | 2 +- .../Shared/Timeline/TimelineModel.swift | 5 ++--- Shared/Extensions/ArticleUtilities.swift | 12 ++++++++++++ iOS/Article/WebViewController.swift | 10 ++-------- .../MasterTimelineViewController.swift | 18 ++++-------------- iOS/SceneCoordinator.swift | 8 ++------ 7 files changed, 24 insertions(+), 33 deletions(-) diff --git a/Mac/MainWindow/Timeline/ArticlePasteboardWriter.swift b/Mac/MainWindow/Timeline/ArticlePasteboardWriter.swift index 272c55e16..908eed74b 100644 --- a/Mac/MainWindow/Timeline/ArticlePasteboardWriter.swift +++ b/Mac/MainWindow/Timeline/ArticlePasteboardWriter.swift @@ -38,7 +38,7 @@ extension Article: PasteboardWriterOwner { func writableTypes(for pasteboard: NSPasteboard) -> [NSPasteboard.PasteboardType] { var types = [ArticlePasteboardWriter.articleUTIType] - if let link = article.preferredLink, let _ = URL(string: link) { + if let _ = article.preferredURL { types += [.URL] } types += [.string, .html, ArticlePasteboardWriter.articleUTIInternalType] diff --git a/Multiplatform/Shared/Article/ArticleToolbarModifier.swift b/Multiplatform/Shared/Article/ArticleToolbarModifier.swift index dd148e4aa..380cba519 100644 --- a/Multiplatform/Shared/Article/ArticleToolbarModifier.swift +++ b/Multiplatform/Shared/Article/ArticleToolbarModifier.swift @@ -107,7 +107,7 @@ struct ArticleToolbarModifier: ViewModifier { .disabled(sceneModel.shareButtonState == nil) .help("Share") .sheet(isPresented: $showActivityView) { - if let article = sceneModel.selectedArticles.first, let link = article.preferredLink, let url = URL(string: link) { + if let article = sceneModel.selectedArticles.first, let url = article.preferredURL { ActivityViewController(title: article.title, url: url) } } diff --git a/Multiplatform/Shared/Timeline/TimelineModel.swift b/Multiplatform/Shared/Timeline/TimelineModel.swift index b96e5422a..8c77c3a88 100644 --- a/Multiplatform/Shared/Timeline/TimelineModel.swift +++ b/Multiplatform/Shared/Timeline/TimelineModel.swift @@ -249,12 +249,11 @@ class TimelineModel: ObservableObject, UndoableCommandRunner { } func openIndicatedArticleInBrowser(_ article: Article) { - guard let link = article.preferredLink else { return } - #if os(macOS) + guard let link = article.preferredLink else { return } Browser.open(link, invertPreference: NSApp.currentEvent?.modifierFlags.contains(.shift) ?? false) #else - guard let url = URL(string: link) else { return } + guard let url = article.preferredURL else { return } UIApplication.shared.open(url, options: [:]) #endif } diff --git a/Shared/Extensions/ArticleUtilities.swift b/Shared/Extensions/ArticleUtilities.swift index 5e1822cba..1ad6a0f6f 100644 --- a/Shared/Extensions/ArticleUtilities.swift +++ b/Shared/Extensions/ArticleUtilities.swift @@ -56,6 +56,18 @@ extension Article { return nil } + var preferredURL: URL? { + guard let link = preferredLink else { return nil } + // If required, we replace any space characters to handle malformed links that are otherwise percent + // encoded but contain spaces. For performance reasons, only try this if initial URL init fails. + if let url = URL(string: link) { + return url + } else if let url = URL(string: link.replacingOccurrences(of: " ", with: "%20")) { + return url + } + return nil + } + var body: String? { return contentHTML ?? contentText ?? summary } diff --git a/iOS/Article/WebViewController.swift b/iOS/Article/WebViewController.swift index 18486efc3..968a00ea6 100644 --- a/iOS/Article/WebViewController.swift +++ b/iOS/Article/WebViewController.swift @@ -235,20 +235,14 @@ class WebViewController: UIViewController { } func showActivityDialog(popOverBarButtonItem: UIBarButtonItem? = nil) { - guard let preferredLink = article?.preferredLink, let url = URL(string: preferredLink) else { - return - } - + guard let url = article?.preferredURL else { return } let activityViewController = UIActivityViewController(url: url, title: article?.title, applicationActivities: [FindInArticleActivity(), OpenInBrowserActivity()]) activityViewController.popoverPresentationController?.barButtonItem = popOverBarButtonItem present(activityViewController, animated: true) } func openInAppBrowser() { - guard let preferredLink = article?.preferredLink, let url = URL(string: preferredLink) else { - return - } - + guard let url = article?.preferredURL else { return } let vc = SFSafariViewController(url: url) present(vc, animated: true) } diff --git a/iOS/MasterTimeline/MasterTimelineViewController.swift b/iOS/MasterTimeline/MasterTimelineViewController.swift index cc7ef3cd2..1a99608bd 100644 --- a/iOS/MasterTimeline/MasterTimelineViewController.swift +++ b/iOS/MasterTimeline/MasterTimelineViewController.swift @@ -889,9 +889,7 @@ private extension MasterTimelineViewController { } func openInBrowserAction(_ article: Article) -> UIAction? { - guard let preferredLink = article.preferredLink, let _ = URL(string: preferredLink) else { - return nil - } + guard let _ = article.preferredURL else { return nil } let title = NSLocalizedString("Open in Browser", comment: "Open in Browser") let action = UIAction(title: title, image: AppAssets.safariImage) { [weak self] action in self?.coordinator.showBrowserForArticle(article) @@ -900,9 +898,7 @@ private extension MasterTimelineViewController { } func openInBrowserAlertAction(_ article: Article, completion: @escaping (Bool) -> Void) -> UIAlertAction? { - guard let preferredLink = article.preferredLink, let _ = URL(string: preferredLink) else { - return nil - } + guard let _ = article.preferredURL else { return nil } let title = NSLocalizedString("Open in Browser", comment: "Open in Browser") let action = UIAlertAction(title: title, style: .default) { [weak self] action in self?.coordinator.showBrowserForArticle(article) @@ -923,10 +919,7 @@ private extension MasterTimelineViewController { } func shareAction(_ article: Article, indexPath: IndexPath) -> UIAction? { - guard let preferredLink = article.preferredLink, let url = URL(string: preferredLink) else { - return nil - } - + guard let url = article.preferredURL else { return nil } let title = NSLocalizedString("Share", comment: "Share") let action = UIAction(title: title, image: AppAssets.shareImage) { [weak self] action in self?.shareDialogForTableCell(indexPath: indexPath, url: url, title: article.title) @@ -935,10 +928,7 @@ private extension MasterTimelineViewController { } func shareAlertAction(_ article: Article, indexPath: IndexPath, completion: @escaping (Bool) -> Void) -> UIAlertAction? { - guard let preferredLink = article.preferredLink, let url = URL(string: preferredLink) else { - return nil - } - + guard let url = article.preferredURL else { return nil } let title = NSLocalizedString("Share", comment: "Share") let action = UIAlertAction(title: title, style: .default) { [weak self] action in completion(true) diff --git a/iOS/SceneCoordinator.swift b/iOS/SceneCoordinator.swift index 87d9ba4d1..cffdff45d 100644 --- a/iOS/SceneCoordinator.swift +++ b/iOS/SceneCoordinator.swift @@ -1238,16 +1238,12 @@ class SceneCoordinator: NSObject, UndoableCommandRunner, UnreadCountProvider { } func showBrowserForArticle(_ article: Article) { - guard let preferredLink = article.preferredLink, let url = URL(string: preferredLink) else { - return - } + guard let url = article.preferredURL else { return } UIApplication.shared.open(url, options: [:]) } func showBrowserForCurrentArticle() { - guard let preferredLink = currentArticle?.preferredLink, let url = URL(string: preferredLink) else { - return - } + guard let url = currentArticle?.preferredURL else { return } UIApplication.shared.open(url, options: [:]) }