From ea90e4fbdfc5c4d5fd3f478eea420e3af206e921 Mon Sep 17 00:00:00 2001 From: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Mon, 24 Nov 2025 16:18:09 -0700 Subject: [PATCH 01/12] Fix `apps.wordpress.com` URLs not being handled --- WordPress/Classes/System/WordPressAppDelegate.swift | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/WordPress/Classes/System/WordPressAppDelegate.swift b/WordPress/Classes/System/WordPressAppDelegate.swift index 64e938f2129d..d466d5a1dedd 100644 --- a/WordPress/Classes/System/WordPressAppDelegate.swift +++ b/WordPress/Classes/System/WordPressAppDelegate.swift @@ -456,6 +456,12 @@ extension WordPressAppDelegate { return } + // Don't try to resolve `apps.wordpress.com` URLs + if url.host == "apps.wordpress.com" { + UniversalLinkRouter.shared.handle(url: url) + return + } + trackDeepLink(for: url) { url in DispatchQueue.main.async { UniversalLinkRouter.shared.handle(url: url) From 52bec8cece9d5c88638fe9e067296b8be6ca0c06 Mon Sep 17 00:00:00 2001 From: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Mon, 24 Nov 2025 16:18:44 -0700 Subject: [PATCH 02/12] =?UTF-8?q?Don=E2=80=99t=20reload=20Reader=20views?= =?UTF-8?q?=20from=20deep=20links=20if=20they=E2=80=99re=20already=20displ?= =?UTF-8?q?ayed?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../System/Root View/ReaderPresenter.swift | 53 +++++++++++++++++++ .../Universal Links/ContentIdentifiable.swift | 9 ++++ .../ReaderStreamViewController.swift | 18 +++++++ .../Detail/ReaderDetailViewController.swift | 11 ++++ 4 files changed, 91 insertions(+) create mode 100644 WordPress/Classes/Utility/Universal Links/ContentIdentifiable.swift diff --git a/WordPress/Classes/System/Root View/ReaderPresenter.swift b/WordPress/Classes/System/Root View/ReaderPresenter.swift index b38a6bfbf216..5347498a4d61 100644 --- a/WordPress/Classes/System/Root View/ReaderPresenter.swift +++ b/WordPress/Classes/System/Root View/ReaderPresenter.swift @@ -199,6 +199,9 @@ public final class ReaderPresenter: NSObject, SplitViewDisplayable { /// column (split view) or pushing to the navigation stack. private func show(_ viewController: UIViewController, isLargeTitle: Bool = false) { if let splitViewController { + guard !self.contentIsAlreadyDisplayed(viewController, in: splitViewController, for: .secondary) else { + return + } (viewController as? ReaderStreamViewController)?.isNotificationsBarButtonEnabled = true let navigationVC = UINavigationController(rootViewController: viewController) @@ -207,6 +210,11 @@ public final class ReaderPresenter: NSObject, SplitViewDisplayable { } splitViewController.setViewController(navigationVC, for: .secondary) } else { + // Don't push a view controller on top of another with the same content + guard !self.contentIsAlreadyDisplayed(viewController, in: mainNavigationController) else { + return + } + mainNavigationController.safePushViewController(viewController, animated: true) } } @@ -215,14 +223,59 @@ public final class ReaderPresenter: NSObject, SplitViewDisplayable { /// the `.secondary` column (split view) or to the main navigation stack. private func push(_ viewController: UIViewController) { if let splitViewController { + // Don't push a view controller on top of another with the same content + guard !contentIsAlreadyDisplayed(viewController, in: splitViewController, for: .secondary) else { + return + } let navigationVC = splitViewController.viewController(for: .secondary) as? UINavigationController wpAssert(navigationVC != nil) navigationVC?.safePushViewController(viewController, animated: true) } else { + // Don't push a view controller on top of another with the same content + guard !self.contentIsAlreadyDisplayed(viewController, in: mainNavigationController) else { + return + } + mainNavigationController.safePushViewController(viewController, animated: true) } } + private func contentIsAlreadyDisplayed(_ viewController: UIViewController, in nav: UINavigationController) -> Bool { + if let current = nav.topViewController as? ContentIdentifiable { + if let new = viewController as? ContentIdentifiable { + if new.contentIdentifier == current.contentIdentifier { + return true + } + } + } + + return false + } + + private func contentIsAlreadyDisplayed( + _ viewController: UIViewController, + in split: UISplitViewController, + for column: UISplitViewController.Column + ) -> Bool { + guard let top = split.viewController(for: column) else { + return false + } + + if let nav = top as? UINavigationController { + return contentIsAlreadyDisplayed(viewController, in: nav) + } + + if let current = top as? ContentIdentifiable { + if let new = viewController as? ContentIdentifiable { + if new.contentIdentifier == current.contentIdentifier { + return true + } + } + } + + return false + } + // MARK: - Deep Links (ReaderNavigationPath) func navigate(to path: ReaderNavigationPath) { diff --git a/WordPress/Classes/Utility/Universal Links/ContentIdentifiable.swift b/WordPress/Classes/Utility/Universal Links/ContentIdentifiable.swift new file mode 100644 index 000000000000..26073cb2964e --- /dev/null +++ b/WordPress/Classes/Utility/Universal Links/ContentIdentifiable.swift @@ -0,0 +1,9 @@ +import Foundation + +/// A protocol representing a content identifier – it could be a URL, ISBN, etc +/// +/// More than one object might share a content identifier – two objects with the same identifier represent the same content. +/// +public protocol ContentIdentifiable { + var contentIdentifier: String? { get } +} diff --git a/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderStreamViewController.swift b/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderStreamViewController.swift index 1e5148a2759b..c88a76bc879f 100644 --- a/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderStreamViewController.swift +++ b/WordPress/Classes/ViewRelated/Reader/Controllers/ReaderStreamViewController.swift @@ -1637,6 +1637,24 @@ extension ReaderStreamViewController: UITableViewDelegate, JPScrollViewDelegate } } +extension ReaderStreamViewController: ContentIdentifiable { + var contentIdentifier: String? { + if let siteId = self.siteID { + return "https://wordpress.com/reader/feeds/\(siteId)/" + } + + if let tagSlug = self.tagSlug { + return "https://wordpress.com/tag/\(tagSlug)" + } + + if let readerTopic { + return readerTopic.path + } + + return nil + } +} + private enum Strings { static let postRemoved = NSLocalizedString("reader.savedPostRemovedNotificationTitle", value: "Saved post removed", comment: "Notification title for when saved post is removed") } diff --git a/WordPress/Classes/ViewRelated/Reader/Detail/ReaderDetailViewController.swift b/WordPress/Classes/ViewRelated/Reader/Detail/ReaderDetailViewController.swift index 0af60e32e8d0..afdcc8883b12 100644 --- a/WordPress/Classes/ViewRelated/Reader/Detail/ReaderDetailViewController.swift +++ b/WordPress/Classes/ViewRelated/Reader/Detail/ReaderDetailViewController.swift @@ -1313,3 +1313,14 @@ extension ReaderDetailViewController: BorderedButtonTableViewCellDelegate { ) } } + +extension ReaderDetailViewController: ContentIdentifiable { + var contentIdentifier: String? { + // Only ever try to resolve this based on the site ID and post ID – the post object is fetched later + if let siteId = self.coordinator?.siteID?.intValue, let postId = self.coordinator?.postID?.intValue { + return "https://wordpress.com/reader/feeds/\(siteId)/posts/\(postId)" + } + + return nil + } +} From c6a3e76dd21e10661158307d17b94e4296c82cca Mon Sep 17 00:00:00 2001 From: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Mon, 24 Nov 2025 16:30:09 -0700 Subject: [PATCH 03/12] Properly handle links to the reader root --- .../Classes/ViewRelated/System/WPTabBarController+Swift.swift | 2 ++ 1 file changed, 2 insertions(+) diff --git a/WordPress/Classes/ViewRelated/System/WPTabBarController+Swift.swift b/WordPress/Classes/ViewRelated/System/WPTabBarController+Swift.swift index b6a5c4ddc23e..39476a798890 100644 --- a/WordPress/Classes/ViewRelated/System/WPTabBarController+Swift.swift +++ b/WordPress/Classes/ViewRelated/System/WPTabBarController+Swift.swift @@ -40,6 +40,8 @@ extension WPTabBarController { showReaderTab() if let path { self.readerPresenter?.navigate(to: path) + } else { // navigate back to the reader root + self.readerPresenter?.navigate(to: .discover) } } From d1ea8c9565b0fffb4e1d02febba28f55c95d0d8b Mon Sep 17 00:00:00 2001 From: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Wed, 26 Nov 2025 13:38:11 -0700 Subject: [PATCH 04/12] Rework opening a post by URL --- .../ReaderPostServiceRemote+V2.swift | 29 +++++++++++ .../Reader Post/ReaderPostService.swift | 14 ++++++ .../Universal Links/RouteMatcher.swift | 22 +++------ .../Universal Links/Routes+Reader.swift | 25 +--------- .../Detail/ReaderDetailCoordinator.swift | 48 +++++-------------- .../Detail/ReaderDetailViewController.swift | 29 +++++++---- 6 files changed, 83 insertions(+), 84 deletions(-) diff --git a/Modules/Sources/WordPressKit/ReaderPostServiceRemote+V2.swift b/Modules/Sources/WordPressKit/ReaderPostServiceRemote+V2.swift index f402ab6411bb..428a016ae094 100644 --- a/Modules/Sources/WordPressKit/ReaderPostServiceRemote+V2.swift +++ b/Modules/Sources/WordPressKit/ReaderPostServiceRemote+V2.swift @@ -1,5 +1,10 @@ import Foundation +public struct ResolvedReaderPost: Decodable { + public let postId: UInt64 + public let siteId: UInt64 +} + extension ReaderPostServiceRemote { /// Returns a collection of RemoteReaderPost /// This method returns the best available content for the given topics. @@ -31,6 +36,30 @@ extension ReaderPostServiceRemote { }) } + public func resolveUrl( + _ url: URL, + success: @escaping (ResolvedReaderPost) -> Void, + failure: @escaping (Error) -> Void + ) { + + wordPressComRESTAPI.get("/wpcom/v2/mobile/resolve-reader-url", parameters: [ + "url": url.absoluteString + ]) { data, response in + guard + let responseDict = data as? [String: UInt64], + let siteId = responseDict["site_id"], + let postId = responseDict["post_id"] + else { + failure(CocoaError(.coderInvalidValue)) + return + } + + success(ResolvedReaderPost(postId: postId, siteId: siteId)) + } failure: { error, response in + failure(error) + } + } + private func postsEndpoint(for topics: [String], page: String? = nil) -> String? { var path = URLComponents(string: "read/tags/posts") diff --git a/WordPress/Classes/Services/Reader Post/ReaderPostService.swift b/WordPress/Classes/Services/Reader Post/ReaderPostService.swift index 0b3314480515..71646f705238 100644 --- a/WordPress/Classes/Services/Reader Post/ReaderPostService.swift +++ b/WordPress/Classes/Services/Reader Post/ReaderPostService.swift @@ -4,6 +4,20 @@ import WordPressKit extension ReaderPostService { + // MARK: – WP.com URL resolution + + /// Ask the server for details about a Reader Post. + /// + /// It's impossible to resolve a post URL into anything that the API can understand client-side. So we can ask the server to do it for us. + func resolvePostUrl( + _ url: URL, + success: @escaping (ResolvedReaderPost) -> Void, + failure: @escaping(Error) -> Void + ) { + let remoteService = ReaderPostServiceRemote(wordPressComRestApi: apiForRequest()) + remoteService.resolveUrl(url, success: success, failure: failure) + } + // MARK: - Fetch Unblocked Posts /// Fetches a list of posts from the API and filters out the posts that belong to a blocked author. diff --git a/WordPress/Classes/Utility/Universal Links/RouteMatcher.swift b/WordPress/Classes/Utility/Universal Links/RouteMatcher.swift index e39f59b48ccd..8c28479f2bcb 100644 --- a/WordPress/Classes/Utility/Universal Links/RouteMatcher.swift +++ b/WordPress/Classes/Utility/Universal Links/RouteMatcher.swift @@ -25,7 +25,7 @@ class RouteMatcher { func routesMatching(_ url: URL) -> [MatchedRoute] { let pathComponents = url.pathComponents - return routes.compactMap({ route in + return routes.compactMap({ route -> MatchedRoute? in let values = valuesDictionary(forURL: url) // If the paths are the same, we definitely have a match @@ -40,23 +40,15 @@ class RouteMatcher { return nil } - guard let placeholderValues = placeholderDictionary(forKeyComponents: routeComponents, - valueComponents: pathComponents) else { - return nil - } - - let allValues = values.merging(placeholderValues, - uniquingKeysWith: { (current, _) in current }) - - // If it's a wpcomPost reader route, then check if it's a valid wpcom url. - // Need to check since we have no guarantee that it's a valid wpcom url, - // other than the path having 4 components. - if let readerRoute = route as? ReaderRoute, - readerRoute == .wpcomPost, - !readerRoute.isValidWpcomUrl(allValues) { + guard let placeholderValues = placeholderDictionary( + forKeyComponents: routeComponents, + valueComponents: pathComponents + ) else { return nil } + let allValues = values.merging(placeholderValues, uniquingKeysWith: { (current, _) in current }) + return route.matched(with: allValues) }) } diff --git a/WordPress/Classes/Utility/Universal Links/Routes+Reader.swift b/WordPress/Classes/Utility/Universal Links/Routes+Reader.swift index d5926a35b147..1172c85f8e37 100644 --- a/WordPress/Classes/Utility/Universal Links/Routes+Reader.swift +++ b/WordPress/Classes/Utility/Universal Links/Routes+Reader.swift @@ -113,9 +113,7 @@ extension ReaderRoute: NavigationAction { presenter.showReader(path: .post(postID: postID, siteID: blogID)) } case .wpcomPost: - if let urlString = values[MatchedRouteURLComponentKey.url.rawValue], - let url = URL(string: urlString), - isValidWpcomUrl(values) { + if let urlString = values[MatchedRouteURLComponentKey.url.rawValue], let url = URL(string: urlString) { presenter.showReader(path: .postURL(url)) } } @@ -142,27 +140,6 @@ extension ReaderRoute: NavigationAction { return (blogID, postID) } - - func isValidWpcomUrl(_ values: [String: String]) -> Bool { - let year = Int(values["post_year"] ?? "") ?? 0 - let month = Int(values["post_month"] ?? "") ?? 0 - let day = Int(values["post_day"] ?? "") ?? 0 - - // we assume no posts were made in the 1800's or earlier - func isYear(_ year: Int) -> Bool { - year > 1900 - } - - func isMonth(_ month: Int) -> Bool { - (1...12).contains(month) - } - - func isDay(_ day: Int) -> Bool { - (1...31).contains(day) - } - - return isYear(year) && isMonth(month) && isDay(day) - } } // MARK: - RootViewPresenter (Extensions) diff --git a/WordPress/Classes/ViewRelated/Reader/Detail/ReaderDetailCoordinator.swift b/WordPress/Classes/ViewRelated/Reader/Detail/ReaderDetailCoordinator.swift index adc05fc6e104..14f96f4b5945 100644 --- a/WordPress/Classes/ViewRelated/Reader/Detail/ReaderDetailCoordinator.swift +++ b/WordPress/Classes/ViewRelated/Reader/Detail/ReaderDetailCoordinator.swift @@ -347,7 +347,7 @@ class ReaderDetailCoordinator { self?.post = post self?.renderPostAndBumpStats() }, failure: { [weak self] error in - self?.postURL == nil ? self?.showError(error: error) : self?.view?.showErrorWithWebAction(error: error?.localizedDescription) + self?.postURL == nil ? self?.showError(error: error) : self?.view?.showErrorWithWebAction(error: error) self?.reportPostLoadFailure() }) } @@ -357,15 +357,17 @@ class ReaderDetailCoordinator { /// Use this method to fetch a ReaderPost from a URL. /// - Parameter url: a post URL private func fetch(_ url: URL) { - readerPostService.fetchPost(at: url, - success: { [weak self] post in - self?.post = post - self?.renderPostAndBumpStats() - }, failure: { [weak self] error in - DDLogError("Error fetching post for detail: \(String(describing: error?.localizedDescription))") - self?.postURL == nil ? self?.showError(error: error) : self?.view?.showErrorWithWebAction(error: error?.localizedDescription) - self?.reportPostLoadFailure() - }) + readerPostService.resolvePostUrl(url) { resolvedPost in + self.fetch( + postID: NSNumber(value: resolvedPost.postId), + siteID: NSNumber(value: resolvedPost.siteId), + isFeed: false + ) + } failure: { error in + DDLogError("Error fetching post for detail: \(String(describing: error.localizedDescription))") + self.showError(error: error) + self.reportPostLoadFailure() + } } private func showError(error: Error?) { @@ -476,8 +478,6 @@ class ReaderDetailCoordinator { presentWebViewController(url) } else if readerLinkRouter.canHandle(url: url) { readerLinkRouter.handle(url: url, shouldTrack: false, source: .inApp(presenter: viewController)) - } else if url.isWordPressDotComPost { - presentReaderDetail(url) } else if url.isLinkProtocol { readerLinkRouter.handle(url: url, shouldTrack: false, source: .inApp(presenter: viewController)) } else { @@ -525,30 +525,6 @@ class ReaderDetailCoordinator { }) } - /// Given a URL presents it in a new Reader detail screen - /// - private func presentReaderDetail(_ url: URL) { - - // In cross post Notifications, if the user tapped the link to the original post in the Notification body, - // use the original post's info to display reader detail. - // The API endpoint used by controllerWithPostID returns subscription flags for the post. - // The API endpoint used by controllerWithPostURL does not return this information. - // These flags are needed to display the `Follow conversation by email` option. - // So if we can call controllerWithPostID, do so. Otherwise, fallback to controllerWithPostURL. - // Ref: https://github.com/wordpress-mobile/WordPress-iOS/issues/17158 - - let readerDetail: ReaderDetailViewController = { - if let post, - selectedUrlIsCrossPost(url) { - return ReaderDetailViewController.controllerWithPostID(post.crossPostMeta.postID, siteID: post.crossPostMeta.siteID) - } - - return ReaderDetailViewController.controllerWithPostURL(url) - }() - - viewController?.navigationController?.pushViewController(readerDetail, animated: true) - } - private func selectedUrlIsCrossPost(_ url: URL) -> Bool { // Trim trailing slashes to facilitate URL comparison. let characterSet = CharacterSet(charactersIn: "/") diff --git a/WordPress/Classes/ViewRelated/Reader/Detail/ReaderDetailViewController.swift b/WordPress/Classes/ViewRelated/Reader/Detail/ReaderDetailViewController.swift index afdcc8883b12..2b1808314bdc 100644 --- a/WordPress/Classes/ViewRelated/Reader/Detail/ReaderDetailViewController.swift +++ b/WordPress/Classes/ViewRelated/Reader/Detail/ReaderDetailViewController.swift @@ -14,7 +14,7 @@ protocol ReaderDetailView: AnyObject { func renderRelatedPosts(_ posts: [RemoteReaderSimplePost]) func showLoading() func showError(subtitle: String?) - func showErrorWithWebAction(error: String?) + func showErrorWithWebAction(error: Error?) func scroll(to: String) func updateHeader() func updateLikesView(with viewModel: ReaderDetailLikesViewModel) @@ -109,6 +109,11 @@ class ReaderDetailViewController: UIViewController, ReaderDetailView { return coordinator?.post } + /// The URL to a post can change – we might set it when we instantiate the controller, but then when we fetch the details from the server, we know + /// that `https://wordpress.com/reader/{site_id}/posts/{post_id}` is actually `https://example.com/foo/bar`. We need to keep + /// an unchanging reference to the URL so that we can identify the content this view is presenting before we fetch the post from the server. + private var originalUrl: URL? = nil + /// The related posts for the post being shown var relatedPosts: [RelatedPostsSection] = [] @@ -399,8 +404,8 @@ class ReaderDetailViewController: UIViewController, ReaderDetailView { } /// Shown an error with a button to open the post on the browser - func showErrorWithWebAction(error: String?) { - displayLoadingViewWithWebAction(title: LoadingText.errorLoadingTitle, subtitle: error) + func showErrorWithWebAction(error: Error?) { + displayLoadingViewWithWebAction(title: LoadingText.errorLoadingTitle, error: error) } /// Scroll the content to a given #hash @@ -832,7 +837,7 @@ class ReaderDetailViewController: UIViewController, ReaderDetailView { let coordinator = ReaderDetailCoordinator(view: controller) coordinator.postURL = url controller.coordinator = coordinator - + controller.originalUrl = url return controller } @@ -1121,11 +1126,13 @@ private extension ReaderDetailViewController { showLoadingView() } - func displayLoadingViewWithWebAction(title: String, subtitle: String? = nil, accessoryView: UIView? = nil) { - noResultsViewController.configure(title: title, - buttonTitle: LoadingText.errorLoadingPostURLButtonTitle, - subtitle: subtitle, - accessoryView: accessoryView) + func displayLoadingViewWithWebAction(title: String, error: Error? = nil, accessoryView: UIView? = nil) { + noResultsViewController.configure( + title: title, + buttonTitle: LoadingText.errorLoadingPostURLButtonTitle, + subtitle: error?.localizedDescription, + accessoryView: accessoryView + ) showLoadingView() } @@ -1321,6 +1328,10 @@ extension ReaderDetailViewController: ContentIdentifiable { return "https://wordpress.com/reader/feeds/\(siteId)/posts/\(postId)" } + if let originalUrl { + return originalUrl.absoluteString + } + return nil } } From 3502ea5f3d85c7ad44f38ea61b74bc784fa21622 Mon Sep 17 00:00:00 2001 From: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Wed, 26 Nov 2025 14:10:49 -0700 Subject: [PATCH 05/12] fix broken test --- .../Tests/Reader/ReaderDetailCoordinatorTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/KeystoneTests/Tests/Reader/ReaderDetailCoordinatorTests.swift b/Tests/KeystoneTests/Tests/Reader/ReaderDetailCoordinatorTests.swift index fcb0f3047cfb..caef1401955e 100644 --- a/Tests/KeystoneTests/Tests/Reader/ReaderDetailCoordinatorTests.swift +++ b/Tests/KeystoneTests/Tests/Reader/ReaderDetailCoordinatorTests.swift @@ -330,7 +330,7 @@ private class ReaderDetailViewMock: UIViewController, ReaderDetailView { didCallShowError = true } - func showErrorWithWebAction(error: String?) { + func showErrorWithWebAction(error: (any Error)?) { didCallShowErrorWithWebAction = true } From df7a0b5fff1e6582dffb55c92569faf824d57677 Mon Sep 17 00:00:00 2001 From: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Wed, 26 Nov 2025 14:11:46 -0700 Subject: [PATCH 06/12] release note --- RELEASE-NOTES.txt | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index 3868898abe39..982af3cc6790 100644 --- a/RELEASE-NOTES.txt +++ b/RELEASE-NOTES.txt @@ -21,6 +21,12 @@ * [*] Fix overly long related post titles in Reader [#25011] * [*] Increase number of lines for post tiles in Reader to three [#25019] * [*] Fix horizontal insets in Reader article view [#25010] +* [*] Fix a bug where the app can't access some Jetpack connected sites [#24976] +* [*] Add support for editing custom taxonomy terms from "Post Settings" [#24964] +* [*] Fix overly long related post titles in Reader [#25011] +* [*] Increase number of lines for post tiles in Reader to three [#25019] +* [*] Fix horizontal insets in Reader article view [#25010] +* [*] Fixed several reader bugs causing posts to load strangely, or not at all [#25016] 26.4 ----- From a9472f78f7674ee8a35727240b123d180c610d05 Mon Sep 17 00:00:00 2001 From: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Wed, 26 Nov 2025 14:31:04 -0700 Subject: [PATCH 07/12] Address PR review feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove unused `Decodable` conformance from `ResolvedReaderPost` - Add `[weak self]` to closures in `fetch(_ url:)` to prevent retain cycles - Simplify nested conditionals in `contentIsAlreadyDisplayed` - Add debug logging when skipping duplicate content presentation - Make `self.` usage consistent 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- .../ReaderPostServiceRemote+V2.swift | 2 +- .../System/Root View/ReaderPresenter.swift | 24 ++++++++++++------- .../Detail/ReaderDetailCoordinator.swift | 10 ++++---- 3 files changed, 22 insertions(+), 14 deletions(-) diff --git a/Modules/Sources/WordPressKit/ReaderPostServiceRemote+V2.swift b/Modules/Sources/WordPressKit/ReaderPostServiceRemote+V2.swift index 428a016ae094..4842ef43337f 100644 --- a/Modules/Sources/WordPressKit/ReaderPostServiceRemote+V2.swift +++ b/Modules/Sources/WordPressKit/ReaderPostServiceRemote+V2.swift @@ -1,6 +1,6 @@ import Foundation -public struct ResolvedReaderPost: Decodable { +public struct ResolvedReaderPost { public let postId: UInt64 public let siteId: UInt64 } diff --git a/WordPress/Classes/System/Root View/ReaderPresenter.swift b/WordPress/Classes/System/Root View/ReaderPresenter.swift index 5347498a4d61..58b47d041512 100644 --- a/WordPress/Classes/System/Root View/ReaderPresenter.swift +++ b/WordPress/Classes/System/Root View/ReaderPresenter.swift @@ -200,6 +200,7 @@ public final class ReaderPresenter: NSObject, SplitViewDisplayable { private func show(_ viewController: UIViewController, isLargeTitle: Bool = false) { if let splitViewController { guard !self.contentIsAlreadyDisplayed(viewController, in: splitViewController, for: .secondary) else { + DDLogInfo("View controller for \(viewController.contentIdentifier) already presented – skipping show") return } (viewController as? ReaderStreamViewController)?.isNotificationsBarButtonEnabled = true @@ -212,6 +213,7 @@ public final class ReaderPresenter: NSObject, SplitViewDisplayable { } else { // Don't push a view controller on top of another with the same content guard !self.contentIsAlreadyDisplayed(viewController, in: mainNavigationController) else { + DDLogInfo("View controller for \(viewController.contentIdentifier) already presented – skipping show") return } @@ -225,6 +227,7 @@ public final class ReaderPresenter: NSObject, SplitViewDisplayable { if let splitViewController { // Don't push a view controller on top of another with the same content guard !contentIsAlreadyDisplayed(viewController, in: splitViewController, for: .secondary) else { + DDLogInfo("View controller for \(viewController.contentIdentifier) already presented – skipping push") return } let navigationVC = splitViewController.viewController(for: .secondary) as? UINavigationController @@ -233,6 +236,7 @@ public final class ReaderPresenter: NSObject, SplitViewDisplayable { } else { // Don't push a view controller on top of another with the same content guard !self.contentIsAlreadyDisplayed(viewController, in: mainNavigationController) else { + DDLogInfo("View controller for \(viewController.contentIdentifier) already presented – skipping push") return } @@ -262,18 +266,14 @@ public final class ReaderPresenter: NSObject, SplitViewDisplayable { } if let nav = top as? UINavigationController { - return contentIsAlreadyDisplayed(viewController, in: nav) + return self.contentIsAlreadyDisplayed(viewController, in: nav) } - if let current = top as? ContentIdentifiable { - if let new = viewController as? ContentIdentifiable { - if new.contentIdentifier == current.contentIdentifier { - return true - } - } + guard let current = top as? ContentIdentifiable, let new = viewController as? ContentIdentifiable else { + return false } - return false + return current.contentIdentifier == new.contentIdentifier } // MARK: - Deep Links (ReaderNavigationPath) @@ -324,3 +324,11 @@ private extension UINavigationController { pushViewController(viewController, animated: animated) } } + +fileprivate extension UIViewController { + /// Helper for logging – if a user hits a bug where we're not showing the content they expect, this should help debug it. + /// Using a non-nil value makes the log lines easier to write. + var contentIdentifier: String { + (self as? ContentIdentifiable)?.contentIdentifier ?? "(unknown)" + } +} diff --git a/WordPress/Classes/ViewRelated/Reader/Detail/ReaderDetailCoordinator.swift b/WordPress/Classes/ViewRelated/Reader/Detail/ReaderDetailCoordinator.swift index 14f96f4b5945..bc577ae70955 100644 --- a/WordPress/Classes/ViewRelated/Reader/Detail/ReaderDetailCoordinator.swift +++ b/WordPress/Classes/ViewRelated/Reader/Detail/ReaderDetailCoordinator.swift @@ -357,16 +357,16 @@ class ReaderDetailCoordinator { /// Use this method to fetch a ReaderPost from a URL. /// - Parameter url: a post URL private func fetch(_ url: URL) { - readerPostService.resolvePostUrl(url) { resolvedPost in - self.fetch( + readerPostService.resolvePostUrl(url) { [weak self] resolvedPost in + self?.fetch( postID: NSNumber(value: resolvedPost.postId), siteID: NSNumber(value: resolvedPost.siteId), isFeed: false ) - } failure: { error in + } failure: { [weak self] error in DDLogError("Error fetching post for detail: \(String(describing: error.localizedDescription))") - self.showError(error: error) - self.reportPostLoadFailure() + self?.showError(error: error) + self?.reportPostLoadFailure() } } From 3e56bdd4048d97fe24419b5ff589f3b890a222e6 Mon Sep 17 00:00:00 2001 From: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Wed, 26 Nov 2025 15:07:37 -0700 Subject: [PATCH 08/12] Clean up contentIsAlreadyDisplayed and add comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Simplify nested conditionals in `contentIsAlreadyDisplayed(_:in nav:)` to use guard-let pattern - Add clarifying comment for UInt64 type requirement in resolveUrl 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- .../WordPressKit/ReaderPostServiceRemote+V2.swift | 1 + .../Classes/System/Root View/ReaderPresenter.swift | 13 ++++++------- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Modules/Sources/WordPressKit/ReaderPostServiceRemote+V2.swift b/Modules/Sources/WordPressKit/ReaderPostServiceRemote+V2.swift index 4842ef43337f..4ee6b92e72ab 100644 --- a/Modules/Sources/WordPressKit/ReaderPostServiceRemote+V2.swift +++ b/Modules/Sources/WordPressKit/ReaderPostServiceRemote+V2.swift @@ -46,6 +46,7 @@ extension ReaderPostServiceRemote { "url": url.absoluteString ]) { data, response in guard + // If the numbers aren't positive integers the response is invalid let responseDict = data as? [String: UInt64], let siteId = responseDict["site_id"], let postId = responseDict["post_id"] diff --git a/WordPress/Classes/System/Root View/ReaderPresenter.swift b/WordPress/Classes/System/Root View/ReaderPresenter.swift index 58b47d041512..728024bbc360 100644 --- a/WordPress/Classes/System/Root View/ReaderPresenter.swift +++ b/WordPress/Classes/System/Root View/ReaderPresenter.swift @@ -245,15 +245,14 @@ public final class ReaderPresenter: NSObject, SplitViewDisplayable { } private func contentIsAlreadyDisplayed(_ viewController: UIViewController, in nav: UINavigationController) -> Bool { - if let current = nav.topViewController as? ContentIdentifiable { - if let new = viewController as? ContentIdentifiable { - if new.contentIdentifier == current.contentIdentifier { - return true - } - } + guard + let current = nav.topViewController as? ContentIdentifiable, + let new = viewController as? ContentIdentifiable + else { + return false } - return false + return current.contentIdentifier == new.contentIdentifier } private func contentIsAlreadyDisplayed( From 3040de4e074edadf936fc10dc713e88487c83761 Mon Sep 17 00:00:00 2001 From: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Wed, 26 Nov 2025 15:38:53 -0700 Subject: [PATCH 09/12] Use wordPressComRestApi.perform --- .../ReaderPostServiceRemote+V2.swift | 33 +++++++++---------- 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/Modules/Sources/WordPressKit/ReaderPostServiceRemote+V2.swift b/Modules/Sources/WordPressKit/ReaderPostServiceRemote+V2.swift index 4ee6b92e72ab..d171f0f2db4c 100644 --- a/Modules/Sources/WordPressKit/ReaderPostServiceRemote+V2.swift +++ b/Modules/Sources/WordPressKit/ReaderPostServiceRemote+V2.swift @@ -1,8 +1,13 @@ import Foundation -public struct ResolvedReaderPost { +public struct ResolvedReaderPost: Decodable { public let postId: UInt64 public let siteId: UInt64 + + enum CodingKeys: String, CodingKey { + case postId = "post_id" + case siteId = "site_id" + } } extension ReaderPostServiceRemote { @@ -41,23 +46,15 @@ extension ReaderPostServiceRemote { success: @escaping (ResolvedReaderPost) -> Void, failure: @escaping (Error) -> Void ) { - - wordPressComRESTAPI.get("/wpcom/v2/mobile/resolve-reader-url", parameters: [ - "url": url.absoluteString - ]) { data, response in - guard - // If the numbers aren't positive integers the response is invalid - let responseDict = data as? [String: UInt64], - let siteId = responseDict["site_id"], - let postId = responseDict["post_id"] - else { - failure(CocoaError(.coderInvalidValue)) - return - } - - success(ResolvedReaderPost(postId: postId, siteId: siteId)) - } failure: { error, response in - failure(error) + let path = "/wpcom/v2/mobile/resolve-reader-url" + + Task { @MainActor [wordPressComRestApi] in + await wordPressComRestApi.perform(.get, URLString: path, parameters: [ + "url": url.absoluteString, + ], type: ResolvedReaderPost.self) + .map { $0.body } + .eraseToError() + .execute(onSuccess: success, onFailure: failure) } } From 523028f4a0982046ba680a205a31e6e549f18e22 Mon Sep 17 00:00:00 2001 From: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Wed, 26 Nov 2025 18:25:12 -0700 Subject: [PATCH 10/12] Handle `/read` and `/reader` URLs --- .../Utility/Universal Links/Route.swift | 26 ++++++--- .../Universal Links/RouteMatcher.swift | 54 +++++++++---------- .../Universal Links/Routes+Reader.swift | 34 +++++++----- 3 files changed, 67 insertions(+), 47 deletions(-) diff --git a/WordPress/Classes/Utility/Universal Links/Route.swift b/WordPress/Classes/Utility/Universal Links/Route.swift index 4d0119f3dbbc..7d40adbf57ac 100644 --- a/WordPress/Classes/Utility/Universal Links/Route.swift +++ b/WordPress/Classes/Utility/Universal Links/Route.swift @@ -8,7 +8,8 @@ import UIKit /// Path: /me/account/:username /// protocol Route { - var path: String { get } + var path: RoutePath { get } + var alternatePaths: [RoutePath] { get } var section: DeepLinkSection? { get } var source: DeepLinkSource { get } var action: NavigationAction { get } @@ -16,6 +17,24 @@ protocol Route { var jetpackPowered: Bool { get } } +extension Route { + var alternatePaths: [RoutePath] { + [] + } + + var allPaths: [RoutePath] { + [path] + alternatePaths + } +} + +typealias RoutePath = String + +extension RoutePath { + var components: [String] { + return (self as NSString).pathComponents + } +} + extension Route { // Default routes to handling links rather than other source types var source: DeepLinkSource { @@ -75,11 +94,6 @@ struct FailureNavigationAction: NavigationAction { // MARK: - Route helper methods extension Route { - /// Returns the path components of a route's path. - var components: [String] { - return (path as NSString).pathComponents - } - func isEqual(to route: Route) -> Bool { return path == route.path } diff --git a/WordPress/Classes/Utility/Universal Links/RouteMatcher.swift b/WordPress/Classes/Utility/Universal Links/RouteMatcher.swift index 8c28479f2bcb..bd35e24f36e0 100644 --- a/WordPress/Classes/Utility/Universal Links/RouteMatcher.swift +++ b/WordPress/Classes/Utility/Universal Links/RouteMatcher.swift @@ -24,33 +24,33 @@ class RouteMatcher { /// func routesMatching(_ url: URL) -> [MatchedRoute] { let pathComponents = url.pathComponents - - return routes.compactMap({ route -> MatchedRoute? in - let values = valuesDictionary(forURL: url) - - // If the paths are the same, we definitely have a match - if route.path == url.path { - return route.matched(with: values) - } - - let routeComponents = route.components - - // Ensure the paths have the same number of components - guard routeComponents.count == pathComponents.count else { - return nil - } - - guard let placeholderValues = placeholderDictionary( - forKeyComponents: routeComponents, - valueComponents: pathComponents - ) else { - return nil - } - - let allValues = values.merging(placeholderValues, uniquingKeysWith: { (current, _) in current }) - - return route.matched(with: allValues) - }) + let values = valuesDictionary(forURL: url) + + return routes.compactMap { route -> MatchedRoute? in + route.allPaths.compactMap { candidate in + // If the paths are the same, we definitely have a match + if candidate == url.path { + return route.matched(with: values) + } + + let candidateComponents = candidate.components + + // Ensure the paths have the same number of components + guard candidateComponents.count == pathComponents.count else { + return nil + } + + guard let placeholderValues = placeholderDictionary( + forKeyComponents: candidateComponents, + valueComponents: pathComponents + ) else { + return nil + } + + let allValues = values.merging(placeholderValues, uniquingKeysWith: { (current, _) in current }) + return route.matched(with: allValues) + }.first + } } private func valuesDictionary(forURL url: URL) -> [String: String] { diff --git a/WordPress/Classes/Utility/Universal Links/Routes+Reader.swift b/WordPress/Classes/Utility/Universal Links/Routes+Reader.swift index 1172c85f8e37..d3fa8ee68bd5 100644 --- a/WordPress/Classes/Utility/Universal Links/Routes+Reader.swift +++ b/WordPress/Classes/Utility/Universal Links/Routes+Reader.swift @@ -20,35 +20,41 @@ enum ReaderRoute { extension ReaderRoute: Route { var path: String { + // Guaranteed to be safe force-unwrap if `alternatePaths` is exhaustive and returns at least one value + // for every variant + alternatePaths.first! + } + + var alternatePaths: [RoutePath] { switch self { case .root: - return "/read" + return ["/read", "/reader"] case .discover: - return "/discover" + return ["/discover"] case .search: - return "/read/search" + return ["/read/search", "/reader/search"] case .a8c: - return "/read/a8c" + return ["/read/a8c", "/reader/a8c"] case .p2: - return "/read/p2" + return ["/read/p2", "/reader/p2"] case .likes: - return "/activities/likes" + return ["/activities/likes"] case .manageFollowing: - return "/following/manage" + return ["/following/manage"] case .list: - return "/read/list/:username/:list_name" + return ["/read/list/:username/:list_name", "/reader/list/:username/:list_name"] case .tag: - return "/tag/:tag_name" + return ["/tag/:tag_name"] case .feed: - return "/read/feeds/:feed_id" + return ["/read/feeds/:feed_id", "/reader/feeds/:feed_id"] case .blog: - return "/read/blogs/:blog_id" + return ["/read/blogs/:blog_id", "/reader/blogs/:blog_id"] case .feedsPost: - return "/read/feeds/:feed_id/posts/:post_id" + return ["/read/feeds/:feed_id/posts/:post_id", "/reader/feeds/:feed_id/posts/:post_id"] case .blogsPost: - return "/read/blogs/:blog_id/posts/:post_id" + return ["/read/blogs/:blog_id/posts/:post_id", "/reader/blogs/:blog_id/posts/:post_id"] case .wpcomPost: - return "/:post_year/:post_month/:post_day/:post_name" + return ["/:post_year/:post_month/:post_day/:post_name"] } } From f7746ec797bfe8a2efe8dbe31d2e884bb079b827 Mon Sep 17 00:00:00 2001 From: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Wed, 26 Nov 2025 19:14:28 -0700 Subject: [PATCH 11/12] Remove tests --- .../Reader/ReaderDetailCoordinatorTests.swift | 62 ------------------- 1 file changed, 62 deletions(-) diff --git a/Tests/KeystoneTests/Tests/Reader/ReaderDetailCoordinatorTests.swift b/Tests/KeystoneTests/Tests/Reader/ReaderDetailCoordinatorTests.swift index caef1401955e..af807a52495e 100644 --- a/Tests/KeystoneTests/Tests/Reader/ReaderDetailCoordinatorTests.swift +++ b/Tests/KeystoneTests/Tests/Reader/ReaderDetailCoordinatorTests.swift @@ -21,19 +21,6 @@ class ReaderDetailCoordinatorTests: CoreDataTestCase { expect(serviceMock.didCallFetchPostWithIsFeed).to(beTrue()) } - /// Given a URL, retrieves the post - /// - func testRetrieveAReaderPostWhenURLIsGiven() { - let serviceMock = ReaderPostServiceMock() - let viewMock = ReaderDetailViewMock() - let coordinator = ReaderDetailCoordinator(readerPostService: serviceMock, view: viewMock) - coordinator.postURL = URL(string: "https://wpmobilep2.wordpress.com/post/") - - coordinator.start() - - expect(serviceMock.didCallFetchWithURL).to(equal(URL(string: "https://wpmobilep2.wordpress.com/post/"))) - } - /// Inform the view to render a post after it is fetched /// func testUpdateViewWithRetrievedPost() { @@ -63,39 +50,6 @@ class ReaderDetailCoordinatorTests: CoreDataTestCase { expect(viewMock.didCallShowError).to(beTrue()) } - /// When an error happens, tell the view to show an error - /// - func testShowErrorWithWebActionInView() { - let serviceMock = ReaderPostServiceMock() - serviceMock.forceError = true - let viewMock = ReaderDetailViewMock() - let coordinator = ReaderDetailCoordinator(readerPostService: serviceMock, view: viewMock) - coordinator.postURL = URL(string: "https://wordpress.com/") - - coordinator.start() - - expect(viewMock.didCallShowErrorWithWebAction).to(beTrue()) - } - - /// When an error happens, call the callback - /// - func testCallCallbackWhenAnErrorHappens() { - var didCallPostLoadFailureBlock = false - let serviceMock = ReaderPostServiceMock() - serviceMock.forceError = true - let viewMock = ReaderDetailViewMock() - let coordinator = ReaderDetailCoordinator(readerPostService: serviceMock, view: viewMock) - coordinator.postURL = URL(string: "https://wordpress.com/") - coordinator.postLoadFailureBlock = { - didCallPostLoadFailureBlock = true - } - - coordinator.start() - - expect(didCallPostLoadFailureBlock).to(beTrue()) - expect(coordinator.postLoadFailureBlock).to(beNil()) - } - /// If a post is given, do not call the servce and render the content right away /// func testGivenAPostRenderItRightAway() { @@ -198,22 +152,6 @@ class ReaderDetailCoordinatorTests: CoreDataTestCase { expect(viewMock.didCallPresentWith).to(beAKindOf(LightboxViewController.self)) } - /// Present an URL in a new Reader Detail screen - /// - func testShowPresentURL() { - let post = makeReaderPost() - let serviceMock = ReaderPostServiceMock() - let viewMock = ReaderDetailViewMock() - let coordinator = ReaderDetailCoordinator(readerPostService: serviceMock, view: viewMock) - coordinator.post = post - let navigationControllerMock = UINavigationControllerMock() - viewMock.navigationController = navigationControllerMock - - coordinator.handle(URL(string: "https://wpmobilep2.wordpress.com/2020/06/01/hello-test/")!) - - expect(navigationControllerMock.didCallPushViewControllerWith).to(beAKindOf(ReaderDetailViewController.self)) - } - /// Present an URL in a webview controller /// func testShowPresentURLInWebViewController() { From dbecfbe62aacd56cd6838d4035d09f1332864816 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Thu, 27 Nov 2025 21:41:21 +1300 Subject: [PATCH 12/12] Fix opening WordPress.com News links in the app --- .../Utility/UniversalLinkRouterTests.swift | 18 ++++++++++++++++++ .../Classes/System/WordPressAppDelegate.swift | 9 +++++++++ .../Universal Links/UniversalLinkRouter.swift | 4 ++-- 3 files changed, 29 insertions(+), 2 deletions(-) create mode 100644 Tests/KeystoneTests/Tests/Utility/UniversalLinkRouterTests.swift diff --git a/Tests/KeystoneTests/Tests/Utility/UniversalLinkRouterTests.swift b/Tests/KeystoneTests/Tests/Utility/UniversalLinkRouterTests.swift new file mode 100644 index 000000000000..8e1a1693267e --- /dev/null +++ b/Tests/KeystoneTests/Tests/Utility/UniversalLinkRouterTests.swift @@ -0,0 +1,18 @@ +import Foundation +import Testing + +@testable import WordPress + +struct UniversalLinkRouterTests { + + @Test( + arguments: [ + "http://en.blog.wordpress.com/2025/11/26/wordpress-migration-checklist/", + ] + ) + func supportPostLinks(url: String) { + let router = UniversalLinkRouter.shared + #expect(router.canHandle(url: URL(string: url)!)) + } + +} diff --git a/WordPress/Classes/System/WordPressAppDelegate.swift b/WordPress/Classes/System/WordPressAppDelegate.swift index d466d5a1dedd..fd2121000a07 100644 --- a/WordPress/Classes/System/WordPressAppDelegate.swift +++ b/WordPress/Classes/System/WordPressAppDelegate.swift @@ -462,6 +462,15 @@ extension WordPressAppDelegate { return } + // WordPress.com News links (i.e. http://en.blog.wordpress.com/2025/11/24/managed-vs-shared-wordpress-hosting/), + // which can be parsed by the app, redirect to links (i.e. https://wordpress.com/blog/2025/11/24/managed-vs-shared-wordpress-hosting/) + // that are not parsable by the app. + // Since we can handle post links in blog.wordpress.com, we don't need to resolve them. + if url.host?.hasSuffix("blog.wordpress.com") == true, UniversalLinkRouter.shared.canHandle(url: url) { + UniversalLinkRouter.shared.handle(url: url) + return + } + trackDeepLink(for: url) { url in DispatchQueue.main.async { UniversalLinkRouter.shared.handle(url: url) diff --git a/WordPress/Classes/Utility/Universal Links/UniversalLinkRouter.swift b/WordPress/Classes/Utility/Universal Links/UniversalLinkRouter.swift index b1b928b9aa1d..4232f476fb2e 100644 --- a/WordPress/Classes/Utility/Universal Links/UniversalLinkRouter.swift +++ b/WordPress/Classes/Utility/Universal Links/UniversalLinkRouter.swift @@ -121,8 +121,8 @@ struct UniversalLinkRouter: LinkRouter { } // If there's a hostname, check if it's WordPress.com or jetpack.com/app. - return scheme == "https" - && (host == "wordpress.com" || host == "jetpack.com") + return (scheme == "https" || scheme == "http") + && (host == "wordpress.com" || host == "jetpack.com" || host.hasSuffix(".wordpress.com") || host.hasSuffix(".jetpack.com")) && matcherCanHandle }