From f43e7880f711e0d01fc88010d046d35aaae37e16 Mon Sep 17 00:00:00 2001 From: austinate Date: Mon, 23 Dec 2019 17:20:52 +0200 Subject: [PATCH 001/330] iOS, Add Folder: Make sure Account Name field is in sync with selected account in picker --- iOS/Add/AddFolderViewController.swift | 47 ++++++++++++++++++--------- 1 file changed, 32 insertions(+), 15 deletions(-) diff --git a/iOS/Add/AddFolderViewController.swift b/iOS/Add/AddFolderViewController.swift index 3d2dc0047..26d794a10 100644 --- a/iOS/Add/AddFolderViewController.swift +++ b/iOS/Add/AddFolderViewController.swift @@ -20,7 +20,22 @@ class AddFolderViewController: UITableViewController, AddContainerViewController return accounts.count > 1 } - private var accounts: [Account]! + private var accounts: [Account]! { + didSet { + if let predefinedAccount = accounts.first(where: { $0.accountID == AppDefaults.addFolderAccountID }) { + selectedAccount = predefinedAccount + } else { + selectedAccount = accounts[0] + } + } + } + + private var selectedAccount: Account! { + didSet { + guard selectedAccount != oldValue else { return } + accountLabel.text = selectedAccount.flatMap { ($0 as DisplayNameProvider).nameForDisplay } + } + } weak var delegate: AddContainerViewControllerChildDelegate? @@ -32,13 +47,11 @@ class AddFolderViewController: UITableViewController, AddContainerViewController nameTextField.delegate = self - accountLabel.text = (accounts[0] as DisplayNameProvider).nameForDisplay - if shouldDisplayPicker { accountPickerView.dataSource = self accountPickerView.delegate = self - if let index = accounts.firstIndex(where: { $0.accountID == AppDefaults.addFolderAccountID }) { + if let index = accounts.firstIndex(of: selectedAccount) { accountPickerView.selectRow(index, inComponent: 0, animated: false) } @@ -52,21 +65,26 @@ class AddFolderViewController: UITableViewController, AddContainerViewController NotificationCenter.default.addObserver(self, selector: #selector(textDidChange(_:)), name: UITextField.textDidChangeNotification, object: nameTextField) } + + private func didSelect(_ account: Account) { + AppDefaults.addFolderAccountID = account.accountID + selectedAccount = account + } func cancel() { delegate?.processingDidEnd() } func add() { - let account = accounts[accountPickerView.selectedRow(inComponent: 0)] - if let folderName = nameTextField.text { - account.addFolder(folderName) { result in - switch result { - case .success: - self.delegate?.processingDidEnd() - case .failure(let error): - self.presentError(error) - } + guard let folderName = nameTextField.text else { + return + } + selectedAccount.addFolder(folderName) { result in + switch result { + case .success: + self.delegate?.processingDidEnd() + case .failure(let error): + self.presentError(error) } } } @@ -100,8 +118,7 @@ extension AddFolderViewController: UIPickerViewDataSource, UIPickerViewDelegate } func pickerView(_ pickerView: UIPickerView, didSelectRow row: Int, inComponent component: Int) { - accountLabel.text = (accounts[row] as DisplayNameProvider).nameForDisplay - AppDefaults.addFolderAccountID = accounts[row].accountID + didSelect(accounts[row]) } } From 61370dd04d9ad38cc9bd9cb50140adeaa7e84719 Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Tue, 24 Dec 2019 17:34:47 -0700 Subject: [PATCH 002/330] Fix so that we don't try to scroll before the datasource has been fully applied to the table. Issue #1463 --- iOS/MasterTimeline/MasterTimelineViewController.swift | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/iOS/MasterTimeline/MasterTimelineViewController.swift b/iOS/MasterTimeline/MasterTimelineViewController.swift index 3d8ce6479..709e1f117 100644 --- a/iOS/MasterTimeline/MasterTimelineViewController.swift +++ b/iOS/MasterTimeline/MasterTimelineViewController.swift @@ -74,11 +74,12 @@ class MasterTimelineViewController: UITableViewController, UndoableCommandRunner } resetUI(resetScroll: true) - applyChanges(animated: false) - // Restore the scroll position if we have one stored - if let restoreIndexPath = coordinator.timelineMiddleIndexPath { - tableView.scrollToRow(at: restoreIndexPath, at: .middle, animated: false) + // Load the table and then scroll to the saved position if available + applyChanges(animated: false) { + if let restoreIndexPath = self.coordinator.timelineMiddleIndexPath { + self.tableView.scrollToRow(at: restoreIndexPath, at: .middle, animated: false) + } } } From 84d1ccadfdb16a26cebf206ae559052fdb258fcb Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Tue, 24 Dec 2019 17:55:56 -0700 Subject: [PATCH 003/330] Clear the saved middle position when emptying the timeline. Issue #1464 --- iOS/SceneCoordinator.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/iOS/SceneCoordinator.swift b/iOS/SceneCoordinator.swift index 9cb6d5ef1..76955cd20 100644 --- a/iOS/SceneCoordinator.swift +++ b/iOS/SceneCoordinator.swift @@ -1551,6 +1551,7 @@ private extension SceneCoordinator { func emptyTheTimeline() { if !articles.isEmpty { + timelineMiddleIndexPath = nil replaceArticles(with: Set
(), animated: false) } } From 758fb1a1c0fd2ac3cba3ea8c453e7e6d93eafbcf Mon Sep 17 00:00:00 2001 From: Nate Weaver Date: Tue, 24 Dec 2019 20:12:10 -0600 Subject: [PATCH 004/330] Make initial scale 100% and don't let WebKit increase text size Issue #1459. --- iOS/Resources/page.html | 2 +- iOS/Resources/styleSheet.css | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/iOS/Resources/page.html b/iOS/Resources/page.html index 9d60b7f68..fae517747 100644 --- a/iOS/Resources/page.html +++ b/iOS/Resources/page.html @@ -1,6 +1,6 @@ - + diff --git a/Shared/Article Rendering/ArticleRenderer.swift b/Shared/Article Rendering/ArticleRenderer.swift index e19adec0a..25a510907 100644 --- a/Shared/Article Rendering/ArticleRenderer.swift +++ b/Shared/Article Rendering/ArticleRenderer.swift @@ -13,7 +13,7 @@ import Account struct ArticleRenderer { - typealias Rendering = (style: String, html: String) + typealias Rendering = (style: String, html: String, title: String, baseURL: String) typealias Page = (url: URL, baseURL: URL) static var imageIconScheme = "nnwImageIcon" @@ -49,27 +49,27 @@ struct ArticleRenderer { static func articleHTML(article: Article, extractedArticle: ExtractedArticle? = nil, style: ArticleStyle) -> Rendering { let renderer = ArticleRenderer(article: article, extractedArticle: extractedArticle, style: style) - return (renderer.styleString(), renderer.articleHTML) + return (renderer.styleString(), renderer.articleHTML, renderer.title, renderer.baseURL ?? "") } static func multipleSelectionHTML(style: ArticleStyle) -> Rendering { let renderer = ArticleRenderer(article: nil, extractedArticle: nil, style: style) - return (renderer.styleString(), renderer.multipleSelectionHTML) + return (renderer.styleString(), renderer.multipleSelectionHTML, renderer.title, renderer.baseURL ?? "") } static func loadingHTML(style: ArticleStyle) -> Rendering { let renderer = ArticleRenderer(article: nil, extractedArticle: nil, style: style) - return (renderer.styleString(), renderer.loadingHTML) + return (renderer.styleString(), renderer.loadingHTML, renderer.title, renderer.baseURL ?? "") } static func noSelectionHTML(style: ArticleStyle) -> Rendering { let renderer = ArticleRenderer(article: nil, extractedArticle: nil, style: style) - return (renderer.styleString(), renderer.noSelectionHTML) + return (renderer.styleString(), renderer.noSelectionHTML, renderer.title, renderer.baseURL ?? "") } static func noContentHTML(style: ArticleStyle) -> Rendering { let renderer = ArticleRenderer(article: nil, extractedArticle: nil, style: style) - return (renderer.styleString(), renderer.noContentHTML) + return (renderer.styleString(), renderer.noContentHTML, renderer.title, renderer.baseURL ?? "") } } @@ -79,26 +79,26 @@ private extension ArticleRenderer { private var articleHTML: String { let body = try! MacroProcessor.renderedText(withTemplate: template(), substitutions: articleSubstitutions()) - return renderHTML(withBody: body) + return body } private var multipleSelectionHTML: String { let body = "

Multiple selection

" - return renderHTML(withBody: body) + return body } private var loadingHTML: String { let body = "

Loading...

" - return renderHTML(withBody: body) + return body } private var noSelectionHTML: String { let body = "

No selection

" - return renderHTML(withBody: body) + return body } private var noContentHTML: String { - return renderHTML(withBody: "") + return "" } static var defaultStyleSheet: String = { @@ -225,17 +225,6 @@ private extension ArticleRenderer { return dateFormatter.string(from: date) } - func renderHTML(withBody body: String) -> String { - var s = "" - if let baseURL = baseURL { - s += ("") - } - s += title.htmlBySurroundingWithTag("title") - - s += body - return s - } - } // MARK: - Article extension diff --git a/Shared/Article Rendering/main.js b/Shared/Article Rendering/main.js index ddfcf3e18..c99a7ece4 100644 --- a/Shared/Article Rendering/main.js +++ b/Shared/Article Rendering/main.js @@ -90,6 +90,13 @@ function error() { function render(data, scrollY) { document.getElementsByTagName("style")[0].innerHTML = data.style; + + let title = document.getElementsByTagName("title")[0]; + title.textContent = data.title + + let base = document.getElementsByTagName("base")[0]; + base.href = data.baseURL + document.body.innerHTML = data.body; window.scrollTo(0, scrollY); diff --git a/iOS/Article/WebViewController.swift b/iOS/Article/WebViewController.swift index b85f1dd40..71a7c37a1 100644 --- a/iOS/Article/WebViewController.swift +++ b/iOS/Article/WebViewController.swift @@ -437,6 +437,8 @@ extension WebViewController: UIScrollViewDelegate { private struct TemplateData: Codable { let style: String let body: String + let title: String + let baseURL: String } private struct ImageClickMessage: Codable { @@ -478,7 +480,7 @@ private extension WebViewController { rendering = ArticleRenderer.noSelectionHTML(style: style) } - let templateData = TemplateData(style: rendering.style, body: rendering.html) + let templateData = TemplateData(style: rendering.style, body: rendering.html, title: rendering.title, baseURL: rendering.baseURL) let encoder = JSONEncoder() var render = "error();" diff --git a/iOS/Resources/page.html b/iOS/Resources/page.html index fae517747..f130430ea 100644 --- a/iOS/Resources/page.html +++ b/iOS/Resources/page.html @@ -1,5 +1,7 @@ + + - - + + + - - - + + + diff --git a/iOS/Resources/page.html b/iOS/Resources/page.html index f130430ea..22bc5881c 100644 --- a/iOS/Resources/page.html +++ b/iOS/Resources/page.html @@ -1,17 +1,17 @@ - + - - - + + + - - - + + + From ab8deab7268797c9c81536a80f43a06aa6b81b59 Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Thu, 30 Jan 2020 12:53:27 -0700 Subject: [PATCH 254/330] Correct object ownership to remove retain cycle --- iOS/Article/ArticleIconSchemeHandler.swift | 4 ++-- iOS/Article/WebViewProvider.swift | 4 +--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/iOS/Article/ArticleIconSchemeHandler.swift b/iOS/Article/ArticleIconSchemeHandler.swift index 498986199..2fa681a55 100644 --- a/iOS/Article/ArticleIconSchemeHandler.swift +++ b/iOS/Article/ArticleIconSchemeHandler.swift @@ -12,7 +12,7 @@ import Articles class ArticleIconSchemeHandler: NSObject, WKURLSchemeHandler { - let coordinator: SceneCoordinator + weak var coordinator: SceneCoordinator? init(coordinator: SceneCoordinator) { self.coordinator = coordinator @@ -20,7 +20,7 @@ class ArticleIconSchemeHandler: NSObject, WKURLSchemeHandler { func webView(_ webView: WKWebView, start urlSchemeTask: WKURLSchemeTask) { - guard let url = urlSchemeTask.request.url else { + guard let url = urlSchemeTask.request.url, let coordinator = coordinator else { urlSchemeTask.didFailWithError(URLError(.fileDoesNotExist)) return } diff --git a/iOS/Article/WebViewProvider.swift b/iOS/Article/WebViewProvider.swift index 21ab0bf50..0f02894b5 100644 --- a/iOS/Article/WebViewProvider.swift +++ b/iOS/Article/WebViewProvider.swift @@ -14,7 +14,6 @@ import WebKit class WebViewProvider: NSObject, WKNavigationDelegate { let articleIconSchemeHandler: ArticleIconSchemeHandler - let viewController: UIViewController private let minimumQueueDepth = 3 private let maximumQueueDepth = 6 @@ -25,9 +24,8 @@ class WebViewProvider: NSObject, WKNavigationDelegate { init(coordinator: SceneCoordinator, viewController: UIViewController) { articleIconSchemeHandler = ArticleIconSchemeHandler(coordinator: coordinator) - self.viewController = viewController super.init() - self.viewController.view.insertSubview(queue, at: 0) + viewController.view.insertSubview(queue, at: 0) replenishQueueIfNeeded() } From 6454330c8b140eb92bd9cd00e0de2b586122bcb7 Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Thu, 30 Jan 2020 13:27:17 -0700 Subject: [PATCH 255/330] Add the subviews to the contentVIew (per the documentation). Issue #1717 --- iOS/MasterFeed/Cell/MasterFeedTableViewSectionHeader.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/iOS/MasterFeed/Cell/MasterFeedTableViewSectionHeader.swift b/iOS/MasterFeed/Cell/MasterFeedTableViewSectionHeader.swift index 2088cfcf7..6b0076b48 100644 --- a/iOS/MasterFeed/Cell/MasterFeedTableViewSectionHeader.swift +++ b/iOS/MasterFeed/Cell/MasterFeedTableViewSectionHeader.swift @@ -155,7 +155,7 @@ private extension MasterFeedTableViewSectionHeader { } func addSubviewAtInit(_ view: UIView) { - addSubview(view) + contentView.addSubview(view) view.translatesAutoresizingMaskIntoConstraints = false } From 13a1ef06052750135d33a67e678e177c98ab8e0e Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Thu, 30 Jan 2020 13:44:31 -0700 Subject: [PATCH 256/330] Don't animate the disclosure icon until it has been added to the view hierarchy. Issue #1717 --- .../Cell/MasterFeedTableViewSectionHeader.swift | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/iOS/MasterFeed/Cell/MasterFeedTableViewSectionHeader.swift b/iOS/MasterFeed/Cell/MasterFeedTableViewSectionHeader.swift index 6b0076b48..74323a095 100644 --- a/iOS/MasterFeed/Cell/MasterFeedTableViewSectionHeader.swift +++ b/iOS/MasterFeed/Cell/MasterFeedTableViewSectionHeader.swift @@ -42,7 +42,6 @@ class MasterFeedTableViewSectionHeader: UITableViewHeaderFooterView { set { if titleView.text != newValue { titleView.text = newValue - setNeedsDisplay() setNeedsLayout() } } @@ -50,7 +49,7 @@ class MasterFeedTableViewSectionHeader: UITableViewHeaderFooterView { var disclosureExpanded = false { didSet { - updateExpandedState() + updateExpandedState(animate: true) updateUnreadCountView() } } @@ -116,19 +115,22 @@ private extension MasterFeedTableViewSectionHeader { func commonInit() { addSubviewAtInit(unreadCountView) addSubviewAtInit(titleView) - updateExpandedState() addSubviewAtInit(disclosureView) + updateExpandedState(animate: false) addBackgroundView() addSubviewAtInit(topSeparatorView) addSubviewAtInit(bottomSeparatorView) } - func updateExpandedState() { + func updateExpandedState(animate: Bool) { if !isLastSection && self.disclosureExpanded { self.bottomSeparatorView.isHidden = false } + + let duration = animate ? 0.3 : 0.0 + UIView.animate( - withDuration: 0.3, + withDuration: duration, animations: { if self.disclosureExpanded { self.disclosureView.transform = CGAffineTransform(rotationAngle: 1.570796) From a339b05bf87fa86cbc9a3b5a5bd4fb9dc6c27d29 Mon Sep 17 00:00:00 2001 From: Kiel Gillard Date: Wed, 29 Jan 2020 18:28:37 +1100 Subject: [PATCH 257/330] Santizes right to left div elements from Feedly content. --- .../Account/Account.xcodeproj/project.pbxproj | 28 ++++ .../Feedly/FeedlyCollectionParserTests.swift | 28 ++++ .../Feedly/FeedlyEntryParserTests.swift | 121 ++++++++++++++++++ .../Feedly/FeedlyFeedParserTests.swift | 41 ++++++ .../Feedly/FeedlyTextSanitizationTests.swift | 38 ++++++ .../Models/FeedlyCollectionParser.swift | 23 ++++ .../Feedly/Models/FeedlyEntryParser.swift | 6 +- .../Feedly/Models/FeedlyFeedParser.swift | 32 +++++ .../Models/FeedlyRTLTextSanitizer.swift | 28 ++++ ...teFeedsForCollectionFoldersOperation.swift | 8 +- ...yMirrorCollectionsAsFoldersOperation.swift | 40 +++--- 11 files changed, 365 insertions(+), 28 deletions(-) create mode 100644 Frameworks/Account/AccountTests/Feedly/FeedlyCollectionParserTests.swift create mode 100644 Frameworks/Account/AccountTests/Feedly/FeedlyEntryParserTests.swift create mode 100644 Frameworks/Account/AccountTests/Feedly/FeedlyFeedParserTests.swift create mode 100644 Frameworks/Account/AccountTests/Feedly/FeedlyTextSanitizationTests.swift create mode 100644 Frameworks/Account/Feedly/Models/FeedlyCollectionParser.swift create mode 100644 Frameworks/Account/Feedly/Models/FeedlyFeedParser.swift create mode 100644 Frameworks/Account/Feedly/Models/FeedlyRTLTextSanitizer.swift diff --git a/Frameworks/Account/Account.xcodeproj/project.pbxproj b/Frameworks/Account/Account.xcodeproj/project.pbxproj index 338b98813..62d8daf90 100644 --- a/Frameworks/Account/Account.xcodeproj/project.pbxproj +++ b/Frameworks/Account/Account.xcodeproj/project.pbxproj @@ -111,6 +111,9 @@ 9E44C90F23C6FF3600CCC286 /* FeedlyIngestStreamArticleIdsOperation.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9E44C90E23C6FF3600CCC286 /* FeedlyIngestStreamArticleIdsOperation.swift */; }; 9E5ABE9A236BE6BD00B5DE9F /* feedly-1-initial in Resources */ = {isa = PBXBuildFile; fileRef = 9E5ABE99236BE6BC00B5DE9F /* feedly-1-initial */; }; 9E5DE60E23C3F4B70064DA30 /* FeedlyFetchIdsForMissingArticlesOperation.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9E5DE60D23C3F4B70064DA30 /* FeedlyFetchIdsForMissingArticlesOperation.swift */; }; + 9E5EC15923E01D8A00A4E503 /* FeedlyCollectionParser.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9E5EC15823E01D8A00A4E503 /* FeedlyCollectionParser.swift */; }; + 9E5EC15B23E01DEF00A4E503 /* FeedlyRTLTextSanitizer.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9E5EC15A23E01DEF00A4E503 /* FeedlyRTLTextSanitizer.swift */; }; + 9E5EC15D23E0D58500A4E503 /* FeedlyFeedParser.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9E5EC15C23E0D58500A4E503 /* FeedlyFeedParser.swift */; }; 9E672394236F7CA0000BE141 /* FeedlyRefreshAccessTokenOperation.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9E672393236F7CA0000BE141 /* FeedlyRefreshAccessTokenOperation.swift */; }; 9E672396236F7E68000BE141 /* OAuthAcessTokenRefreshing.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9E672395236F7E68000BE141 /* OAuthAcessTokenRefreshing.swift */; }; 9E7299D723505E9600DAEFB7 /* FeedlyAddFeedToCollectionOperation.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9E7299D623505E9600DAEFB7 /* FeedlyAddFeedToCollectionOperation.swift */; }; @@ -164,6 +167,10 @@ 9EF1B10723590D61000A486A /* FeedlyGetStreamIdsOperation.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9EF1B10623590D61000A486A /* FeedlyGetStreamIdsOperation.swift */; }; 9EF1B10923590E93000A486A /* FeedlyStreamIds.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9EF1B10823590E93000A486A /* FeedlyStreamIds.swift */; }; 9EF2602C23C91FFE006D160C /* FeedlyGetUpdatedArticleIdsOperation.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9EF2602B23C91FFE006D160C /* FeedlyGetUpdatedArticleIdsOperation.swift */; }; + 9EF58EB023E1606000992A2B /* FeedlyTextSanitizationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9EF58EAF23E1606000992A2B /* FeedlyTextSanitizationTests.swift */; }; + 9EF58EB223E1647400992A2B /* FeedlyCollectionParserTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9EF58EB123E1647400992A2B /* FeedlyCollectionParserTests.swift */; }; + 9EF58EB423E1655300992A2B /* FeedlyFeedParserTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9EF58EB323E1655300992A2B /* FeedlyFeedParserTests.swift */; }; + 9EF58EB623E1669F00992A2B /* FeedlyEntryParserTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9EF58EB523E1669F00992A2B /* FeedlyEntryParserTests.swift */; }; /* End PBXBuildFile section */ /* Begin PBXContainerItemProxy section */ @@ -327,6 +334,9 @@ 9E489E92236101FC004372EE /* FeedlyUpdateAccountFeedsWithItemsOperationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlyUpdateAccountFeedsWithItemsOperationTests.swift; sourceTree = ""; }; 9E5ABE99236BE6BC00B5DE9F /* feedly-1-initial */ = {isa = PBXFileReference; lastKnownFileType = folder; path = "feedly-1-initial"; sourceTree = ""; }; 9E5DE60D23C3F4B70064DA30 /* FeedlyFetchIdsForMissingArticlesOperation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlyFetchIdsForMissingArticlesOperation.swift; sourceTree = ""; }; + 9E5EC15823E01D8A00A4E503 /* FeedlyCollectionParser.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlyCollectionParser.swift; sourceTree = ""; }; + 9E5EC15A23E01DEF00A4E503 /* FeedlyRTLTextSanitizer.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlyRTLTextSanitizer.swift; sourceTree = ""; }; + 9E5EC15C23E0D58500A4E503 /* FeedlyFeedParser.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlyFeedParser.swift; sourceTree = ""; }; 9E672393236F7CA0000BE141 /* FeedlyRefreshAccessTokenOperation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlyRefreshAccessTokenOperation.swift; sourceTree = ""; }; 9E672395236F7E68000BE141 /* OAuthAcessTokenRefreshing.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OAuthAcessTokenRefreshing.swift; sourceTree = ""; }; 9E7299D623505E9600DAEFB7 /* FeedlyAddFeedToCollectionOperation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlyAddFeedToCollectionOperation.swift; sourceTree = ""; }; @@ -386,6 +396,10 @@ 9EF1B10623590D61000A486A /* FeedlyGetStreamIdsOperation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlyGetStreamIdsOperation.swift; sourceTree = ""; }; 9EF1B10823590E93000A486A /* FeedlyStreamIds.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlyStreamIds.swift; sourceTree = ""; }; 9EF2602B23C91FFE006D160C /* FeedlyGetUpdatedArticleIdsOperation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlyGetUpdatedArticleIdsOperation.swift; sourceTree = ""; }; + 9EF58EAF23E1606000992A2B /* FeedlyTextSanitizationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlyTextSanitizationTests.swift; sourceTree = ""; }; + 9EF58EB123E1647400992A2B /* FeedlyCollectionParserTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlyCollectionParserTests.swift; sourceTree = ""; }; + 9EF58EB323E1655300992A2B /* FeedlyFeedParserTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlyFeedParserTests.swift; sourceTree = ""; }; + 9EF58EB523E1669F00992A2B /* FeedlyEntryParserTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlyEntryParserTests.swift; sourceTree = ""; }; D511EEB5202422BB00712EC3 /* Account_project_debug.xcconfig */ = {isa = PBXFileReference; lastKnownFileType = text.xcconfig; path = Account_project_debug.xcconfig; sourceTree = ""; }; D511EEB6202422BB00712EC3 /* Account_target.xcconfig */ = {isa = PBXFileReference; lastKnownFileType = text.xcconfig; path = Account_target.xcconfig; sourceTree = ""; }; D511EEB7202422BB00712EC3 /* Account_project_release.xcconfig */ = {isa = PBXFileReference; lastKnownFileType = text.xcconfig; path = Account_project_release.xcconfig; sourceTree = ""; }; @@ -652,6 +666,10 @@ 9E0260CA236FF99A00D122D3 /* FeedlyRefreshAccessTokenOperationTests.swift */, 9E784EBF237E8BE100099B1B /* FeedlyLogoutOperationTests.swift */, 9EA643D823945CE00018A28C /* FeedlyAddNewFeedOperationTests.swift */, + 9EF58EAF23E1606000992A2B /* FeedlyTextSanitizationTests.swift */, + 9EF58EB123E1647400992A2B /* FeedlyCollectionParserTests.swift */, + 9EF58EB323E1655300992A2B /* FeedlyFeedParserTests.swift */, + 9EF58EB523E1669F00992A2B /* FeedlyEntryParserTests.swift */, 9E79F7732395C9EF0031DB98 /* feedly-add-new-feed */, 9E5ABE99236BE6BC00B5DE9F /* feedly-1-initial */, 9EC804E4236C1A7F0057CFCB /* feedly-2-changestatuses */, @@ -720,7 +738,9 @@ children = ( 9E1773D823458D590056A5A8 /* FeedlyResourceId.swift */, 9EAEC60B2332FE830085D7C9 /* FeedlyCollection.swift */, + 9E5EC15823E01D8A00A4E503 /* FeedlyCollectionParser.swift */, 9EAEC60D2332FEC20085D7C9 /* FeedlyFeed.swift */, + 9E5EC15C23E0D58500A4E503 /* FeedlyFeedParser.swift */, 9EAEC623233315F60085D7C9 /* FeedlyEntry.swift */, 9E1773D4234570E30056A5A8 /* FeedlyEntryParser.swift */, 9EAEC625233318400085D7C9 /* FeedlyStream.swift */, @@ -731,6 +751,7 @@ 9E1773D6234575AB0056A5A8 /* FeedlyTag.swift */, 9EA643D4239306AC0018A28C /* FeedlyFeedsSearchResponse.swift */, 9EBD49C123C67784005AD5CD /* FeedlyEntryIdentifierProviding.swift */, + 9E5EC15A23E01DEF00A4E503 /* FeedlyRTLTextSanitizer.swift */, ); path = Models; sourceTree = ""; @@ -977,10 +998,12 @@ 51E5959B228C781500FCC42B /* FeedbinStarredEntry.swift in Sources */, 846E77451F6EF9B900A165E2 /* Container.swift in Sources */, 9EA643D3239305680018A28C /* FeedlySearchOperation.swift in Sources */, + 9E5EC15D23E0D58500A4E503 /* FeedlyFeedParser.swift in Sources */, 9E1D15532334304B00F4944C /* FeedlyGetStreamContentsOperation.swift in Sources */, 9E12B0202334696A00ADE5A0 /* FeedlyCreateFeedsForCollectionFoldersOperation.swift in Sources */, 552032FD229D5D5A009559E0 /* ReaderAPITagging.swift in Sources */, 9EAEC62A23331EE70085D7C9 /* FeedlyOrigin.swift in Sources */, + 9E5EC15B23E01DEF00A4E503 /* FeedlyRTLTextSanitizer.swift in Sources */, 511B9804237CD4270028BCAA /* FeedIdentifier.swift in Sources */, 84F73CF1202788D90000BCEF /* ArticleFetcher.swift in Sources */, 841974251F6DDCE4006346C4 /* AccountDelegate.swift in Sources */, @@ -995,6 +1018,7 @@ 9EEAE06E235D002D00E3FEE4 /* FeedlyGetCollectionsService.swift in Sources */, 5165D72922835F7A00D9D53D /* FeedSpecifier.swift in Sources */, 9E85C8ED2367020700D0F1F7 /* FeedlyGetEntriesService.swift in Sources */, + 9E5EC15923E01D8A00A4E503 /* FeedlyCollectionParser.swift in Sources */, 9E84DC492359A73600D6E809 /* FeedlyCheckpointOperation.swift in Sources */, 9E85C8EB236700E600D0F1F7 /* FeedlyGetEntriesOperation.swift in Sources */, 9E1D154D233370D800F4944C /* FeedlySyncAllOperation.swift in Sources */, @@ -1081,6 +1105,7 @@ 9EC228572362C7F900766EF8 /* FeedlyCheckpointOperationTests.swift in Sources */, 9E03C122235E62E100FB6D9E /* FeedlyTestSupport.swift in Sources */, 9EAADA1023C93144003A801F /* TestGetEntriesService.swift in Sources */, + 9EF58EB423E1655300992A2B /* FeedlyFeedParserTests.swift in Sources */, 9E1FF8622368219B00834C24 /* TestGetPagedStreamIdsService.swift in Sources */, 9E03C11E235D976500FB6D9E /* FeedlyGetCollectionsOperationTests.swift in Sources */, 9E85C8E62366FED600D0F1F7 /* TestGetStreamContentsService.swift in Sources */, @@ -1088,14 +1113,17 @@ 9E03C11C235D921400FB6D9E /* FeedlyOperationTests.swift in Sources */, 9E1FF8602368216B00834C24 /* TestGetStreamIdsService.swift in Sources */, 9E85C8E82366FF4200D0F1F7 /* TestGetPagedStreamContentsService.swift in Sources */, + 9EF58EB023E1606000992A2B /* FeedlyTextSanitizationTests.swift in Sources */, 5165D7122282080C00D9D53D /* AccountFeedbinFolderContentsSyncTest.swift in Sources */, 51D5875E227F643C00900287 /* AccountFeedbinFolderSyncTest.swift in Sources */, + 9EF58EB623E1669F00992A2B /* FeedlyEntryParserTests.swift in Sources */, 9EC804E3236C18AB0057CFCB /* FeedlySyncAllMockResponseProvider.swift in Sources */, 9E1FF8682368EE4900834C24 /* TestGetCollectionsService.swift in Sources */, 5107A09B227DE49500C7C3C5 /* TestAccountManager.swift in Sources */, 513323082281070D00C30F19 /* AccountFeedbinSyncTest.swift in Sources */, 5107A09D227DE77700C7C3C5 /* TestTransport.swift in Sources */, 9E1773DB234593CF0056A5A8 /* FeedlyResourceIdTests.swift in Sources */, + 9EF58EB223E1647400992A2B /* FeedlyCollectionParserTests.swift in Sources */, ); runOnlyForDeploymentPostprocessing = 0; }; diff --git a/Frameworks/Account/AccountTests/Feedly/FeedlyCollectionParserTests.swift b/Frameworks/Account/AccountTests/Feedly/FeedlyCollectionParserTests.swift new file mode 100644 index 000000000..3c732a821 --- /dev/null +++ b/Frameworks/Account/AccountTests/Feedly/FeedlyCollectionParserTests.swift @@ -0,0 +1,28 @@ +// +// FeedlyCollectionParserTests.swift +// AccountTests +// +// Created by Kiel Gillard on 29/1/20. +// Copyright © 2020 Ranchero Software, LLC. All rights reserved. +// + +import XCTest +@testable import Account + +class FeedlyCollectionParserTests: XCTestCase { + + func testParsing() { + let collection = FeedlyCollection(feeds: [], label: "Test Collection", id: "test/collection/1") + let parser = FeedlyCollectionParser(collection: collection) + XCTAssertEqual(parser.folderName, collection.label) + XCTAssertEqual(parser.externalID, collection.id) + } + + func testSanitization() { + let name = "Test Collection" + let collection = FeedlyCollection(feeds: [], label: "
\(name)
", id: "test/collection/1") + let parser = FeedlyCollectionParser(collection: collection) + XCTAssertEqual(parser.folderName, name) + XCTAssertEqual(parser.externalID, collection.id) + } +} diff --git a/Frameworks/Account/AccountTests/Feedly/FeedlyEntryParserTests.swift b/Frameworks/Account/AccountTests/Feedly/FeedlyEntryParserTests.swift new file mode 100644 index 000000000..e62a18e6b --- /dev/null +++ b/Frameworks/Account/AccountTests/Feedly/FeedlyEntryParserTests.swift @@ -0,0 +1,121 @@ +// +// FeedlyEntryParserTests.swift +// AccountTests +// +// Created by Kiel Gillard on 29/1/20. +// Copyright © 2020 Ranchero Software, LLC. All rights reserved. +// + +import XCTest +@testable import Account + +class FeedlyEntryParserTests: XCTestCase { + + func testParsing() { + let content = FeedlyEntry.Content(content: "Test Content", direction: .leftToRight) + let summary = FeedlyEntry.Content(content: "Test Summary", direction: .leftToRight) + let origin = FeedlyOrigin(title: "Test Feed", streamId: "tests://feeds/1", htmlUrl: nil) + let entry = FeedlyEntry(id: "tests/feeds/1/entries/1", + title: "Test Entry 1", + content: content, + summary: summary, + author: nil, + crawled: .distantPast, + recrawled: Date(timeIntervalSinceReferenceDate: 0), + origin: origin, + canonical: nil, + alternate: nil, + unread: false, + tags: nil, + categories: nil, + enclosure: nil) + + let parser = FeedlyEntryParser(entry: entry) + + XCTAssertEqual(parser.id, entry.id) + XCTAssertEqual(parser.feedUrl, origin.streamId) + XCTAssertEqual(parser.externalUrl, nil) + XCTAssertEqual(parser.title, entry.title) + XCTAssertEqual(parser.contentHMTL, content.content) + XCTAssertEqual(parser.summary, summary.content) + XCTAssertEqual(parser.datePublished, .distantPast) + XCTAssertEqual(parser.dateModified, Date(timeIntervalSinceReferenceDate: 0)) + } + + func testSanitization() { + let content = FeedlyEntry.Content(content: "
Test Content
", direction: .rightToLeft) + let summaryContent = "Test Summary" + let summary = FeedlyEntry.Content(content: "
\(summaryContent)
", direction: .rightToLeft) + let origin = FeedlyOrigin(title: "Test Feed", streamId: "tests://feeds/1", htmlUrl: nil) + let title = "Test Entry 1" + let entry = FeedlyEntry(id: "tests/feeds/1/entries/1", + title: "
\(title)
", + content: content, + summary: summary, + author: nil, + crawled: .distantPast, + recrawled: nil, + origin: origin, + canonical: nil, + alternate: nil, + unread: false, + tags: nil, + categories: nil, + enclosure: nil) + + let parser = FeedlyEntryParser(entry: entry) + + // These should be sanitized + XCTAssertEqual(parser.title, title) + XCTAssertEqual(parser.summary, summaryContent) + + // These should not be sanitized because it is supposed to be HTML content. + XCTAssertEqual(parser.contentHMTL, content.content) + } + + func testLocatesCanonicalExternalUrl() { + let canonicalLink = FeedlyLink(href: "tests://feeds/1/entries/1", type: "text/html") + let alternateLink = FeedlyLink(href: "tests://feeds/1/entries/alternate/1", type: "text/html") + let entry = FeedlyEntry(id: "tests/feeds/1/entries/1", + title: "Test Entry 1", + content: nil, + summary: nil, + author: nil, + crawled: .distantPast, + recrawled: Date(timeIntervalSinceReferenceDate: 0), + origin: nil, + canonical: [canonicalLink], + alternate: [alternateLink], + unread: false, + tags: nil, + categories: nil, + enclosure: nil) + + let parser = FeedlyEntryParser(entry: entry) + + XCTAssertEqual(parser.externalUrl, canonicalLink.href) + } + + func testLocatesAlternateExternalUrl() { + let canonicalLink = FeedlyLink(href: "tests://feeds/1/entries/1", type: "text/json") + let alternateLink = FeedlyLink(href: "tests://feeds/1/entries/alternate/1", type: nil) + let entry = FeedlyEntry(id: "tests/feeds/1/entries/1", + title: "Test Entry 1", + content: nil, + summary: nil, + author: nil, + crawled: .distantPast, + recrawled: Date(timeIntervalSinceReferenceDate: 0), + origin: nil, + canonical: [canonicalLink], + alternate: [alternateLink], + unread: false, + tags: nil, + categories: nil, + enclosure: nil) + + let parser = FeedlyEntryParser(entry: entry) + + XCTAssertEqual(parser.externalUrl, alternateLink.href) + } +} diff --git a/Frameworks/Account/AccountTests/Feedly/FeedlyFeedParserTests.swift b/Frameworks/Account/AccountTests/Feedly/FeedlyFeedParserTests.swift new file mode 100644 index 000000000..8ddd4aaec --- /dev/null +++ b/Frameworks/Account/AccountTests/Feedly/FeedlyFeedParserTests.swift @@ -0,0 +1,41 @@ +// +// FeedlyFeedParserTests.swift +// AccountTests +// +// Created by Kiel Gillard on 29/1/20. +// Copyright © 2020 Ranchero Software, LLC. All rights reserved. +// + +import XCTest +@testable import Account + +class FeedlyFeedParserTests: XCTestCase { + + func testParsing() { + let name = "Test Feed" + let website = "tests://nnw/feed/1" + let url = "tests://nnw/feed.xml" + let id = "feed/\(url)" + let updated = Date.distantPast + let feed = FeedlyFeed(id: id, title: name, updated: updated, website: website) + let parser = FeedlyFeedParser(feed: feed) + XCTAssertEqual(parser.title, name) + XCTAssertEqual(parser.homePageURL, website) + XCTAssertEqual(parser.url, url) + XCTAssertEqual(parser.webFeedID, id) + } + + func testSanitization() { + let name = "Test Feed" + let website = "tests://nnw/feed/1" + let url = "tests://nnw/feed.xml" + let id = "feed/\(url)" + let updated = Date.distantPast + let feed = FeedlyFeed(id: id, title: "
\(name)
", updated: updated, website: website) + let parser = FeedlyFeedParser(feed: feed) + XCTAssertEqual(parser.title, name) + XCTAssertEqual(parser.homePageURL, website) + XCTAssertEqual(parser.url, url) + XCTAssertEqual(parser.webFeedID, id) + } +} diff --git a/Frameworks/Account/AccountTests/Feedly/FeedlyTextSanitizationTests.swift b/Frameworks/Account/AccountTests/Feedly/FeedlyTextSanitizationTests.swift new file mode 100644 index 000000000..ae9eefba0 --- /dev/null +++ b/Frameworks/Account/AccountTests/Feedly/FeedlyTextSanitizationTests.swift @@ -0,0 +1,38 @@ +// +// FeedlyTextSanitizationTests.swift +// AccountTests +// +// Created by Kiel Gillard on 29/1/20. +// Copyright © 2020 Ranchero Software, LLC. All rights reserved. +// + +import XCTest +@testable import Account + +class FeedlyTextSanitizationTests: XCTestCase { + + func testRTLSanitization() { + + let targetsAndExpectations: [(target: String?, expectation: String?)] = [ + (nil, nil), + ("", ""), + (" ", " "), + ("text", "text"), + ("
", "
"), + ("
", "
"), + ("
text", "
text"), + ("text
", "text
"), + ("
", ""), + ("
", "
"), + ("
", "
"), + ("
text
", "text"), + ] + + let sanitizer = FeedlyRTLTextSanitizer() + + for (target, expectation) in targetsAndExpectations { + let calculated = sanitizer.sanitize(target) + XCTAssertEqual(expectation, calculated) + } + } +} diff --git a/Frameworks/Account/Feedly/Models/FeedlyCollectionParser.swift b/Frameworks/Account/Feedly/Models/FeedlyCollectionParser.swift new file mode 100644 index 000000000..229498592 --- /dev/null +++ b/Frameworks/Account/Feedly/Models/FeedlyCollectionParser.swift @@ -0,0 +1,23 @@ +// +// FeedlyCollectionParser.swift +// Account +// +// Created by Kiel Gillard on 28/1/20. +// Copyright © 2020 Ranchero Software, LLC. All rights reserved. +// + +import Foundation + +struct FeedlyCollectionParser { + let collection: FeedlyCollection + + private let rightToLeftTextSantizer = FeedlyRTLTextSanitizer() + + var folderName: String { + return rightToLeftTextSantizer.sanitize(collection.label) ?? "" + } + + var externalID: String { + return collection.id + } +} diff --git a/Frameworks/Account/Feedly/Models/FeedlyEntryParser.swift b/Frameworks/Account/Feedly/Models/FeedlyEntryParser.swift index 06b287806..2e71b3ab8 100644 --- a/Frameworks/Account/Feedly/Models/FeedlyEntryParser.swift +++ b/Frameworks/Account/Feedly/Models/FeedlyEntryParser.swift @@ -13,6 +13,8 @@ import RSParser struct FeedlyEntryParser { let entry: FeedlyEntry + private let rightToLeftTextSantizer = FeedlyRTLTextSanitizer() + var id: String { return entry.id } @@ -36,7 +38,7 @@ struct FeedlyEntryParser { } var title: String? { - return entry.title + return rightToLeftTextSantizer.sanitize(entry.title) } var contentHMTL: String? { @@ -49,7 +51,7 @@ struct FeedlyEntryParser { } var summary: String? { - return entry.summary?.content + return rightToLeftTextSantizer.sanitize(entry.summary?.content) } var datePublished: Date { diff --git a/Frameworks/Account/Feedly/Models/FeedlyFeedParser.swift b/Frameworks/Account/Feedly/Models/FeedlyFeedParser.swift new file mode 100644 index 000000000..e2f288dce --- /dev/null +++ b/Frameworks/Account/Feedly/Models/FeedlyFeedParser.swift @@ -0,0 +1,32 @@ +// +// FeedlyFeedParser.swift +// Account +// +// Created by Kiel Gillard on 29/1/20. +// Copyright © 2020 Ranchero Software, LLC. All rights reserved. +// + +import Foundation + +struct FeedlyFeedParser { + let feed: FeedlyFeed + + private let rightToLeftTextSantizer = FeedlyRTLTextSanitizer() + + var title: String? { + return rightToLeftTextSantizer.sanitize(feed.title) ?? "" + } + + var webFeedID: String { + return feed.id + } + + var url: String { + let resource = FeedlyFeedResourceId(id: feed.id) + return resource.url + } + + var homePageURL: String? { + return feed.website + } +} diff --git a/Frameworks/Account/Feedly/Models/FeedlyRTLTextSanitizer.swift b/Frameworks/Account/Feedly/Models/FeedlyRTLTextSanitizer.swift new file mode 100644 index 000000000..11eb2dcb5 --- /dev/null +++ b/Frameworks/Account/Feedly/Models/FeedlyRTLTextSanitizer.swift @@ -0,0 +1,28 @@ +// +// FeedlyRTLTextSanitizer.swift +// Account +// +// Created by Kiel Gillard on 28/1/20. +// Copyright © 2020 Ranchero Software, LLC. All rights reserved. +// + +import Foundation + +struct FeedlyRTLTextSanitizer { + private let rightToLeftPrefix = "
" + private let rightToLeftSuffix = "
" + + func sanitize(_ sourceText: String?) -> String? { + guard let source = sourceText, !source.isEmpty else { + return sourceText + } + + guard source.hasPrefix(rightToLeftPrefix) && source.hasSuffix(rightToLeftSuffix) else { + return source + } + + let start = source.index(source.startIndex, offsetBy: rightToLeftPrefix.indices.count) + let end = source.index(source.endIndex, offsetBy: -rightToLeftSuffix.indices.count) + return String(source[start.. (FeedlyCollection, Folder)? in - guard let folder = account.ensureFolder(with: collection.label) else { + feedsAndFolders = collections.compactMap { collection -> ([FeedlyFeed], Folder)? in + let parser = FeedlyCollectionParser(collection: collection) + guard let folder = account.ensureFolder(with: parser.folderName) else { assertionFailure("Why wasn't a folder created?") return nil } - folder.externalID = collection.id - return (collection, folder) - } - - collectionsAndFolders = pairs - os_log(.debug, log: log, "Ensured %i folders for %i collections.", pairs.count, collections.count) - - feedsAndFolders = pairs.map { (collection, folder) -> (([FeedlyFeed], Folder)) in + folder.externalID = parser.externalID return (collection.feeds, folder) } - // Remove folders without a corresponding collection - let collectionFolders = Set(pairs.map { $0.1 }) - let foldersWithoutCollections = localFolders.subtracting(collectionFolders) - for unmatched in foldersWithoutCollections { - account.removeFolder(unmatched) - } + os_log(.debug, log: log, "Ensured %i folders for %i collections.", feedsAndFolders.count, collections.count) - os_log(.debug, log: log, "Removed %i folders: %@", foldersWithoutCollections.count, foldersWithoutCollections.map { $0.externalID ?? $0.nameForDisplay }) + // Remove folders without a corresponding collection + let collectionFolders = Set(feedsAndFolders.map { $0.1 }) + let foldersWithoutCollections = localFolders.subtracting(collectionFolders) + + if !foldersWithoutCollections.isEmpty { + for unmatched in foldersWithoutCollections { + account.removeFolder(unmatched) + } + + os_log(.debug, log: log, "Removed %i folders: %@", foldersWithoutCollections.count, foldersWithoutCollections.map { $0.externalID ?? $0.nameForDisplay }) + } } } From e5e7163f8ea44b855b334e4068e27f237e980bdd Mon Sep 17 00:00:00 2001 From: Kiel Gillard Date: Fri, 31 Jan 2020 07:52:30 +1100 Subject: [PATCH 258/330] Improve the test coverage and add some documentation. --- .../Feedly/FeedlyEntryParserTests.swift | 83 ++++++++++++++++++- .../Feedly/Models/FeedlyEntryParser.swift | 5 +- 2 files changed, 83 insertions(+), 5 deletions(-) diff --git a/Frameworks/Account/AccountTests/Feedly/FeedlyEntryParserTests.swift b/Frameworks/Account/AccountTests/Feedly/FeedlyEntryParserTests.swift index e62a18e6b..1329b4339 100644 --- a/Frameworks/Account/AccountTests/Feedly/FeedlyEntryParserTests.swift +++ b/Frameworks/Account/AccountTests/Feedly/FeedlyEntryParserTests.swift @@ -15,18 +15,23 @@ class FeedlyEntryParserTests: XCTestCase { let content = FeedlyEntry.Content(content: "Test Content", direction: .leftToRight) let summary = FeedlyEntry.Content(content: "Test Summary", direction: .leftToRight) let origin = FeedlyOrigin(title: "Test Feed", streamId: "tests://feeds/1", htmlUrl: nil) + let canonicalLink = FeedlyLink(href: "tests://feeds/1/entries/1", type: "text/html") + let tags = [ + FeedlyTag(id: "tests/tags/1", label: "Tag 1"), + FeedlyTag(id: "tests/tags/2", label: "Tag 2") + ] let entry = FeedlyEntry(id: "tests/feeds/1/entries/1", title: "Test Entry 1", content: content, summary: summary, - author: nil, + author: "Bob Alice", crawled: .distantPast, recrawled: Date(timeIntervalSinceReferenceDate: 0), origin: origin, - canonical: nil, + canonical: [canonicalLink], alternate: nil, unread: false, - tags: nil, + tags: tags, categories: nil, enclosure: nil) @@ -34,12 +39,37 @@ class FeedlyEntryParserTests: XCTestCase { XCTAssertEqual(parser.id, entry.id) XCTAssertEqual(parser.feedUrl, origin.streamId) - XCTAssertEqual(parser.externalUrl, nil) + XCTAssertEqual(parser.externalUrl, canonicalLink.href) XCTAssertEqual(parser.title, entry.title) XCTAssertEqual(parser.contentHMTL, content.content) XCTAssertEqual(parser.summary, summary.content) XCTAssertEqual(parser.datePublished, .distantPast) XCTAssertEqual(parser.dateModified, Date(timeIntervalSinceReferenceDate: 0)) + + guard let item = parser.parsedItemRepresentation else { + XCTFail("Expected a parsed item representation.") + return + } + + XCTAssertEqual(item.syncServiceID, entry.id) + XCTAssertEqual(item.uniqueID, entry.id) + + // The following is not an error. + // The feedURL must match the webFeedID for the article to be connected to its matching feed. + XCTAssertEqual(item.feedURL, origin.streamId) + XCTAssertEqual(item.title, entry.title) + XCTAssertEqual(item.contentHTML, content.content) + XCTAssertEqual(item.contentText, nil, "Is it now free of HTML characters?") + XCTAssertEqual(item.summary, summary.content) + XCTAssertEqual(item.datePublished, entry.crawled) + XCTAssertEqual(item.dateModified, entry.recrawled) + + let expectedTags = Set(tags.compactMap { $0.label }) + XCTAssertEqual(item.tags, expectedTags) + + let expectedAuthors = Set([entry.author]) + let calculatedAuthors = Set(item.authors?.compactMap { $0.name } ?? []) + XCTAssertEqual(calculatedAuthors, expectedAuthors) } func testSanitization() { @@ -118,4 +148,49 @@ class FeedlyEntryParserTests: XCTestCase { XCTAssertEqual(parser.externalUrl, alternateLink.href) } + + func testContentPreferredToSummary() { + let content = FeedlyEntry.Content(content: "Test Content", direction: .leftToRight) + let summary = FeedlyEntry.Content(content: "Test Summary", direction: .leftToRight) + let entry = FeedlyEntry(id: "tests/feeds/1/entries/1", + title: "Test Entry 1", + content: content, + summary: summary, + author: nil, + crawled: .distantPast, + recrawled: Date(timeIntervalSinceReferenceDate: 0), + origin: nil, + canonical: nil, + alternate: nil, + unread: false, + tags: nil, + categories: nil, + enclosure: nil) + + let parser = FeedlyEntryParser(entry: entry) + + XCTAssertEqual(parser.contentHMTL, content.content) + } + + func testSummaryUsedAsContentWhenContentMissing() { + let summary = FeedlyEntry.Content(content: "Test Summary", direction: .leftToRight) + let entry = FeedlyEntry(id: "tests/feeds/1/entries/1", + title: "Test Entry 1", + content: nil, + summary: summary, + author: nil, + crawled: .distantPast, + recrawled: Date(timeIntervalSinceReferenceDate: 0), + origin: nil, + canonical: nil, + alternate: nil, + unread: false, + tags: nil, + categories: nil, + enclosure: nil) + + let parser = FeedlyEntryParser(entry: entry) + + XCTAssertEqual(parser.contentHMTL, summary.content) + } } diff --git a/Frameworks/Account/Feedly/Models/FeedlyEntryParser.swift b/Frameworks/Account/Feedly/Models/FeedlyEntryParser.swift index 2e71b3ab8..8af6714c7 100644 --- a/Frameworks/Account/Feedly/Models/FeedlyEntryParser.swift +++ b/Frameworks/Account/Feedly/Models/FeedlyEntryParser.swift @@ -19,9 +19,11 @@ struct FeedlyEntryParser { return entry.id } + /// When ingesting articles, the feedURL must match a feed's `webFeedID` for the article to be reachable between it and its matching feed. It reminds me of a foreign key. var feedUrl: String? { guard let id = entry.origin?.streamId else { - assertionFailure() + // At this point, check Feedly's API isn't glitching or the response has not changed structure. + assertionFailure("Entries need to be traceable to a feed or this entry will be dropped.") return nil } return id @@ -69,6 +71,7 @@ struct FeedlyEntryParser { return Set([ParsedAuthor(name: name, url: nil, avatarURL: nil, emailAddress: nil)]) } + /// While there is not yet a tagging interface, articles can still be searched for by tags. var tags: Set? { guard let labels = entry.tags?.compactMap({ $0.label }), !labels.isEmpty else { return nil From f9dea333756eb1e856645c470e4ce61dea2a16b1 Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Thu, 30 Jan 2020 13:59:15 -0700 Subject: [PATCH 259/330] Remove force unwraps of url and container. Issue #1721 --- iOS/ShareExtension/ShareViewController.swift | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/iOS/ShareExtension/ShareViewController.swift b/iOS/ShareExtension/ShareViewController.swift index 6a529124d..6b5bc55f1 100644 --- a/iOS/ShareExtension/ShareViewController.swift +++ b/iOS/ShareExtension/ShareViewController.swift @@ -95,6 +95,12 @@ class ShareViewController: SLComposeServiceViewController, ShareFolderPickerCont } override func didSelectPost() { + + guard let url = url, let container = container else { + self.extensionContext!.completeRequest(returningItems: [], completionHandler: nil) + return + } + var account: Account? if let containerAccount = container as? Account { account = containerAccount @@ -105,7 +111,7 @@ class ShareViewController: SLComposeServiceViewController, ShareFolderPickerCont return } - if let urlString = url?.absoluteString, account!.hasWebFeed(withURL: urlString) { + if account!.hasWebFeed(withURL: url.absoluteString) { let errorTitle = NSLocalizedString("Error", comment: "Error") presentError(title: errorTitle, message: AccountError.createErrorAlreadySubscribed.localizedDescription) self.extensionContext!.cancelRequest(withError: AccountError.createErrorAlreadySubscribed) @@ -118,7 +124,7 @@ class ShareViewController: SLComposeServiceViewController, ShareFolderPickerCont guard !expired else { return } DispatchQueue.main.async { - account!.createWebFeed(url: self.url!.absoluteString, name: feedName, container: self.container!) { result in + account!.createWebFeed(url: url.absoluteString, name: feedName, container: container) { result in account!.save() AccountManager.shared.suspendNetworkAll() AccountManager.shared.suspendDatabaseAll() From 63e15d78c2ae812f75837d659a6434c391f2e207 Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Thu, 30 Jan 2020 17:39:21 -0700 Subject: [PATCH 260/330] Fix max size phone landscape layout problems. Issue #1743 --- iOS/MasterFeed/Cell/MasterFeedTableViewSectionHeader.swift | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/iOS/MasterFeed/Cell/MasterFeedTableViewSectionHeader.swift b/iOS/MasterFeed/Cell/MasterFeedTableViewSectionHeader.swift index 74323a095..0227184bf 100644 --- a/iOS/MasterFeed/Cell/MasterFeedTableViewSectionHeader.swift +++ b/iOS/MasterFeed/Cell/MasterFeedTableViewSectionHeader.swift @@ -104,7 +104,10 @@ class MasterFeedTableViewSectionHeader: UITableViewHeaderFooterView { override func layoutSubviews() { super.layoutSubviews() - let layout = MasterFeedTableViewSectionHeaderLayout(cellWidth: bounds.size.width, insets: safeAreaInsets, label: titleView, unreadCountView: unreadCountView) + let layout = MasterFeedTableViewSectionHeaderLayout(cellWidth: contentView.bounds.size.width, + insets: contentView.safeAreaInsets, + label: titleView, + unreadCountView: unreadCountView) layoutWith(layout) } From df3c78b1c67c79b1a3217e7dda87aafbee7e751b Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Thu, 30 Jan 2020 18:37:22 -0700 Subject: [PATCH 261/330] Change next/prev feed searches so that they skip collapsed sections. Issue#1755 --- iOS/SceneCoordinator.swift | 45 ++++++++++++++------------------------ 1 file changed, 16 insertions(+), 29 deletions(-) diff --git a/iOS/SceneCoordinator.swift b/iOS/SceneCoordinator.swift index 4daa24072..b09d74094 100644 --- a/iOS/SceneCoordinator.swift +++ b/iOS/SceneCoordinator.swift @@ -161,37 +161,19 @@ class SceneCoordinator: NSObject, UndoableCommandRunner, UnreadCountProvider { private(set) var showFeedNames = false private(set) var showIcons = false - var isPrevFeedAvailable: Bool { - guard let indexPath = currentFeedIndexPath else { - return false - } - return indexPath.section > 0 || indexPath.row > 0 - } - - var isNextFeedAvailable: Bool { - guard let indexPath = currentFeedIndexPath else { - return false - } - - let nextIndexPath: IndexPath = { - if indexPath.row + 1 >= shadowTable[indexPath.section].count { - return IndexPath(row: 0, section: indexPath.section + 1) - } else { - return IndexPath(row: indexPath.row + 1, section: indexPath.section) - } - }() - - return nextIndexPath.section < shadowTable.count && nextIndexPath.row < shadowTable[nextIndexPath.section].count - } - var prevFeedIndexPath: IndexPath? { - guard isPrevFeedAvailable, let indexPath = currentFeedIndexPath else { + guard let indexPath = currentFeedIndexPath else { return nil } - let prevIndexPath: IndexPath = { + let prevIndexPath: IndexPath? = { if indexPath.row - 1 < 0 { - return IndexPath(row: shadowTable[indexPath.section - 1].count - 1, section: indexPath.section - 1) + for i in (0.. 0 { + return IndexPath(row: shadowTable[i].count - 1, section: i) + } + } + return nil } else { return IndexPath(row: indexPath.row - 1, section: indexPath.section) } @@ -201,13 +183,18 @@ class SceneCoordinator: NSObject, UndoableCommandRunner, UnreadCountProvider { } var nextFeedIndexPath: IndexPath? { - guard isNextFeedAvailable, let indexPath = currentFeedIndexPath else { + guard let indexPath = currentFeedIndexPath else { return nil } - let nextIndexPath: IndexPath = { + let nextIndexPath: IndexPath? = { if indexPath.row + 1 >= shadowTable[indexPath.section].count { - return IndexPath(row: 0, section: indexPath.section + 1) + for i in indexPath.section + 1.. 0 { + return IndexPath(row: 0, section: i) + } + } + return nil } else { return IndexPath(row: indexPath.row + 1, section: indexPath.section) } From a0f429448f31c77762122ffc6b9218ad43ff38c8 Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Thu, 30 Jan 2020 18:48:51 -0700 Subject: [PATCH 262/330] Call completions immediately if the previous WebViewController was deallocated because the iPad was rotated. Issue #1757 --- iOS/Article/ImageTransition.swift | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/iOS/Article/ImageTransition.swift b/iOS/Article/ImageTransition.swift index 0608b3c7b..25951b301 100644 --- a/iOS/Article/ImageTransition.swift +++ b/iOS/Article/ImageTransition.swift @@ -93,11 +93,16 @@ class ImageTransition: NSObject, UIViewControllerAnimatedTransitioning { animations: { imageView.frame = self.originFrame }, completion: { _ in - self.webViewController?.showClickedImage() { - DispatchQueue.main.asyncAfter(deadline: .now() + 0.1) { - imageView.removeFromSuperview() - transitionContext.completeTransition(true) + if let controller = self.webViewController { + controller.showClickedImage() { + DispatchQueue.main.asyncAfter(deadline: .now() + 0.1) { + imageView.removeFromSuperview() + transitionContext.completeTransition(true) + } } + } else { + imageView.removeFromSuperview() + transitionContext.completeTransition(true) } }) } From b87882a91967ae208efaf8cf3998f986c9346a04 Mon Sep 17 00:00:00 2001 From: Kiel Gillard Date: Fri, 31 Jan 2020 17:00:30 +1100 Subject: [PATCH 263/330] Log failures attempting to add new feeds to a Feedly account to help debug #1691 --- .../Feedly/Operations/FeedlyAddNewFeedOperation.swift | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Frameworks/Account/Feedly/Operations/FeedlyAddNewFeedOperation.swift b/Frameworks/Account/Feedly/Operations/FeedlyAddNewFeedOperation.swift index 62edf7a54..642b8b37c 100644 --- a/Frameworks/Account/Feedly/Operations/FeedlyAddNewFeedOperation.swift +++ b/Frameworks/Account/Feedly/Operations/FeedlyAddNewFeedOperation.swift @@ -84,6 +84,7 @@ class FeedlyAddNewFeedOperation: FeedlyOperation, FeedlyOperationDelegate, Feedl operationQueue.addOperation(addRequest) let createFeeds = FeedlyCreateFeedsForCollectionFoldersOperation(account: account, feedsAndFoldersProvider: addRequest, log: log) + createFeeds.delegate = self createFeeds.addDependency(addRequest) createFeeds.downloadProgress = downloadProgress operationQueue.addOperation(createFeeds) @@ -91,17 +92,20 @@ class FeedlyAddNewFeedOperation: FeedlyOperation, FeedlyOperationDelegate, Feedl let syncUnread = FeedlyIngestUnreadArticleIdsOperation(account: account, credentials: credentials, service: syncUnreadIdsService, database: database, newerThan: nil, log: log) syncUnread.addDependency(createFeeds) syncUnread.downloadProgress = downloadProgress + syncUnread.delegate = self operationQueue.addOperation(syncUnread) let syncFeed = FeedlySyncStreamContentsOperation(account: account, resource: feedResourceId, service: getStreamContentsService, isPagingEnabled: false, newerThan: nil, log: log) syncFeed.addDependency(syncUnread) syncFeed.downloadProgress = downloadProgress + syncFeed.delegate = self operationQueue.addOperation(syncFeed) let finishOperation = FeedlyCheckpointOperation() finishOperation.checkpointDelegate = self finishOperation.downloadProgress = downloadProgress finishOperation.addDependency(syncFeed) + finishOperation.delegate = self operationQueue.addOperation(finishOperation) } @@ -109,6 +113,8 @@ class FeedlyAddNewFeedOperation: FeedlyOperation, FeedlyOperationDelegate, Feedl addCompletionHandler?(.failure(error)) addCompletionHandler = nil + os_log(.debug, log: log, "Unable to add new feed: %{public}@.", error as NSError) + cancel() } From 8cb28b63b8f5c3d090fa9bb336c8913e9dd13001 Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Fri, 31 Jan 2020 21:07:08 -0700 Subject: [PATCH 264/330] Always swap out the web view to prevent white flashing between articles and make loading faster. --- iOS/Article/WebViewController.swift | 129 ++++++++++++++++------------ 1 file changed, 72 insertions(+), 57 deletions(-) diff --git a/iOS/Article/WebViewController.swift b/iOS/Article/WebViewController.swift index 71a7c37a1..52506356e 100644 --- a/iOS/Article/WebViewController.swift +++ b/iOS/Article/WebViewController.swift @@ -29,7 +29,10 @@ class WebViewController: UIViewController { private var topShowBarsViewConstraint: NSLayoutConstraint! private var bottomShowBarsViewConstraint: NSLayoutConstraint! - private var webView: WKWebView! + private var webView: WKWebView? { + return view.subviews[0] as? WKWebView + } + private lazy var contextMenuInteraction = UIContextMenuInteraction(delegate: self) private var isFullScreenAvailable: Bool { return traitCollection.userInterfaceIdiom == .phone && coordinator.isRootSplitCollapsed @@ -46,7 +49,7 @@ class WebViewController: UIViewController { var isShowingExtractedArticle = false { didSet { if isShowingExtractedArticle != oldValue { - reloadHTML() + loadWebView() } } } @@ -68,25 +71,18 @@ class WebViewController: UIViewController { } if article != oldValue { windowScrollY = 0 - reloadHTML() + loadWebView() } } } let scrollPositionQueue = CoalescingQueue(name: "Article Scroll Position", interval: 0.3, maxInterval: 1.0) - var windowScrollY = 0 + var windowScrollY = 0 deinit { - if webView != nil { - webView?.evaluateJavaScript("cancelImageLoad();") - webView.configuration.userContentController.removeScriptMessageHandler(forName: MessageName.imageWasClicked) - webView.configuration.userContentController.removeScriptMessageHandler(forName: MessageName.imageWasShown) - webView.removeFromSuperview() - coordinator.webViewProvider.enqueueWebView(webView) - webView = nil - } + recycleWebView(webView) } - + override func viewDidLoad() { super.viewDidLoad() @@ -94,44 +90,17 @@ class WebViewController: UIViewController { NotificationCenter.default.addObserver(self, selector: #selector(avatarDidBecomeAvailable(_:)), name: .AvatarDidBecomeAvailable, object: nil) NotificationCenter.default.addObserver(self, selector: #selector(faviconDidBecomeAvailable(_:)), name: .FaviconDidBecomeAvailable, object: nil) NotificationCenter.default.addObserver(self, selector: #selector(contentSizeCategoryDidChange(_:)), name: UIContentSizeCategory.didChangeNotification, object: nil) - - coordinator.webViewProvider.dequeueWebView() { webView in - - // Add the webview - self.webView = webView - webView.translatesAutoresizingMaskIntoConstraints = false - self.view.addSubview(webView) - NSLayoutConstraint.activate([ - self.view.leadingAnchor.constraint(equalTo: webView.leadingAnchor), - self.view.trailingAnchor.constraint(equalTo: webView.trailingAnchor), - self.view.topAnchor.constraint(equalTo: webView.topAnchor), - self.view.bottomAnchor.constraint(equalTo: webView.bottomAnchor) - ]) - - // Configure the tap zones - self.configureTopShowBarsView() - self.configureBottomShowBarsView() - - // Configure the webview - webView.navigationDelegate = self - webView.uiDelegate = self - webView.scrollView.delegate = self - self.configureContextMenuInteraction() - - webView.configuration.userContentController.add(WrapperScriptMessageHandler(self), name: MessageName.imageWasClicked) - webView.configuration.userContentController.add(WrapperScriptMessageHandler(self), name: MessageName.imageWasShown) - - self.reloadHTML() - - self.view.setNeedsLayout() - self.view.layoutIfNeeded() - } + // Configure the tap zones + configureTopShowBarsView() + configureBottomShowBarsView() + + loadWebView() + } override func viewWillDisappear(_ animated: Bool) { super.viewWillDisappear(animated) - stopMediaPlayback() } @@ -162,14 +131,17 @@ class WebViewController: UIViewController { // MARK: API func focus() { - webView.becomeFirstResponder() + webView?.becomeFirstResponder() } func canScrollDown() -> Bool { + guard let webView = webView else { return false } return webView.scrollView.contentOffset.y < finalScrollPosition() } func scrollPageDown() { + guard let webView = webView else { return } + let scrollToY: CGFloat = { let fullScroll = webView.scrollView.contentOffset.y + webView.scrollView.layoutMarginsGuide.layoutFrame.height let final = finalScrollPosition() @@ -191,7 +163,7 @@ class WebViewController: UIViewController { } func fullReload() { - self.reloadHTML() + self.loadWebView() } func showBars() { @@ -269,7 +241,7 @@ extension WebViewController: ArticleExtractorDelegate { func articleExtractionDidFail(with: Error) { stopArticleExtractor() articleExtractorButtonState = .error - reloadHTML() + loadWebView() } func articleExtractionDidComplete(extractedArticle: ExtractedArticle) { @@ -352,10 +324,6 @@ extension WebViewController: WKNavigationDelegate { } - func webView(_ webView: WKWebView, didFinish navigation: WKNavigation!) { - renderPage() - } - } // MARK: WKUIDelegate @@ -454,8 +422,53 @@ private struct ImageClickMessage: Codable { private extension WebViewController { - func reloadHTML() { - webView?.loadFileURL(ArticleRenderer.page.url, allowingReadAccessTo: ArticleRenderer.page.baseURL) + func loadWebView() { + guard isViewLoaded else { return } + + coordinator.webViewProvider.dequeueWebView() { webView in + + let webViewToRecycle = self.webView + + // Add the webview + webView.translatesAutoresizingMaskIntoConstraints = false + self.view.insertSubview(webView, at: 0) + NSLayoutConstraint.activate([ + self.view.leadingAnchor.constraint(equalTo: webView.leadingAnchor), + self.view.trailingAnchor.constraint(equalTo: webView.trailingAnchor), + self.view.topAnchor.constraint(equalTo: webView.topAnchor), + self.view.bottomAnchor.constraint(equalTo: webView.bottomAnchor) + ]) + + self.view.setNeedsLayout() + self.view.layoutIfNeeded() + + // Configure the webview + webView.navigationDelegate = self + webView.uiDelegate = self + webView.scrollView.delegate = self + self.configureContextMenuInteraction() + + webView.configuration.userContentController.add(WrapperScriptMessageHandler(self), name: MessageName.imageWasClicked) + webView.configuration.userContentController.add(WrapperScriptMessageHandler(self), name: MessageName.imageWasShown) + + self.renderPage() + self.recycleWebView(webViewToRecycle) + + } + + } + + func recycleWebView(_ webView: WKWebView?) { + guard let webView = webView else { return } + + webView.removeFromSuperview() + webView.evaluateJavaScript("cancelImageLoad();") + + webView.configuration.userContentController.removeScriptMessageHandler(forName: MessageName.imageWasClicked) + webView.configuration.userContentController.removeScriptMessageHandler(forName: MessageName.imageWasShown) + webView.interactions.removeAll() + + coordinator.webViewProvider.enqueueWebView(webView) } func renderPage() { @@ -492,11 +505,12 @@ private extension WebViewController { windowScrollY = 0 webView.scrollView.setZoomScale(1.0, animated: false) - webView.evaluateJavaScript(render) + webView.evaluateJavaScript(render) } func finalScrollPosition() -> CGFloat { + guard let webView = webView else { return 0 } return webView.scrollView.contentSize.height - webView.scrollView.bounds.height + webView.scrollView.safeAreaInsets.bottom } @@ -522,7 +536,8 @@ private extension WebViewController { } func imageWasClicked(body: String?) { - guard let body = body, + guard let webView = webView, + let body = body, let data = body.data(using: .utf8), let clickMessage = try? JSONDecoder().decode(ImageClickMessage.self, from: data), let range = clickMessage.imageURL.range(of: ";base64,") From b86341f44c9e353eb24cc1f7014cae644d762f71 Mon Sep 17 00:00:00 2001 From: Nate Weaver Date: Sat, 1 Feb 2020 01:12:54 -0600 Subject: [PATCH 265/330] Don't block vertical pan gestures --- iOS/MasterFeed/MasterFeedViewController.swift | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/iOS/MasterFeed/MasterFeedViewController.swift b/iOS/MasterFeed/MasterFeedViewController.swift index df1ba37c7..1ea187b56 100644 --- a/iOS/MasterFeed/MasterFeedViewController.swift +++ b/iOS/MasterFeed/MasterFeedViewController.swift @@ -215,7 +215,9 @@ class MasterFeedViewController: UITableViewController, UndoableCommandRunner { headerView.addGestureRecognizer(tap) // Without this the swipe gesture registers on the cell below - headerView.addGestureRecognizer(UIPanGestureRecognizer(target: nil, action: nil)) + let gestureRecognizer = UIPanGestureRecognizer(target: nil, action: nil) + gestureRecognizer.delegate = self + headerView.addGestureRecognizer(gestureRecognizer) headerView.interactions.removeAll() if section != 0 { @@ -1229,3 +1231,13 @@ private extension MasterFeedViewController { } } + +extension MasterFeedViewController: UIGestureRecognizerDelegate { + func gestureRecognizerShouldBegin(_ gestureRecognizer: UIGestureRecognizer) -> Bool { + guard let gestureRecognizer = gestureRecognizer as? UIPanGestureRecognizer else { + return false + } + let velocity = gestureRecognizer.velocity(in: self.view) + return abs(velocity.x) > abs(velocity.y); + } +} From b50fabe325cb77a259b47bf34e6e74e1e2ace614 Mon Sep 17 00:00:00 2001 From: Brent Simmons Date: Sat, 1 Feb 2020 12:19:39 -0800 Subject: [PATCH 266/330] =?UTF-8?q?Fix=20a=20crash=20that=20could=20happen?= =?UTF-8?q?=20when=20laying=20out=20the=20progress=20view=20while=20it?= =?UTF-8?q?=E2=80=99s=20not=20in=20the=20view=20hierarchy.=20Fix=20#1764.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- iOS/MasterFeed/RefreshProgressView.swift | 38 ++++++++++++++++-------- 1 file changed, 26 insertions(+), 12 deletions(-) diff --git a/iOS/MasterFeed/RefreshProgressView.swift b/iOS/MasterFeed/RefreshProgressView.swift index 09b9f6574..6a627e10a 100644 --- a/iOS/MasterFeed/RefreshProgressView.swift +++ b/iOS/MasterFeed/RefreshProgressView.swift @@ -13,7 +13,7 @@ class RefreshProgressView: UIView { @IBOutlet weak var progressView: UIProgressView! @IBOutlet weak var label: UILabel! - private lazy var progressWidth = progressView.widthAnchor.constraint(equalToConstant: 100.0) + private lazy var progressWidthConstraint = progressView.widthAnchor.constraint(equalToConstant: 100.0) override func awakeFromNib() { NotificationCenter.default.addObserver(self, selector: #selector(progressDidChange(_:)), name: .AccountRefreshProgressDidChange, object: nil) @@ -28,6 +28,10 @@ class RefreshProgressView: UIView { scheduleUpdateRefreshLabel() } + override func didMoveToSuperview() { + progressChanged() + } + func updateRefreshLabel() { if let accountLastArticleFetchEndTime = AccountManager.shared.lastArticleFetchEndTime { @@ -71,28 +75,38 @@ class RefreshProgressView: UIView { private extension RefreshProgressView { func progressChanged() { + // Layout may crash if not in the view hierarchy. + // https://github.com/Ranchero-Software/NetNewsWire/issues/1764 + let isInViewHierarchy = self.superview != nil + let progress = AccountManager.shared.combinedRefreshProgress if progress.isComplete { - progressView.setProgress(1, animated: true) + if isInViewHierarchy { + progressView.setProgress(1, animated: true) + } DispatchQueue.main.asyncAfter(deadline: .now() + 0.5) { self.updateRefreshLabel() self.label.isHidden = false self.progressView.isHidden = true - self.progressWidth.isActive = false - self.progressView.setProgress(0, animated: true) + self.progressWidthConstraint.isActive = false + if isInViewHierarchy { + self.progressView.setProgress(0, animated: true) + } } } else { label.isHidden = true progressView.isHidden = false - self.progressWidth.isActive = true - self.progressView.setNeedsLayout() - self.progressView.layoutIfNeeded() - let percent = Float(progress.numberCompleted) / Float(progress.numberOfTasks) - - // Don't let the progress bar go backwards unless we need to go back more than 25% - if percent > progressView.progress || progressView.progress - percent > 0.25 { - progressView.setProgress(percent, animated: true) + progressWidthConstraint.isActive = true + if isInViewHierarchy { + progressView.setNeedsLayout() + progressView.layoutIfNeeded() + let percent = Float(progress.numberCompleted) / Float(progress.numberOfTasks) + + // Don't let the progress bar go backwards unless we need to go back more than 25% + if percent > progressView.progress || progressView.progress - percent > 0.25 { + progressView.setProgress(percent, animated: true) + } } } } From 30cf8c6a0838cba680cd4e0ab0b5cf07f5408276 Mon Sep 17 00:00:00 2001 From: Brent Simmons Date: Sat, 1 Feb 2020 15:00:36 -0800 Subject: [PATCH 267/330] Add, as a micro-optimization, a custom hash function for FeedIdentifier. --- Frameworks/Account/FeedIdentifier.swift | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/Frameworks/Account/FeedIdentifier.swift b/Frameworks/Account/FeedIdentifier.swift index 1bd3230d7..eed0a7555 100644 --- a/Frameworks/Account/FeedIdentifier.swift +++ b/Frameworks/Account/FeedIdentifier.swift @@ -79,5 +79,19 @@ public enum FeedIdentifier: CustomStringConvertible, Hashable { return nil } } - + + // MARK: - Hashable + + public func hash(into hasher: inout Hasher) { + switch self { + case .smartFeed(let id): + hasher.combine(id) + case .script(let id): + hasher.combine(id) + case .webFeed(_, let webFeedID): + hasher.combine(webFeedID) + case .folder(_, let folderName): + hasher.combine(folderName) + } + } } From 9628b3021b565914402eff2e5478b4d5f6120e78 Mon Sep 17 00:00:00 2001 From: Brent Simmons Date: Sat, 1 Feb 2020 15:00:59 -0800 Subject: [PATCH 268/330] Create FetchUnreadCountsForFeedsOperation. --- .../FetchUnreadCountsForFeedsOperation.swift | 88 +++++++++++++++++++ 1 file changed, 88 insertions(+) create mode 100644 Frameworks/ArticlesDatabase/Operations/FetchUnreadCountsForFeedsOperation.swift diff --git a/Frameworks/ArticlesDatabase/Operations/FetchUnreadCountsForFeedsOperation.swift b/Frameworks/ArticlesDatabase/Operations/FetchUnreadCountsForFeedsOperation.swift new file mode 100644 index 000000000..38b2c9d22 --- /dev/null +++ b/Frameworks/ArticlesDatabase/Operations/FetchUnreadCountsForFeedsOperation.swift @@ -0,0 +1,88 @@ +// +// FetchUnreadCountsForFeedsOperation.swift +// ArticlesDatabase +// +// Created by Brent Simmons on 2/1/20. +// Copyright © 2020 Ranchero Software. All rights reserved. +// + +import Foundation +import RSCore +import RSDatabase + +/// Fetch the unread counts for a number of feeds. +public final class FetchUnreadCountsForFeedsOperation: MainThreadOperation { + + public var unreadCountDictionary: UnreadCountDictionary? + public let feedIDs: Set + + // MainThreadOperation + public var isCanceled = false + public var id: Int? + public weak var operationDelegate: MainThreadOperationDelegate? + public var name: String? + public var completionBlock: MainThreadOperation.MainThreadOperationCompletionBlock? + + private let queue: DatabaseQueue + private let cutoffDate: Date + + init(feedIDs: Set, databaseQueue: DatabaseQueue, cutoffDate: Date) { + self.feedIDs = feedIDs + self.queue = databaseQueue + self.cutoffDate = cutoffDate + } + + public func run() { + queue.runInDatabase { databaseResult in + if self.isCanceled { + self.informOperationDelegateOfCompletion() + return + } + + switch databaseResult { + case .success(let database): + self.fetchUnreadCounts(database) + case .failure: + self.informOperationDelegateOfCompletion() + } + } + } +} + +private extension FetchUnreadCountsForFeedsOperation { + + func fetchUnreadCounts(_ database: FMDatabase) { + let placeholders = NSString.rs_SQLValueList(withPlaceholders: UInt(feedIDs.count))! + let sql = "select distinct feedID, count(*) from articles natural join statuses where feedID in \(placeholders) and read=0 and userDeleted=0 and (starred=1 or dateArrived>?) group by feedID;" + + var parameters = [Any]() + parameters += Array(feedIDs) as [Any] + parameters += [cutoffDate] as [Any] + + guard let resultSet = database.executeQuery(sql, withArgumentsIn: parameters) else { + informOperationDelegateOfCompletion() + return + } + if isCanceled { + informOperationDelegateOfCompletion() + return + } + + var d = UnreadCountDictionary() + while resultSet.next() { + if isCanceled { + resultSet.close() + informOperationDelegateOfCompletion() + return + } + let unreadCount = resultSet.long(forColumnIndex: 1) + if let webFeedID = resultSet.string(forColumnIndex: 0) { + d[webFeedID] = unreadCount + } + } + resultSet.close() + + unreadCountDictionary = d + informOperationDelegateOfCompletion() + } +} From 3f4c84e4424ade269c73800bfbc61c2a3f36c7c1 Mon Sep 17 00:00:00 2001 From: Brent Simmons Date: Sat, 1 Feb 2020 15:01:47 -0800 Subject: [PATCH 269/330] Use the new FetchUnreadCountsForFeedsOperation. --- Frameworks/Account/Account.swift | 65 +++++++++------ .../ArticlesDatabase/ArticlesDatabase.swift | 9 +- .../project.pbxproj | 6 +- .../ArticlesDatabase/ArticlesTable.swift | 83 ++----------------- 4 files changed, 59 insertions(+), 104 deletions(-) diff --git a/Frameworks/Account/Account.swift b/Frameworks/Account/Account.swift index f2c11aa1b..740a7f349 100644 --- a/Frameworks/Account/Account.swift +++ b/Frameworks/Account/Account.swift @@ -235,8 +235,9 @@ public final class Account: DisplayNameProvider, UnreadCountProvider, Container, private enum OperationName { static let FetchAllUnreadCounts = "FetchAllUnreadCounts" static let FetchFeedUnreadCount = "FetchFeedUnreadCount" + static let FetchUnreadCountsForFeeds = "FetchUnreadCountsForFeeds" } - private static let discardableOperationNames = [OperationName.FetchAllUnreadCounts, OperationName.FetchFeedUnreadCount] + private static let discardableOperationNames = [OperationName.FetchAllUnreadCounts, OperationName.FetchFeedUnreadCount, OperationName.FetchUnreadCountsForFeeds] init?(dataFolder: String, type: AccountType, accountID: String, transport: Transport? = nil) { switch type { @@ -622,27 +623,7 @@ public final class Account: DisplayNameProvider, UnreadCountProvider, Container, } public func updateUnreadCounts(for webFeeds: Set, completion: VoidCompletionBlock? = nil) { - if webFeeds.isEmpty { - completion?() - return - } - - if webFeeds.count == 1, let feed = webFeeds.first { - fetchUnreadCount(feed, completion) - } - else { - database.fetchUnreadCounts(for: webFeeds.webFeedIDs()) { unreadCountDictionaryResult in - if let unreadCountDictionary = try? unreadCountDictionaryResult.get() { - for webFeed in webFeeds { - if let unreadCount = unreadCountDictionary[webFeed.webFeedID] { - webFeed.unreadCount = unreadCount - } - } - } - - completion?() - } - } + fetchUnreadCounts(webFeeds, completion) } public func fetchArticles(_ fetchType: FetchType) throws -> Set
{ @@ -1246,6 +1227,25 @@ private extension Account { NotificationCenter.default.post(name: .StatusesDidChange, object: self, userInfo: [UserInfoKey.articleIDs: articleIDs]) } + /// Fetch unread counts for zero or more feeds. + /// + /// Uses the most efficient method based on how many feeds were passed in. + func fetchUnreadCounts(for feeds: Set, _ completion: VoidCompletionBlock?) { + if feeds.isEmpty { + completion?() + return + } + if feeds.count == 1, let feed = feeds.first { + fetchUnreadCount(feed, completion) + } + else if feeds.count < 10 { + fetchUnreadCounts(feeds, completion) + } + else { + fetchAllUnreadCounts() + } + } + func fetchUnreadCount(_ feed: WebFeed, _ completion: VoidCompletionBlock?) { let operation = database.createFetchFeedUnreadCountOperation(feedID: feed.webFeedID) operation.name = OperationName.FetchFeedUnreadCount @@ -1260,6 +1260,21 @@ private extension Account { operationQueue.addOperation(operation) } + func fetchUnreadCounts(_ feeds: Set, _ completion: VoidCompletionBlock?) { + let feedIDs = feeds.map { $0.webFeedID } + let operation = database.createFetchUnreadCountsForFeedsOperation(feedIDs: Set(feedIDs)) + operation.name = OperationName.FetchUnreadCountsForFeeds + operation.completionBlock = { operation in + let fetchOperation = operation as! FetchUnreadCountsForFeedsOperation + if let unreadCountDictionary = fetchOperation.unreadCountDictionary { + self.processUnreadCounts(unreadCountDictionary: unreadCountDictionary, feeds: feeds) + } + completion?() + } + + operationQueue.addOperation(operation) + } + func fetchAllUnreadCounts() { fetchingAllUnreadCounts = true operationQueue.cancelOperations(named: OperationName.FetchAllUnreadCounts) @@ -1271,7 +1286,7 @@ private extension Account { guard let unreadCountDictionary = fetchOperation.unreadCountDictionary else { return } - self.processUnreadCounts(unreadCountDictionary: unreadCountDictionary) + self.processUnreadCounts(unreadCountDictionary: unreadCountDictionary, feeds: self.flattenedWebFeeds()) self.fetchingAllUnreadCounts = false self.updateUnreadCount() @@ -1282,8 +1297,8 @@ private extension Account { operationQueue.addOperation(operation) } - func processUnreadCounts(unreadCountDictionary: UnreadCountDictionary) { - for feed in flattenedWebFeeds() { + func processUnreadCounts(unreadCountDictionary: UnreadCountDictionary, feeds: Set) { + for feed in feeds { // When the unread count is zero, it won’t appear in unreadCountDictionary. let unreadCount = unreadCountDictionary[feed.webFeedID] ?? 0 feed.unreadCount = unreadCount diff --git a/Frameworks/ArticlesDatabase/ArticlesDatabase.swift b/Frameworks/ArticlesDatabase/ArticlesDatabase.swift index 9f6b34ba1..bb341bc4f 100644 --- a/Frameworks/ArticlesDatabase/ArticlesDatabase.swift +++ b/Frameworks/ArticlesDatabase/ArticlesDatabase.swift @@ -137,10 +137,6 @@ public final class ArticlesDatabase { // MARK: - Unread Counts - public func fetchUnreadCounts(for webFeedIDs: Set, _ completion: @escaping UnreadCountDictionaryCompletionBlock) { - articlesTable.fetchUnreadCounts(webFeedIDs, completion) - } - public func fetchUnreadCountForToday(for webFeedIDs: Set, completion: @escaping SingleUnreadCountCompletionBlock) { fetchUnreadCount(for: webFeedIDs, since: todayCutoffDate(), completion: completion) } @@ -203,6 +199,11 @@ public final class ArticlesDatabase { return FetchFeedUnreadCountOperation(feedID: feedID, databaseQueue: queue, cutoffDate: articlesTable.articleCutoffDate) } + /// Create an operation that fetches unread counts for a number of feedIDs. + public func createFetchUnreadCountsForFeedsOperation(feedIDs: Set) -> FetchUnreadCountsForFeedsOperation { + return FetchUnreadCountsForFeedsOperation(feedIDs: feedIDs, databaseQueue: queue, cutoffDate: articlesTable.articleCutoffDate) + } + // MARK: - Suspend and Resume (for iOS) /// Close the database and stop running database calls. diff --git a/Frameworks/ArticlesDatabase/ArticlesDatabase.xcodeproj/project.pbxproj b/Frameworks/ArticlesDatabase/ArticlesDatabase.xcodeproj/project.pbxproj index 7d1277606..055dd02a4 100644 --- a/Frameworks/ArticlesDatabase/ArticlesDatabase.xcodeproj/project.pbxproj +++ b/Frameworks/ArticlesDatabase/ArticlesDatabase.xcodeproj/project.pbxproj @@ -20,6 +20,7 @@ 845580671F0AEBCD003CCFA1 /* Constants.swift in Sources */ = {isa = PBXBuildFile; fileRef = 845580661F0AEBCD003CCFA1 /* Constants.swift */; }; 845580761F0AF670003CCFA1 /* Article+Database.swift in Sources */ = {isa = PBXBuildFile; fileRef = 845580751F0AF670003CCFA1 /* Article+Database.swift */; }; 8455807A1F0AF67D003CCFA1 /* ArticleStatus+Database.swift in Sources */ = {isa = PBXBuildFile; fileRef = 845580791F0AF67D003CCFA1 /* ArticleStatus+Database.swift */; }; + 84611DCC23E62FE200BC630C /* FetchUnreadCountsForFeedsOperation.swift in Sources */ = {isa = PBXBuildFile; fileRef = 84611DCB23E62FE200BC630C /* FetchUnreadCountsForFeedsOperation.swift */; }; 8477ACBC2221E76F00DF7F37 /* SearchTable.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8477ACBB2221E76F00DF7F37 /* SearchTable.swift */; }; 848E3EB920FBCFD20004B7ED /* RSCore.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 848E3EB820FBCFD20004B7ED /* RSCore.framework */; }; 848E3EBD20FBCFDE0004B7ED /* RSDatabase.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 848E3EBC20FBCFDE0004B7ED /* RSDatabase.framework */; }; @@ -126,6 +127,7 @@ 845580661F0AEBCD003CCFA1 /* Constants.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = Constants.swift; sourceTree = ""; }; 845580751F0AF670003CCFA1 /* Article+Database.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; name = "Article+Database.swift"; path = "Extensions/Article+Database.swift"; sourceTree = ""; }; 845580791F0AF67D003CCFA1 /* ArticleStatus+Database.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; name = "ArticleStatus+Database.swift"; path = "Extensions/ArticleStatus+Database.swift"; sourceTree = ""; }; + 84611DCB23E62FE200BC630C /* FetchUnreadCountsForFeedsOperation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FetchUnreadCountsForFeedsOperation.swift; sourceTree = ""; }; 8461461E1F0ABC7300870CB3 /* RSParser.xcodeproj */ = {isa = PBXFileReference; lastKnownFileType = "wrapper.pb-project"; name = RSParser.xcodeproj; path = ../RSParser/RSParser.xcodeproj; sourceTree = ""; }; 8477ACBB2221E76F00DF7F37 /* SearchTable.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SearchTable.swift; sourceTree = ""; }; 848E3EB820FBCFD20004B7ED /* RSCore.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; path = RSCore.framework; sourceTree = BUILT_PRODUCTS_DIR; }; @@ -245,8 +247,9 @@ 84C242C723DEB42700C50516 /* Operations */ = { isa = PBXGroup; children = ( - 84C242C823DEB45C00C50516 /* FetchAllUnreadCountsOperation.swift */, 84116B8823E01E86000B2E98 /* FetchFeedUnreadCountOperation.swift */, + 84611DCB23E62FE200BC630C /* FetchUnreadCountsForFeedsOperation.swift */, + 84C242C823DEB45C00C50516 /* FetchAllUnreadCountsOperation.swift */, ); path = Operations; sourceTree = ""; @@ -535,6 +538,7 @@ 84F20F8F1F180D8700D8E682 /* AuthorsTable.swift in Sources */, 84288A001F6A3C4400395871 /* DatabaseObject+Database.swift in Sources */, 8477ACBC2221E76F00DF7F37 /* SearchTable.swift in Sources */, + 84611DCC23E62FE200BC630C /* FetchUnreadCountsForFeedsOperation.swift in Sources */, 843702C31F70D15D00B18807 /* ParsedArticle+Database.swift in Sources */, 84E156EC1F0AB80E00F8CC05 /* ArticlesTable.swift in Sources */, 84E156EE1F0AB81400F8CC05 /* StatusesTable.swift in Sources */, diff --git a/Frameworks/ArticlesDatabase/ArticlesTable.swift b/Frameworks/ArticlesDatabase/ArticlesTable.swift index 64cd7770a..1f0ab8414 100644 --- a/Frameworks/ArticlesDatabase/ArticlesTable.swift +++ b/Frameworks/ArticlesDatabase/ArticlesTable.swift @@ -237,31 +237,6 @@ final class ArticlesTable: DatabaseTable { // MARK: - Unread Counts - func fetchUnreadCounts(_ webFeedIDs: Set, _ completion: @escaping UnreadCountDictionaryCompletionBlock) { - if webFeedIDs.isEmpty { - completion(.success(UnreadCountDictionary())) - return - } - - fetchAllUnreadCounts { (unreadCountsResult) in - - func createUnreadCountDictionary(_ unreadCountDictionary: UnreadCountDictionary) -> UnreadCountDictionary { - var d = UnreadCountDictionary() - for webFeedID in webFeedIDs { - d[webFeedID] = unreadCountDictionary[webFeedID] ?? 0 - } - return d - } - - switch unreadCountsResult { - case .success(let unreadCountDictionary): - completion(.success(createUnreadCountDictionary(unreadCountDictionary))) - case .failure(let databaseError): - completion(.failure(databaseError)) - } - } - } - func fetchUnreadCount(_ webFeedIDs: Set, _ since: Date, _ completion: @escaping SingleUnreadCountCompletionBlock) { // Get unread count for today, for instance. if webFeedIDs.isEmpty { @@ -298,46 +273,6 @@ final class ArticlesTable: DatabaseTable { } } - func fetchAllUnreadCounts(_ completion: @escaping UnreadCountDictionaryCompletionBlock) { - // Returns only where unreadCount > 0. - - let cutoffDate = articleCutoffDate - queue.runInDatabase { databaseResult in - - func makeDatabaseCalls(_ database: FMDatabase) { - let sql = "select distinct feedID, count(*) from articles natural join statuses where read=0 and userDeleted=0 and (starred=1 or dateArrived>?) group by feedID;" - - guard let resultSet = database.executeQuery(sql, withArgumentsIn: [cutoffDate]) else { - DispatchQueue.main.async { - completion(.success(UnreadCountDictionary())) - } - return - } - - var d = UnreadCountDictionary() - while resultSet.next() { - let unreadCount = resultSet.long(forColumnIndex: 1) - if let webFeedID = resultSet.string(forColumnIndex: 0) { - d[webFeedID] = unreadCount - } - } - - DispatchQueue.main.async { - completion(.success(d)) - } - } - - switch databaseResult { - case .success(let database): - makeDatabaseCalls(database) - case .failure(let databaseError): - DispatchQueue.main.async { - completion(.failure(databaseError)) - } - } - } - } - func fetchStarredAndUnreadCount(_ webFeedIDs: Set, _ completion: @escaping SingleUnreadCountCompletionBlock) { if webFeedIDs.isEmpty { completion(.success(0)) @@ -630,15 +565,15 @@ private extension ArticlesTable { } } - func fetchUnreadCount(_ webFeedID: String, _ database: FMDatabase) -> Int { - // Count only the articles that would appear in the UI. - // * Must be unread. - // * Must not be deleted. - // * Must be either 1) starred or 2) dateArrived must be newer than cutoff date. - - let sql = "select count(*) from articles natural join statuses where feedID=? and read=0 and userDeleted=0 and (starred=1 or dateArrived>?);" - return numberWithSQLAndParameters(sql, [webFeedID, articleCutoffDate], in: database) - } +// func fetchUnreadCount(_ webFeedID: String, _ database: FMDatabase) -> Int { +// // Count only the articles that would appear in the UI. +// // * Must be unread. +// // * Must not be deleted. +// // * Must be either 1) starred or 2) dateArrived must be newer than cutoff date. +// +// let sql = "select count(*) from articles natural join statuses where feedID=? and read=0 and userDeleted=0 and (starred=1 or dateArrived>?);" +// return numberWithSQLAndParameters(sql, [webFeedID, articleCutoffDate], in: database) +// } func fetchArticlesMatching(_ searchString: String, _ database: FMDatabase) -> Set
{ let sql = "select rowid from search where search match ?;" From 0b2ec6473ac712aec3c2576672b480e781f967bc Mon Sep 17 00:00:00 2001 From: Brent Simmons Date: Sat, 1 Feb 2020 15:16:24 -0800 Subject: [PATCH 270/330] Call the correct fetchUnreadCounts methd. --- Frameworks/Account/Account.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Frameworks/Account/Account.swift b/Frameworks/Account/Account.swift index 740a7f349..de98b328c 100644 --- a/Frameworks/Account/Account.swift +++ b/Frameworks/Account/Account.swift @@ -623,7 +623,7 @@ public final class Account: DisplayNameProvider, UnreadCountProvider, Container, } public func updateUnreadCounts(for webFeeds: Set, completion: VoidCompletionBlock? = nil) { - fetchUnreadCounts(webFeeds, completion) + fetchUnreadCounts(for: webFeeds, completion: completion) } public func fetchArticles(_ fetchType: FetchType) throws -> Set
{ @@ -1230,7 +1230,7 @@ private extension Account { /// Fetch unread counts for zero or more feeds. /// /// Uses the most efficient method based on how many feeds were passed in. - func fetchUnreadCounts(for feeds: Set, _ completion: VoidCompletionBlock?) { + func fetchUnreadCounts(for feeds: Set, completion: VoidCompletionBlock?) { if feeds.isEmpty { completion?() return From 071bd8c57cbfe218c1353b0ab5642a8638257782 Mon Sep 17 00:00:00 2001 From: Brent Simmons Date: Sat, 1 Feb 2020 15:32:26 -0800 Subject: [PATCH 271/330] Bump version to 34. --- xcconfig/common/NetNewsWire_ios_target_common.xcconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xcconfig/common/NetNewsWire_ios_target_common.xcconfig b/xcconfig/common/NetNewsWire_ios_target_common.xcconfig index 25353e80e..66e7e9d82 100644 --- a/xcconfig/common/NetNewsWire_ios_target_common.xcconfig +++ b/xcconfig/common/NetNewsWire_ios_target_common.xcconfig @@ -1,7 +1,7 @@ // High Level Settings common to both the iOS application and any extensions we bundle with it MARKETING_VERSION = 5.0 -CURRENT_PROJECT_VERSION = 33 +CURRENT_PROJECT_VERSION = 34 ALWAYS_EMBED_SWIFT_STANDARD_LIBRARIES = YES ASSETCATALOG_COMPILER_APPICON_NAME = AppIcon From f65bf63bb1b555ae90485377410faad314eab7e3 Mon Sep 17 00:00:00 2001 From: Nate Weaver Date: Fri, 31 Jan 2020 15:59:16 -0600 Subject: [PATCH 272/330] Add the ability to ignore types to FaviconURLFinder, and ignore SVG favicons --- Shared/Favicons/FaviconDownloader.swift | 4 ++- Shared/Favicons/FaviconURLFinder.swift | 39 +++++++++++++++++++++++-- 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/Shared/Favicons/FaviconDownloader.swift b/Shared/Favicons/FaviconDownloader.swift index 447012cd6..94b562e85 100644 --- a/Shared/Favicons/FaviconDownloader.swift +++ b/Shared/Favicons/FaviconDownloader.swift @@ -208,7 +208,9 @@ private extension FaviconDownloader { return } - FaviconURLFinder.findFaviconURLs(homePageURL) { (faviconURLs) in + let ignoredTypes = [kUTTypeScalableVectorGraphics as String] + + FaviconURLFinder.findFaviconURLs(with: homePageURL, ignoredTypes: ignoredTypes) { (faviconURLs) in var defaultFaviconURL: String? = nil if let scheme = url.scheme, let host = url.host { diff --git a/Shared/Favicons/FaviconURLFinder.swift b/Shared/Favicons/FaviconURLFinder.swift index 654dd1f3b..fc05dd66c 100644 --- a/Shared/Favicons/FaviconURLFinder.swift +++ b/Shared/Favicons/FaviconURLFinder.swift @@ -13,15 +13,50 @@ import RSParser struct FaviconURLFinder { - static func findFaviconURLs(_ homePageURL: String, _ completion: @escaping ([String]?) -> Void) { + /// Finds favicon URLs in a web page. + /// - Parameters: + /// - homePageURL: The page to search. + /// - ignoredTypes: An array of uniform type identifiers to ignore. + /// - completion: A closure called when the links have been found. + /// - urls: An array of favicon URLs as strings. + static func findFaviconURLs(with homePageURL: String, ignoredTypes ignoredTypes: [String]? = nil, _ completion: @escaping (_ urls:[String]?) -> Void) { guard let _ = URL(string: homePageURL) else { completion(nil) return } + var ignoredMimeTypes = [String]() + var ignoredExtensions = [String]() + + if let ignoredTypes = ignoredTypes { + for type in ignoredTypes { + if let mimeTypes = UTTypeCopyAllTagsWithClass(type as CFString, kUTTagClassMIMEType)?.takeRetainedValue() { + ignoredMimeTypes.append(contentsOf: mimeTypes as! [String]) + } + if let extensions = UTTypeCopyAllTagsWithClass(type as CFString, kUTTagClassFilenameExtension)?.takeRetainedValue() { + ignoredExtensions.append(contentsOf: extensions as! [String]) + } + } + } + + // If the favicon has an explicit type, check that for an ignored type; otherwise, check the file extension. HTMLMetadataDownloader.downloadMetadata(for: homePageURL) { (htmlMetadata) in - completion(htmlMetadata?.faviconLinks) + let faviconURLs = htmlMetadata?.faviconLinks.filter({ (faviconLink) -> Bool in + if faviconLink.type != nil { + if ignoredMimeTypes.contains(faviconLink.type) { + return false + } + } else { + if let url = URL(string: faviconLink.urlString!), ignoredExtensions.contains(url.pathExtension) { + return false + } + } + + return true + }).map { $0.urlString! } + + completion(faviconURLs) } } } From 101e1402332852dc1f1d4be7368413c7c9937911 Mon Sep 17 00:00:00 2001 From: Nate Weaver Date: Fri, 31 Jan 2020 16:09:01 -0600 Subject: [PATCH 273/330] Make ignoredTypes a static property to avoid redundant lookups --- Shared/Favicons/FaviconDownloader.swift | 6 ++--- Shared/Favicons/FaviconURLFinder.swift | 35 ++++++++++++++----------- 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/Shared/Favicons/FaviconDownloader.swift b/Shared/Favicons/FaviconDownloader.swift index 94b562e85..655e20989 100644 --- a/Shared/Favicons/FaviconDownloader.swift +++ b/Shared/Favicons/FaviconDownloader.swift @@ -60,6 +60,8 @@ final class FaviconDownloader { loadHomePageToFaviconURLCache() loadHomePageURLsWithNoFaviconURLCache() + FaviconURLFinder.ignoredTypes = [kUTTypeScalableVectorGraphics as String] + NotificationCenter.default.addObserver(self, selector: #selector(didLoadFavicon(_:)), name: .DidLoadFavicon, object: nil) } @@ -208,9 +210,7 @@ private extension FaviconDownloader { return } - let ignoredTypes = [kUTTypeScalableVectorGraphics as String] - - FaviconURLFinder.findFaviconURLs(with: homePageURL, ignoredTypes: ignoredTypes) { (faviconURLs) in + FaviconURLFinder.findFaviconURLs(with: homePageURL) { (faviconURLs) in var defaultFaviconURL: String? = nil if let scheme = url.scheme, let host = url.host { diff --git a/Shared/Favicons/FaviconURLFinder.swift b/Shared/Favicons/FaviconURLFinder.swift index fc05dd66c..21edc1ab2 100644 --- a/Shared/Favicons/FaviconURLFinder.swift +++ b/Shared/Favicons/FaviconURLFinder.swift @@ -13,23 +13,15 @@ import RSParser struct FaviconURLFinder { - /// Finds favicon URLs in a web page. - /// - Parameters: - /// - homePageURL: The page to search. - /// - ignoredTypes: An array of uniform type identifiers to ignore. - /// - completion: A closure called when the links have been found. - /// - urls: An array of favicon URLs as strings. - static func findFaviconURLs(with homePageURL: String, ignoredTypes ignoredTypes: [String]? = nil, _ completion: @escaping (_ urls:[String]?) -> Void) { + private static var ignoredMimeTypes = [String]() + private static var ignoredExtensions = [String]() - guard let _ = URL(string: homePageURL) else { - completion(nil) - return - } + static var ignoredTypes: [String]? { + didSet { + guard let ignoredTypes = ignoredTypes else { + return + } - var ignoredMimeTypes = [String]() - var ignoredExtensions = [String]() - - if let ignoredTypes = ignoredTypes { for type in ignoredTypes { if let mimeTypes = UTTypeCopyAllTagsWithClass(type as CFString, kUTTagClassMIMEType)?.takeRetainedValue() { ignoredMimeTypes.append(contentsOf: mimeTypes as! [String]) @@ -39,6 +31,19 @@ struct FaviconURLFinder { } } } + } + + /// Finds favicon URLs in a web page. + /// - Parameters: + /// - homePageURL: The page to search. + /// - completion: A closure called when the links have been found. + /// - urls: An array of favicon URLs as strings. + static func findFaviconURLs(with homePageURL: String, _ completion: @escaping (_ urls:[String]?) -> Void) { + + guard let _ = URL(string: homePageURL) else { + completion(nil) + return + } // If the favicon has an explicit type, check that for an ignored type; otherwise, check the file extension. HTMLMetadataDownloader.downloadMetadata(for: homePageURL) { (htmlMetadata) in From b088cda5a5ff342ab03204efdade087a28f89405 Mon Sep 17 00:00:00 2001 From: Nate Weaver Date: Thu, 30 Jan 2020 15:41:00 -0600 Subject: [PATCH 274/330] Fix homePageURLsWithNoFaviconURLCache --- Shared/Favicons/FaviconDownloader.swift | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/Shared/Favicons/FaviconDownloader.swift b/Shared/Favicons/FaviconDownloader.swift index eee115711..ccba7c6bb 100644 --- a/Shared/Favicons/FaviconDownloader.swift +++ b/Shared/Favicons/FaviconDownloader.swift @@ -131,20 +131,12 @@ final class FaviconDownloader { } findFaviconURLs(with: url) { (faviconURLs) in - var hasIcons = false - if let faviconURLs = faviconURLs { if let firstIconURL = faviconURLs.first { - hasIcons = true let _ = self.favicon(with: firstIconURL, homePageURL: url) self.remainingFaviconURLs[url] = faviconURLs.dropFirst() } } - - if (!hasIcons) { - self.homePageURLsWithNoFaviconURLCache.insert(url) - self.homePageURLsWithNoFaviconURLCacheDirty = true - } } return nil @@ -167,6 +159,8 @@ final class FaviconDownloader { remainingFaviconURLs[homePageURL] = faviconURLs.dropFirst(); } else { remainingFaviconURLs[homePageURL] = nil + self.homePageURLsWithNoFaviconURLCache.insert(homePageURL) + self.homePageURLsWithNoFaviconURLCacheDirty = true } } return From e9a37642704d16757ee40e013de5efed9b519df9 Mon Sep 17 00:00:00 2001 From: Nate Weaver Date: Fri, 31 Jan 2020 16:33:35 -0600 Subject: [PATCH 275/330] Documentation comment --- Shared/Favicons/FaviconURLFinder.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/Shared/Favicons/FaviconURLFinder.swift b/Shared/Favicons/FaviconURLFinder.swift index 21edc1ab2..b1183f930 100644 --- a/Shared/Favicons/FaviconURLFinder.swift +++ b/Shared/Favicons/FaviconURLFinder.swift @@ -16,6 +16,7 @@ struct FaviconURLFinder { private static var ignoredMimeTypes = [String]() private static var ignoredExtensions = [String]() + /// Uniform types to ignore when finding favicon URLs. static var ignoredTypes: [String]? { didSet { guard let ignoredTypes = ignoredTypes else { From 97049be067fdee54078ff36b0e42f2ab8ceefd51 Mon Sep 17 00:00:00 2001 From: Nate Weaver Date: Thu, 30 Jan 2020 16:02:20 -0600 Subject: [PATCH 276/330] Only add to the no-favicons cache if the only icon was the defaulted favicon.ico --- Shared/Favicons/FaviconDownloader.swift | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/Shared/Favicons/FaviconDownloader.swift b/Shared/Favicons/FaviconDownloader.swift index ccba7c6bb..447012cd6 100644 --- a/Shared/Favicons/FaviconDownloader.swift +++ b/Shared/Favicons/FaviconDownloader.swift @@ -24,6 +24,7 @@ final class FaviconDownloader { private let diskCache: BinaryDiskCache private var singleFaviconDownloaderCache = [String: SingleFaviconDownloader]() // faviconURL: SingleFaviconDownloader private var remainingFaviconURLs = [String: ArraySlice]() // homePageURL: array of faviconURLs that haven't been checked yet + private var currentHomePageHasOnlyFaviconICO = false private var homePageToFaviconURLCache = [String: String]() //homePageURL: faviconURL private var homePageToFaviconURLCachePath: String @@ -132,6 +133,8 @@ final class FaviconDownloader { findFaviconURLs(with: url) { (faviconURLs) in if let faviconURLs = faviconURLs { + self.currentHomePageHasOnlyFaviconICO = faviconURLs.count == 1 + if let firstIconURL = faviconURLs.first { let _ = self.favicon(with: firstIconURL, homePageURL: url) self.remainingFaviconURLs[url] = faviconURLs.dropFirst() @@ -159,8 +162,11 @@ final class FaviconDownloader { remainingFaviconURLs[homePageURL] = faviconURLs.dropFirst(); } else { remainingFaviconURLs[homePageURL] = nil - self.homePageURLsWithNoFaviconURLCache.insert(homePageURL) - self.homePageURLsWithNoFaviconURLCacheDirty = true + + if currentHomePageHasOnlyFaviconICO { + self.homePageURLsWithNoFaviconURLCache.insert(homePageURL) + self.homePageURLsWithNoFaviconURLCacheDirty = true + } } } return From 830db84c567cb1a4b3f1db9c130b75c41c8b5d6e Mon Sep 17 00:00:00 2001 From: Nate Weaver Date: Sat, 1 Feb 2020 01:09:36 -0600 Subject: [PATCH 277/330] Explicitly import CoreServices For UTType stuff; iOS doesn't implicitly import it. --- Shared/Favicons/FaviconDownloader.swift | 1 + Shared/Favicons/FaviconURLFinder.swift | 11 ++++++----- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/Shared/Favicons/FaviconDownloader.swift b/Shared/Favicons/FaviconDownloader.swift index 655e20989..aeb42f960 100644 --- a/Shared/Favicons/FaviconDownloader.swift +++ b/Shared/Favicons/FaviconDownloader.swift @@ -7,6 +7,7 @@ // import Foundation +import CoreServices import Articles import Account import RSCore diff --git a/Shared/Favicons/FaviconURLFinder.swift b/Shared/Favicons/FaviconURLFinder.swift index b1183f930..0f80b44d8 100644 --- a/Shared/Favicons/FaviconURLFinder.swift +++ b/Shared/Favicons/FaviconURLFinder.swift @@ -7,6 +7,7 @@ // import Foundation +import CoreServices import RSParser // The favicon URLs may be specified in the head section of the home page. @@ -48,19 +49,19 @@ struct FaviconURLFinder { // If the favicon has an explicit type, check that for an ignored type; otherwise, check the file extension. HTMLMetadataDownloader.downloadMetadata(for: homePageURL) { (htmlMetadata) in - let faviconURLs = htmlMetadata?.faviconLinks.filter({ (faviconLink) -> Bool in - if faviconLink.type != nil { - if ignoredMimeTypes.contains(faviconLink.type) { + let faviconURLs = htmlMetadata?.favicons.filter({ (favicon) -> Bool in + if let type = favicon.type { + if ignoredMimeTypes.contains(type) { return false } } else { - if let url = URL(string: faviconLink.urlString!), ignoredExtensions.contains(url.pathExtension) { + if let url = URL(string: favicon.urlString), ignoredExtensions.contains(url.pathExtension) { return false } } return true - }).map { $0.urlString! } + }).map { $0.urlString } completion(faviconURLs) } From ce73922a25d58b49fa216a036e70ee8374b513bc Mon Sep 17 00:00:00 2001 From: Nate Weaver Date: Sat, 1 Feb 2020 18:21:01 -0600 Subject: [PATCH 278/330] Update RSParser --- submodules/RSParser | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/submodules/RSParser b/submodules/RSParser index 50072e628..80952dabd 160000 --- a/submodules/RSParser +++ b/submodules/RSParser @@ -1 +1 @@ -Subproject commit 50072e62800b98c3f9f9cd5b1aae6d5b12c3a58c +Subproject commit 80952dabda6b49e8fa72d8f4cd703fe09fb56564 From 596575c0e89384c75abe4bf10e557ade4acf5090 Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Sat, 1 Feb 2020 16:41:10 -0800 Subject: [PATCH 279/330] Delay making the initial web view available to give it some time to load the DOM. Issue #1756 --- iOS/Article/WebViewProvider.swift | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/iOS/Article/WebViewProvider.swift b/iOS/Article/WebViewProvider.swift index 0f02894b5..75eec147c 100644 --- a/iOS/Article/WebViewProvider.swift +++ b/iOS/Article/WebViewProvider.swift @@ -55,8 +55,10 @@ class WebViewProvider: NSObject, WKNavigationDelegate { if waitingForFirstLoad { waitingForFirstLoad = false if let completion = waitingCompletionHandler { - completeRequest(completion: completion) - waitingCompletionHandler = nil + DispatchQueue.main.asyncAfter(deadline: .now() + 0.1) { + self.completeRequest(completion: completion) + self.waitingCompletionHandler = nil + } } } } From 4fe2a3134fb8ebc04a6fe3fb6e96ea27f8bd36ac Mon Sep 17 00:00:00 2001 From: Brent Simmons Date: Sun, 2 Feb 2020 12:11:39 -0800 Subject: [PATCH 280/330] Suspend the databases sooner when suspending the application. --- iOS/AppDelegate.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/iOS/AppDelegate.swift b/iOS/AppDelegate.swift index a4784955b..50778cb92 100644 --- a/iOS/AppDelegate.swift +++ b/iOS/AppDelegate.swift @@ -295,7 +295,8 @@ private extension AppDelegate { guard UIApplication.shared.applicationState == .background else { return } AccountManager.shared.suspendNetworkAll() - + AccountManager.shared.suspendDatabaseAll() + CoalescingQueue.standard.performCallsImmediately() for scene in UIApplication.shared.connectedScenes { if let sceneDelegate = scene.delegate as? SceneDelegate { @@ -303,7 +304,6 @@ private extension AppDelegate { } } - AccountManager.shared.suspendDatabaseAll() os_log("Application processing suspended.", log: self.log, type: .info) } From 83241cc32947b36fb57a2509cd309568ac7898ea Mon Sep 17 00:00:00 2001 From: Martin Hartl Date: Sun, 2 Feb 2020 21:44:54 +0100 Subject: [PATCH 281/330] =?UTF-8?q?Don=E2=80=99t=20show=20=E2=80=9CGo=20to?= =?UTF-8?q?=20feed=E2=80=9D=20action=20if=20already=20inside=20feed?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- iOS/MasterTimeline/MasterTimelineViewController.swift | 6 ++++-- iOS/SceneCoordinator.swift | 8 ++++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/iOS/MasterTimeline/MasterTimelineViewController.swift b/iOS/MasterTimeline/MasterTimelineViewController.swift index 22ac0af25..9e7f83da0 100644 --- a/iOS/MasterTimeline/MasterTimelineViewController.swift +++ b/iOS/MasterTimeline/MasterTimelineViewController.swift @@ -769,7 +769,8 @@ private extension MasterTimelineViewController { } func discloseFeedAction(_ article: Article) -> UIAction? { - guard let webFeed = article.webFeed else { return nil } + guard let webFeed = article.webFeed, + !coordinator.timelineFeedIsEqualTo(webFeed) else { return nil } let title = NSLocalizedString("Go to Feed", comment: "Go to Feed") let action = UIAction(title: title, image: AppAssets.openInSidebarImage) { [weak self] action in @@ -779,7 +780,8 @@ private extension MasterTimelineViewController { } func discloseFeedAlertAction(_ article: Article, completion: @escaping (Bool) -> Void) -> UIAlertAction? { - guard let webFeed = article.webFeed else { return nil } + guard let webFeed = article.webFeed, + !coordinator.timelineFeedIsEqualTo(webFeed) else { return nil } let title = NSLocalizedString("Go to Feed", comment: "Go to Feed") let action = UIAlertAction(title: title, style: .default) { [weak self] action in diff --git a/iOS/SceneCoordinator.swift b/iOS/SceneCoordinator.swift index b09d74094..5b7afe315 100644 --- a/iOS/SceneCoordinator.swift +++ b/iOS/SceneCoordinator.swift @@ -997,6 +997,14 @@ class SceneCoordinator: NSObject, UndoableCommandRunner, UnreadCountProvider { markArticlesWithUndo([article], statusKey: .starred, flag: !article.status.starred) } + func timelineFeedIsEqualTo(_ feed: WebFeed) -> Bool { + guard let timelineFeed = timelineFeed as? WebFeed else { + return false + } + + return timelineFeed == feed + } + func discloseFeed(_ feed: WebFeed, animations: Animations = [], completion: (() -> Void)? = nil) { if isSearching { masterTimelineViewController?.hideSearch() From da501cb39baee32b577f65a86c9aed921f4fccf7 Mon Sep 17 00:00:00 2001 From: Dan Morrison Date: Mon, 3 Feb 2020 09:43:05 +1100 Subject: [PATCH 282/330] Fix horizontal alignment #1768 --- iOS/Add/Add.storyboard | 8 ++++---- iOS/Add/AddWebFeedSelectFolderTableViewCell.xib | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/iOS/Add/Add.storyboard b/iOS/Add/Add.storyboard index d59f560c7..54d418e17 100644 --- a/iOS/Add/Add.storyboard +++ b/iOS/Add/Add.storyboard @@ -68,13 +68,13 @@