diff --git a/Modules/Sources/WordPressKit/ReaderPostServiceRemote+V2.swift b/Modules/Sources/WordPressKit/ReaderPostServiceRemote+V2.swift index f402ab6411bb..d171f0f2db4c 100644 --- a/Modules/Sources/WordPressKit/ReaderPostServiceRemote+V2.swift +++ b/Modules/Sources/WordPressKit/ReaderPostServiceRemote+V2.swift @@ -1,5 +1,15 @@ import Foundation +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 { /// Returns a collection of RemoteReaderPost /// This method returns the best available content for the given topics. @@ -31,6 +41,23 @@ extension ReaderPostServiceRemote { }) } + public func resolveUrl( + _ url: URL, + success: @escaping (ResolvedReaderPost) -> Void, + failure: @escaping (Error) -> Void + ) { + 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) + } + } + private func postsEndpoint(for topics: [String], page: String? = nil) -> String? { var path = URLComponents(string: "read/tags/posts") 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 ----- diff --git a/Tests/KeystoneTests/Tests/Reader/ReaderDetailCoordinatorTests.swift b/Tests/KeystoneTests/Tests/Reader/ReaderDetailCoordinatorTests.swift index fcb0f3047cfb..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() { @@ -330,7 +268,7 @@ private class ReaderDetailViewMock: UIViewController, ReaderDetailView { didCallShowError = true } - func showErrorWithWebAction(error: String?) { + func showErrorWithWebAction(error: (any Error)?) { didCallShowErrorWithWebAction = true } 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/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/System/Root View/ReaderPresenter.swift b/WordPress/Classes/System/Root View/ReaderPresenter.swift index b38a6bfbf216..728024bbc360 100644 --- a/WordPress/Classes/System/Root View/ReaderPresenter.swift +++ b/WordPress/Classes/System/Root View/ReaderPresenter.swift @@ -199,6 +199,10 @@ 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 { + DDLogInfo("View controller for \(viewController.contentIdentifier) already presented – skipping show") + return + } (viewController as? ReaderStreamViewController)?.isNotificationsBarButtonEnabled = true let navigationVC = UINavigationController(rootViewController: viewController) @@ -207,6 +211,12 @@ 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 { + DDLogInfo("View controller for \(viewController.contentIdentifier) already presented – skipping show") + return + } + mainNavigationController.safePushViewController(viewController, animated: true) } } @@ -215,14 +225,56 @@ 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 { + DDLogInfo("View controller for \(viewController.contentIdentifier) already presented – skipping push") + 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 { + DDLogInfo("View controller for \(viewController.contentIdentifier) already presented – skipping push") + return + } + mainNavigationController.safePushViewController(viewController, animated: true) } } + private func contentIsAlreadyDisplayed(_ viewController: UIViewController, in nav: UINavigationController) -> Bool { + guard + let current = nav.topViewController as? ContentIdentifiable, + let new = viewController as? ContentIdentifiable + else { + return false + } + + return current.contentIdentifier == new.contentIdentifier + } + + 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 self.contentIsAlreadyDisplayed(viewController, in: nav) + } + + guard let current = top as? ContentIdentifiable, let new = viewController as? ContentIdentifiable else { + return false + } + + return current.contentIdentifier == new.contentIdentifier + } + // MARK: - Deep Links (ReaderNavigationPath) func navigate(to path: ReaderNavigationPath) { @@ -271,3 +323,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/System/WordPressAppDelegate.swift b/WordPress/Classes/System/WordPressAppDelegate.swift index 64e938f2129d..fd2121000a07 100644 --- a/WordPress/Classes/System/WordPressAppDelegate.swift +++ b/WordPress/Classes/System/WordPressAppDelegate.swift @@ -456,6 +456,21 @@ extension WordPressAppDelegate { return } + // Don't try to resolve `apps.wordpress.com` URLs + if url.host == "apps.wordpress.com" { + UniversalLinkRouter.shared.handle(url: url) + 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/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/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 e39f59b48ccd..bd35e24f36e0 100644 --- a/WordPress/Classes/Utility/Universal Links/RouteMatcher.swift +++ b/WordPress/Classes/Utility/Universal Links/RouteMatcher.swift @@ -24,41 +24,33 @@ class RouteMatcher { /// func routesMatching(_ url: URL) -> [MatchedRoute] { let pathComponents = url.pathComponents - - return routes.compactMap({ route 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 }) - - // 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) { - return nil - } - - 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 d5926a35b147..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"] } } @@ -113,9 +119,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 +146,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/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 } 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/ReaderDetailCoordinator.swift b/WordPress/Classes/ViewRelated/Reader/Detail/ReaderDetailCoordinator.swift index adc05fc6e104..bc577ae70955 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) { [weak self] resolvedPost in + self?.fetch( + postID: NSNumber(value: resolvedPost.postId), + siteID: NSNumber(value: resolvedPost.siteId), + isFeed: false + ) + } failure: { [weak self] 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 0af60e32e8d0..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() } @@ -1313,3 +1320,18 @@ 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)" + } + + if let originalUrl { + return originalUrl.absoluteString + } + + return nil + } +} 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) } }