Skip to content

Commit 97338c8

Browse files
jkmasselclaudecrazytonyli
authored
Reader Bugfixes (#25016)
* Fix `apps.wordpress.com` URLs not being handled * Don’t reload Reader views from deep links if they’re already displayed * Properly handle links to the reader root * Rework opening a post by URL * fix broken test * release note * Address PR review feedback - 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 <[email protected]> * Clean up contentIsAlreadyDisplayed and add comment - 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 <[email protected]> * Use wordPressComRestApi.perform * Handle `/read` and `/reader` URLs * Remove tests * Fix opening WordPress.com News links in the app --------- Co-authored-by: Claude <[email protected]> Co-authored-by: Tony Li <[email protected]>
1 parent b8483ca commit 97338c8

File tree

16 files changed

+283
-189
lines changed

16 files changed

+283
-189
lines changed

‎Modules/Sources/WordPressKit/ReaderPostServiceRemote+V2.swift‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,15 @@
11
import Foundation
22

3+
public struct ResolvedReaderPost: Decodable {
4+
public let postId: UInt64
5+
public let siteId: UInt64
6+
7+
enum CodingKeys: String, CodingKey {
8+
case postId = "post_id"
9+
case siteId = "site_id"
10+
}
11+
}
12+
313
extension ReaderPostServiceRemote {
414
/// Returns a collection of RemoteReaderPost
515
/// This method returns the best available content for the given topics.
@@ -31,6 +41,23 @@ extension ReaderPostServiceRemote {
3141
})
3242
}
3343

44+
public func resolveUrl(
45+
_ url: URL,
46+
success: @escaping (ResolvedReaderPost) -> Void,
47+
failure: @escaping (Error) -> Void
48+
) {
49+
let path = "/wpcom/v2/mobile/resolve-reader-url"
50+
51+
Task { @MainActor [wordPressComRestApi] in
52+
await wordPressComRestApi.perform(.get, URLString: path, parameters: [
53+
"url": url.absoluteString,
54+
], type: ResolvedReaderPost.self)
55+
.map { $0.body }
56+
.eraseToError()
57+
.execute(onSuccess: success, onFailure: failure)
58+
}
59+
}
60+
3461
private func postsEndpoint(for topics: [String], page: String? = nil) -> String? {
3562
var path = URLComponents(string: "read/tags/posts")
3663

‎RELEASE-NOTES.txt‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,12 @@
2121
* [*] Fix overly long related post titles in Reader [#25011]
2222
* [*] Increase number of lines for post tiles in Reader to three [#25019]
2323
* [*] Fix horizontal insets in Reader article view [#25010]
24+
* [*] Fix a bug where the app can't access some Jetpack connected sites [#24976]
25+
* [*] Add support for editing custom taxonomy terms from "Post Settings" [#24964]
26+
* [*] Fix overly long related post titles in Reader [#25011]
27+
* [*] Increase number of lines for post tiles in Reader to three [#25019]
28+
* [*] Fix horizontal insets in Reader article view [#25010]
29+
* [*] Fixed several reader bugs causing posts to load strangely, or not at all [#25016]
2430

2531
26.4
2632
-----

‎Tests/KeystoneTests/Tests/Reader/ReaderDetailCoordinatorTests.swift‎

Lines changed: 1 addition & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -21,19 +21,6 @@ class ReaderDetailCoordinatorTests: CoreDataTestCase {
2121
expect(serviceMock.didCallFetchPostWithIsFeed).to(beTrue())
2222
}
2323

24-
/// Given a URL, retrieves the post
25-
///
26-
func testRetrieveAReaderPostWhenURLIsGiven() {
27-
let serviceMock = ReaderPostServiceMock()
28-
let viewMock = ReaderDetailViewMock()
29-
let coordinator = ReaderDetailCoordinator(readerPostService: serviceMock, view: viewMock)
30-
coordinator.postURL = URL(string: "https://wpmobilep2.wordpress.com/post/")
31-
32-
coordinator.start()
33-
34-
expect(serviceMock.didCallFetchWithURL).to(equal(URL(string: "https://wpmobilep2.wordpress.com/post/")))
35-
}
36-
3724
/// Inform the view to render a post after it is fetched
3825
///
3926
func testUpdateViewWithRetrievedPost() {
@@ -63,39 +50,6 @@ class ReaderDetailCoordinatorTests: CoreDataTestCase {
6350
expect(viewMock.didCallShowError).to(beTrue())
6451
}
6552

66-
/// When an error happens, tell the view to show an error
67-
///
68-
func testShowErrorWithWebActionInView() {
69-
let serviceMock = ReaderPostServiceMock()
70-
serviceMock.forceError = true
71-
let viewMock = ReaderDetailViewMock()
72-
let coordinator = ReaderDetailCoordinator(readerPostService: serviceMock, view: viewMock)
73-
coordinator.postURL = URL(string: "https://wordpress.com/")
74-
75-
coordinator.start()
76-
77-
expect(viewMock.didCallShowErrorWithWebAction).to(beTrue())
78-
}
79-
80-
/// When an error happens, call the callback
81-
///
82-
func testCallCallbackWhenAnErrorHappens() {
83-
var didCallPostLoadFailureBlock = false
84-
let serviceMock = ReaderPostServiceMock()
85-
serviceMock.forceError = true
86-
let viewMock = ReaderDetailViewMock()
87-
let coordinator = ReaderDetailCoordinator(readerPostService: serviceMock, view: viewMock)
88-
coordinator.postURL = URL(string: "https://wordpress.com/")
89-
coordinator.postLoadFailureBlock = {
90-
didCallPostLoadFailureBlock = true
91-
}
92-
93-
coordinator.start()
94-
95-
expect(didCallPostLoadFailureBlock).to(beTrue())
96-
expect(coordinator.postLoadFailureBlock).to(beNil())
97-
}
98-
9953
/// If a post is given, do not call the servce and render the content right away
10054
///
10155
func testGivenAPostRenderItRightAway() {
@@ -198,22 +152,6 @@ class ReaderDetailCoordinatorTests: CoreDataTestCase {
198152
expect(viewMock.didCallPresentWith).to(beAKindOf(LightboxViewController.self))
199153
}
200154

201-
/// Present an URL in a new Reader Detail screen
202-
///
203-
func testShowPresentURL() {
204-
let post = makeReaderPost()
205-
let serviceMock = ReaderPostServiceMock()
206-
let viewMock = ReaderDetailViewMock()
207-
let coordinator = ReaderDetailCoordinator(readerPostService: serviceMock, view: viewMock)
208-
coordinator.post = post
209-
let navigationControllerMock = UINavigationControllerMock()
210-
viewMock.navigationController = navigationControllerMock
211-
212-
coordinator.handle(URL(string: "https://wpmobilep2.wordpress.com/2020/06/01/hello-test/")!)
213-
214-
expect(navigationControllerMock.didCallPushViewControllerWith).to(beAKindOf(ReaderDetailViewController.self))
215-
}
216-
217155
/// Present an URL in a webview controller
218156
///
219157
func testShowPresentURLInWebViewController() {
@@ -330,7 +268,7 @@ private class ReaderDetailViewMock: UIViewController, ReaderDetailView {
330268
didCallShowError = true
331269
}
332270

333-
func showErrorWithWebAction(error: String?) {
271+
func showErrorWithWebAction(error: (any Error)?) {
334272
didCallShowErrorWithWebAction = true
335273
}
336274

‎Tests/KeystoneTests/Tests/Utility/UniversalLinkRouterTests.swift‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
import Foundation
2+
import Testing
3+
4+
@testable import WordPress
5+
6+
struct UniversalLinkRouterTests {
7+
8+
@Test(
9+
arguments: [
10+
"http://en.blog.wordpress.com/2025/11/26/wordpress-migration-checklist/",
11+
]
12+
)
13+
func supportPostLinks(url: String) {
14+
let router = UniversalLinkRouter.shared
15+
#expect(router.canHandle(url: URL(string: url)!))
16+
}
17+
18+
}

‎WordPress/Classes/Services/Reader Post/ReaderPostService.swift‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,20 @@ import WordPressKit
44

55
extension ReaderPostService {
66

7+
// MARK: – WP.com URL resolution
8+
9+
/// Ask the server for details about a Reader Post.
10+
///
11+
/// 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.
12+
func resolvePostUrl(
13+
_ url: URL,
14+
success: @escaping (ResolvedReaderPost) -> Void,
15+
failure: @escaping(Error) -> Void
16+
) {
17+
let remoteService = ReaderPostServiceRemote(wordPressComRestApi: apiForRequest())
18+
remoteService.resolveUrl(url, success: success, failure: failure)
19+
}
20+
721
// MARK: - Fetch Unblocked Posts
822

923
/// Fetches a list of posts from the API and filters out the posts that belong to a blocked author.

‎WordPress/Classes/System/Root View/ReaderPresenter.swift‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,10 @@ public final class ReaderPresenter: NSObject, SplitViewDisplayable {
199199
/// column (split view) or pushing to the navigation stack.
200200
private func show(_ viewController: UIViewController, isLargeTitle: Bool = false) {
201201
if let splitViewController {
202+
guard !self.contentIsAlreadyDisplayed(viewController, in: splitViewController, for: .secondary) else {
203+
DDLogInfo("View controller for \(viewController.contentIdentifier) already presented – skipping show")
204+
return
205+
}
202206
(viewController as? ReaderStreamViewController)?.isNotificationsBarButtonEnabled = true
203207

204208
let navigationVC = UINavigationController(rootViewController: viewController)
@@ -207,6 +211,12 @@ public final class ReaderPresenter: NSObject, SplitViewDisplayable {
207211
}
208212
splitViewController.setViewController(navigationVC, for: .secondary)
209213
} else {
214+
// Don't push a view controller on top of another with the same content
215+
guard !self.contentIsAlreadyDisplayed(viewController, in: mainNavigationController) else {
216+
DDLogInfo("View controller for \(viewController.contentIdentifier) already presented – skipping show")
217+
return
218+
}
219+
210220
mainNavigationController.safePushViewController(viewController, animated: true)
211221
}
212222
}
@@ -215,14 +225,56 @@ public final class ReaderPresenter: NSObject, SplitViewDisplayable {
215225
/// the `.secondary` column (split view) or to the main navigation stack.
216226
private func push(_ viewController: UIViewController) {
217227
if let splitViewController {
228+
// Don't push a view controller on top of another with the same content
229+
guard !contentIsAlreadyDisplayed(viewController, in: splitViewController, for: .secondary) else {
230+
DDLogInfo("View controller for \(viewController.contentIdentifier) already presented – skipping push")
231+
return
232+
}
218233
let navigationVC = splitViewController.viewController(for: .secondary) as? UINavigationController
219234
wpAssert(navigationVC != nil)
220235
navigationVC?.safePushViewController(viewController, animated: true)
221236
} else {
237+
// Don't push a view controller on top of another with the same content
238+
guard !self.contentIsAlreadyDisplayed(viewController, in: mainNavigationController) else {
239+
DDLogInfo("View controller for \(viewController.contentIdentifier) already presented – skipping push")
240+
return
241+
}
242+
222243
mainNavigationController.safePushViewController(viewController, animated: true)
223244
}
224245
}
225246

247+
private func contentIsAlreadyDisplayed(_ viewController: UIViewController, in nav: UINavigationController) -> Bool {
248+
guard
249+
let current = nav.topViewController as? ContentIdentifiable,
250+
let new = viewController as? ContentIdentifiable
251+
else {
252+
return false
253+
}
254+
255+
return current.contentIdentifier == new.contentIdentifier
256+
}
257+
258+
private func contentIsAlreadyDisplayed(
259+
_ viewController: UIViewController,
260+
in split: UISplitViewController,
261+
for column: UISplitViewController.Column
262+
) -> Bool {
263+
guard let top = split.viewController(for: column) else {
264+
return false
265+
}
266+
267+
if let nav = top as? UINavigationController {
268+
return self.contentIsAlreadyDisplayed(viewController, in: nav)
269+
}
270+
271+
guard let current = top as? ContentIdentifiable, let new = viewController as? ContentIdentifiable else {
272+
return false
273+
}
274+
275+
return current.contentIdentifier == new.contentIdentifier
276+
}
277+
226278
// MARK: - Deep Links (ReaderNavigationPath)
227279

228280
func navigate(to path: ReaderNavigationPath) {
@@ -271,3 +323,11 @@ private extension UINavigationController {
271323
pushViewController(viewController, animated: animated)
272324
}
273325
}
326+
327+
fileprivate extension UIViewController {
328+
/// Helper for logging – if a user hits a bug where we're not showing the content they expect, this should help debug it.
329+
/// Using a non-nil value makes the log lines easier to write.
330+
var contentIdentifier: String {
331+
(self as? ContentIdentifiable)?.contentIdentifier ?? "(unknown)"
332+
}
333+
}

‎WordPress/Classes/System/WordPressAppDelegate.swift‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -456,6 +456,21 @@ extension WordPressAppDelegate {
456456
return
457457
}
458458

459+
// Don't try to resolve `apps.wordpress.com` URLs
460+
if url.host == "apps.wordpress.com" {
461+
UniversalLinkRouter.shared.handle(url: url)
462+
return
463+
}
464+
465+
// WordPress.com News links (i.e. http://en.blog.wordpress.com/2025/11/24/managed-vs-shared-wordpress-hosting/),
466+
// which can be parsed by the app, redirect to links (i.e. https://wordpress.com/blog/2025/11/24/managed-vs-shared-wordpress-hosting/)
467+
// that are not parsable by the app.
468+
// Since we can handle post links in blog.wordpress.com, we don't need to resolve them.
469+
if url.host?.hasSuffix("blog.wordpress.com") == true, UniversalLinkRouter.shared.canHandle(url: url) {
470+
UniversalLinkRouter.shared.handle(url: url)
471+
return
472+
}
473+
459474
trackDeepLink(for: url) { url in
460475
DispatchQueue.main.async {
461476
UniversalLinkRouter.shared.handle(url: url)

‎WordPress/Classes/Utility/Universal Links/ContentIdentifiable.swift‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
import Foundation
2+
3+
/// A protocol representing a content identifier – it could be a URL, ISBN, etc
4+
///
5+
/// More than one object might share a content identifier – two objects with the same identifier represent the same content.
6+
///
7+
public protocol ContentIdentifiable {
8+
var contentIdentifier: String? { get }
9+
}

‎WordPress/Classes/Utility/Universal Links/Route.swift‎

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,33 @@ import UIKit
88
/// Path: /me/account/:username
99
///
1010
protocol Route {
11-
var path: String { get }
11+
var path: RoutePath { get }
12+
var alternatePaths: [RoutePath] { get }
1213
var section: DeepLinkSection? { get }
1314
var source: DeepLinkSource { get }
1415
var action: NavigationAction { get }
1516
var shouldTrack: Bool { get }
1617
var jetpackPowered: Bool { get }
1718
}
1819

20+
extension Route {
21+
var alternatePaths: [RoutePath] {
22+
[]
23+
}
24+
25+
var allPaths: [RoutePath] {
26+
[path] + alternatePaths
27+
}
28+
}
29+
30+
typealias RoutePath = String
31+
32+
extension RoutePath {
33+
var components: [String] {
34+
return (self as NSString).pathComponents
35+
}
36+
}
37+
1938
extension Route {
2039
// Default routes to handling links rather than other source types
2140
var source: DeepLinkSource {
@@ -75,11 +94,6 @@ struct FailureNavigationAction: NavigationAction {
7594
// MARK: - Route helper methods
7695

7796
extension Route {
78-
/// Returns the path components of a route's path.
79-
var components: [String] {
80-
return (path as NSString).pathComponents
81-
}
82-
8397
func isEqual(to route: Route) -> Bool {
8498
return path == route.path
8599
}

0 commit comments

Comments
 (0)