From 04cc4398e4cbb3ceb78c88be13839b0f2967160b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20=C5=A0vara?= Date: Mon, 6 Jul 2026 12:58:03 +0200 Subject: [PATCH] Only pop the pre-redirect controller when a redirect crosses contexts. --- Source/Turbo/Navigator/Navigator.swift | 18 ++++- .../NavigationHierarchyControllerTests.swift | 73 ++++++++++++++++++- 2 files changed, 86 insertions(+), 5 deletions(-) diff --git a/Source/Turbo/Navigator/Navigator.swift b/Source/Turbo/Navigator/Navigator.swift index dcc1d91..aecd7b4 100644 --- a/Source/Turbo/Navigator/Navigator.swift +++ b/Source/Turbo/Navigator/Navigator.swift @@ -175,10 +175,20 @@ public class Navigator { extension Navigator: SessionDelegate { public func session(_ session: Session, didProposeVisit proposal: VisitProposal) { if proposal.isRedirect { - // Animate the pop only if we're in the active modal session - // and the visit is proposed on the default context. - let animatePop = session === modalSession && proposal.context == .default - pop(animated: animatePop) + // Turbo re-proposes a followed redirect with a `replace` action (it also + // performed `history.replaceState` on the web side), so routing the + // proposal already rewrites the pre-redirect controller in place. We only + // need to pop that controller when the redirect moves across the + // default <-> modal boundary, otherwise the + // controller is orphaned on the stack it was pushed onto. + let sessionIsModal = session === modalSession + let proposalIsModal = proposal.context == .modal + if sessionIsModal != proposalIsModal { + // Animate the pop only when receding from the active modal session + // to the default context, matching the back-style transition. + let animatePop = sessionIsModal && proposal.context == .default + pop(animated: animatePop) + } } route(proposal) } diff --git a/Tests/Turbo/Navigator/NavigationHierarchyControllerTests.swift b/Tests/Turbo/Navigator/NavigationHierarchyControllerTests.swift index 20ec380..5a997d6 100644 --- a/Tests/Turbo/Navigator/NavigationHierarchyControllerTests.swift +++ b/Tests/Turbo/Navigator/NavigationHierarchyControllerTests.swift @@ -516,6 +516,75 @@ final class NavigationHierarchyControllerTests: XCTestCase { XCTAssertEqual(modalNavigationController.visibleViewController?.isModalInPresentation, false) } + // MARK: Redirects + + /// A followed redirect within the default context is re-proposed by Turbo with + /// a `replace` action, so it replaces the pre-redirect controller in place and + /// must not pop the screen beneath it. + func test_redirect_mainToMain_replacesRedirectedControllerAndKeepsUnderlyingScreen() { + navigator.route(oneURL) + navigator.route(twoURL) + XCTAssertEqual(navigationController.viewControllers.count, 2) + + // /two responded with a redirect to /three, re-proposed by the main session. + let redirect = VisitProposal(path: "/three", action: .replace, context: .default, redirected: true) + navigator.session(session, didProposeVisit: redirect) + + XCTAssertEqual(navigationController.viewControllers.count, 2) + let underlying = navigationController.viewControllers.first as? VisitableViewController + XCTAssertEqual(underlying?.initialVisitableURL, oneURL) + assertVisited(url: redirect.url, on: .main) + } + + /// A redirect that crosses from the default context to a modal is re-proposed by + /// the main session (which performed the request). The stranded main controller + /// must be popped so it isn't left orphaned beneath the modal. + func test_redirect_mainToModal_popsOrphanedMainControllerAndPresentsModal() { + navigator.route(oneURL) + navigator.route(twoURL) + XCTAssertEqual(navigationController.viewControllers.count, 2) + + let redirect = VisitProposal(path: "/modal", action: .replace, context: .modal, redirected: true) + navigator.session(session, didProposeVisit: redirect) + + XCTAssertEqual(navigationController.viewControllers.count, 1) + XCTAssertEqual(modalNavigationController.viewControllers.count, 1) + XCTAssertIdentical(navigationController.presentedViewController, modalNavigationController) + assertVisited(url: redirect.url, on: .modal) + } + + /// A followed redirect within the modal context replaces the pre-redirect + /// controller on the modal stack and must not pop the modal screen beneath it. + func test_redirect_modalToModal_replacesRedirectedControllerAndKeepsUnderlyingModalScreen() { + navigator.route(VisitProposal(path: "/one", context: .modal)) + navigator.route(VisitProposal(path: "/two", context: .modal)) + XCTAssertEqual(modalNavigationController.viewControllers.count, 2) + + let redirect = VisitProposal(path: "/three", action: .replace, context: .modal, redirected: true) + navigator.session(modalSession, didProposeVisit: redirect) + + XCTAssertEqual(modalNavigationController.viewControllers.count, 2) + let underlying = modalNavigationController.viewControllers.first as? VisitableViewController + XCTAssertEqual(underlying?.initialVisitableURL, oneURL) + assertVisited(url: redirect.url, on: .modal) + } + + /// A redirect that crosses from the modal context to the default context is + /// re-proposed by the modal session. The modal is dismissed and the redirected + /// destination is routed onto the main stack. + func test_redirect_modalToMain_dismissesModalAndRoutesOntoMainStack() { + navigator.route(oneURL) + navigator.route(VisitProposal(path: "/modal", context: .modal)) + XCTAssertIdentical(navigationController.presentedViewController, modalNavigationController) + + let redirect = VisitProposal(path: "/three", action: .replace, context: .default, redirected: true) + navigator.session(modalSession, didProposeVisit: redirect) + + XCTAssertEqual(navigationController.viewControllers.count, 1) + assertVisited(url: redirect.url, on: .main) + XCTAssertNil(navigationController.presentedViewController) + } + // MARK: Private private enum Context { @@ -588,13 +657,15 @@ extension VisitProposal { action: VisitAction = .advance, context: Navigation.Context = .default, presentation: Navigation.Presentation = .default, + redirected: Bool = false, additionalProperties: [String: AnyHashable] = [:]) { let baseURL = URL(string: "https://example.com")! var components = URLComponents(url: baseURL, resolvingAgainstBaseURL: false) components?.path = path.hasPrefix("/") ? path : "/\(path)" components?.queryItems = queryItems let url = components!.url! - let options = VisitOptions(action: action, response: nil) + let response = redirected ? VisitResponse(statusCode: 200, redirected: true) : nil + let options = VisitOptions(action: action, response: response) let defaultProperties: PathProperties = [ "context": context.rawValue, "presentation": presentation.rawValue