Skip to content

Fix duplicate domain navigation in Jetpack #25880

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mauropereiira wants to merge 1 commit into
base: trunk
Choose a base branch
from
+54 −32
Open

Fix duplicate domain navigation in Jetpack #25880

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 51 additions & 1 deletion Tests/KeystoneTests/Tests/Features/Domains/DomainDetailsWebViewControllerTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import XCTest
import Testing

@testable import WordPress

Expand Down Expand Up @@ -54,7 +55,56 @@ final class DomainDetailsWebViewControllerTests: XCTestCase {
siteSlug: String = Constants.siteSlug,
viewSlug: String = Constants.viewSlug
) throws -> String {
let url = "\(Constants.domainManagementBase)/\(domain)/\(viewSlug)/\(siteSlug)".addingPercentEncoding(withAllowedCharacters: .urlQueryAllowed)
let url = "\(Constants.domainManagementBase)/\(domain)/\(viewSlug)/\(siteSlug)"
.addingPercentEncoding(withAllowedCharacters: .urlQueryAllowed)
return try XCTUnwrap(url)
}
}

@MainActor
struct DomainDetailsNavigationTests {

@Test func navigationPolicyDistinguishesRedirectsFromLinks() throws {
let controller = DomainDetailsWebViewController(
domain: "example.com",
siteSlug: "example.wordpress.com",
type: .mapped
)
let redirectURL = try #require(
URL(string: "https://wordpress.com/domains/manage/all/example.com/edit/example.wordpress.com?redirected=1")
)
var redirectRequest = URLRequest(url: redirectURL)
redirectRequest.mainDocumentURL = redirectURL
let externalURLHandler = ExternalURLHandlerSpy()

let redirectPolicy = controller.linkBehavior.handle(
request: redirectRequest,
with: .other,
externalURLHandler: externalURLHandler
)

#expect(redirectPolicy == .allow)
#expect(externalURLHandler.openedURL == nil)

let linkURL = try #require(URL(string: "https://wordpress.com/support"))
var linkRequest = URLRequest(url: linkURL)
linkRequest.mainDocumentURL = linkURL

let linkPolicy = controller.linkBehavior.handle(
request: linkRequest,
with: .linkActivated,
externalURLHandler: externalURLHandler
)

#expect(linkPolicy == .cancel)
#expect(externalURLHandler.openedURL == linkURL)
}
}

private final class ExternalURLHandlerSpy: ExternalURLHandler {
private(set) var openedURL: URL?

func open(_ url: URL) {
openedURL = url
}
}
34 changes: 3 additions & 31 deletions WordPress/Classes/ViewRelated/Me/Domain Details/DomainDetailsWebViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,17 @@ final class DomainDetailsWebViewController: WebKitViewController {
static let manageAllDomainsPath = "\(domainsPath)/manage/all"
}

// MARK: - Properties

private let domain: String

private var observation: NSKeyValueObservation?

// MARK: - Init

init(domain: String, siteSlug: String, type: DomainType, analyticsSource: String? = nil) {
self.domain = domain
let url = Self.wpcomDetailsURL(domain: domain, siteSlug: siteSlug, type: type)
let configuration = WebViewControllerConfiguration(url: url)
configuration.customTitle = domain
configuration.analyticsSource = analyticsSource
configuration.secureInteraction = true
if let url {
configuration.linkBehavior = .urlOnly(url)
}

@crazytonyli crazytonyli Aug 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original PR intentionally went with the "observing URL" approach, because the linkBehavior implementation is not sufficient to support the "open links in Safari" UX. See #21964 (comment)

You can re-produce the issue by clicking the "Payment details" button. The existing implementation takes user to Safari which shows the Calypso UI, but this PR shows the web page directly inside the "domain details" in-app web view.

@mauropereiira mauropereiira Aug 10, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, and thanks for digging up the original discussion and checking the Payment details flow. I used LinkBehavior because it cleanly separates the automatic redirect behind CMM-2221 from normal .linkActivated navigation, and the Renews flow I tested still opened externally. I missed that Payment details changes the URL without a WKNavigationDelegate callback, which is exactly why this controller used KVO. So this approach would regress the intended behavior.

configuration.authenticateWithDefaultAccount()
super.init(configuration: configuration)
}
Expand All @@ -40,25 +36,9 @@ final class DomainDetailsWebViewController: WebKitViewController {

override func viewDidLoad() {
super.viewDidLoad()
self.observeURL()
self.trackWebViewShownEvent(url: url)
}

// MARK: - Handling URL Changes

private func observeURL() {
self.observation = webView.observe(\.url) { [weak self] webView, _ in
guard let self, let url = webView.url else {
return
}
if !self.shouldAllowNavigation(for: url) {
// Open URL in device browser then go back to Domain Management page.
self.open(url)
self.goBack()
}

@crazytonyli crazytonyli Aug 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jkmassel @oguzkocer I believe the question of "why don't the links stay in the in-app web view" came up in the walkthrough. I traced this code a little bit, and I believe it's an intention design choice (see pcdRpT-3PI-p2): this specific "domain details" web page is modified to adapt the mobile app look & feel, which is quite different from the default Calypso look & feel. This adaptation is very likely to be per-page basis, which means links in the domain details will take user to the default Calypso UI which won't look right inside the mobile apps.

@mauropereiira mauropereiira Aug 10, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That context helps, thank you for tracing it. I read the external-link behavior as a general navigation policy, but if only the domain details page gets the mobile-specific treatment, keeping the linked Calypso pages out of the embedded web view makes sense.

}
}

// MARK: - Navigation

override func goBack() {
Expand All @@ -84,14 +64,6 @@ final class DomainDetailsWebViewController: WebKitViewController {
WPAnalytics.track(.allDomainsDomainDetailsWebViewShown, properties: properties)
}

private func shouldAllowNavigation(for url: URL) -> Bool {
return url.absoluteString == self.url?.absoluteString
}

private func open(_ url: URL) {
UIApplication.shared.open(url)
}

private static func wpcomDetailsURL(domain: String, siteSlug: String, type: DomainType) -> URL? {
let viewSlug = {
switch type {
Expand Down