Conversation
There was a problem hiding this comment.
Pull request overview
This PR removes a workaround that was previously intercepting sign-in URLs and redirecting them to make users appear logged in. With backend fixes in place (JOU team changes including Terraform updates), the refresh token is now consistently retrieved. Both sign-in and sign-up flows now use the same native Auth0 authentication flow.
Changes:
- Removed sign-in URL interception logic from
EcosiaWebViewModalthat was redirecting to a modified URL - Unified sign-in and sign-up handling in
BrowserViewController+Ecosiato use the same native Auth0 flow - Enabled AI Search feature unconditionally for iOS 16+ (appears unrelated to the PR's stated purpose)
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
firefox-ios/Ecosia/UI/Account/EcosiaWebViewModal.swift |
Removed the sign-in URL interceptor that was redirecting URLs with returnTo parameter |
firefox-ios/Client/Ecosia/Extensions/BrowserViewController+Ecosia.swift |
Unified sign-in and sign-up handling into a single method, removed separate sign-in redirection logic |
firefox-ios/Client/Ecosia/UI/NTP/Header/NTPHeaderViewModel.swift |
Changed AI Search feature flag from experiment-based to always enabled for iOS 16+ |
| var isEnabled: Bool { | ||
| if #available(iOS 16.0, *) { | ||
| return AISearchMVPExperiment.isEnabled | ||
| return true |
There was a problem hiding this comment.
This change from AISearchMVPExperiment.isEnabled to true appears unrelated to the PR's stated purpose of fixing sign-in behavior. This change enables AI Search functionality unconditionally (for iOS 16+), which seems to be a separate feature rollout.
If this is intentional and part of this PR, it should be mentioned in the PR description. Otherwise, this change should be moved to a separate PR focused on the AI Search feature rollout.
There was a problem hiding this comment.
That's a good question, can you elaborate on what has changed here? I guess since the header has accounts we want to always show it and the AI condition is now inside the view, right?
What about the iOS 16 condition?
There was a problem hiding this comment.
That is correct @lucaschifino on the assumption. I would say that was a bug. The Account one was depending on the AISearchMVPExperiment flag. Both SwiftUI under the same "wrapper".
Considering the AI button visibility being covered already by the
isEnabled flag would only depend on the iOS16 (SwiftUI).
| var isEnabled: Bool { | ||
| if #available(iOS 16.0, *) { | ||
| return AISearchMVPExperiment.isEnabled | ||
| return true |
There was a problem hiding this comment.
That's a good question, can you elaborate on what has changed here? I guess since the header has accounts we want to always show it and the AI condition is now inside the view, right?
What about the iOS 16 condition?
…sia.swift Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* [MOB-4154] Add logs * [MOB-4154] Enable accounts by default * [MOB-4154] Revert "[MOB-4154] Add logs" This reverts commit af489be. * [MOB-4154] Remove SignIn workaround. Intercept SignIn * Update firefox-ios/Client/Ecosia/Extensions/BrowserViewController+Ecosia.swift Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> # Conflicts: # firefox-ios/Client/Ecosia/Extensions/BrowserViewController+Ecosia.swift
MOB-4154
Context
Previously, a workaround was implemented in
EcosiaWebViewModalto intercept sign-in URLs and redirect them, ensuring users always appeared logged in. This approach led to inconsistencies between the sign-in and sign-up flows when Web started to implement theSign Inbutton. However, after a fix from JOU (including some Terraform changes), we managed to consistently get the refresh token, so we no longer need the workaround and make the interceptor open the native Auth0 page for both Sign-In and Sign-Up.Approach
Sign-InSign-Inhandle the Auth0 native page openingOther
Before merging
Checklist