[MOB-4254] Handle same url search event#1084
Merged
lucaschifino merged 3 commits intomainfrom Mar 19, 2026
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
Adjusts Ecosia in-app search analytics tracking to correctly fire on same-URL navigations (e.g., re-selecting the current search vertical), by removing the prior previousUrl suppression that was preventing legitimate events. This fits into the Ecosia fork’s navigation/analytics integration by keeping the “eligible in decidePolicyFor, fire in didCommit” bridge while simplifying the eligibility logic.
Changes:
- Remove
previousUrl-based suppression and simplifyecosiaHandleNavigationActionto only suppress.backForward. - Update
WKNavigationDelegateintegration to use the simplified tracking helper signature. - Update/add unit tests to assert same-URL tracking behavior and back/forward suppression.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| firefox-ios/EcosiaTests/Analytics/AnalyticsSpyTests.swift | Updates existing webview-tracking tests and adds a new test covering same-URL navigation via link activation. |
| firefox-ios/Client/Frontend/Browser/BrowserViewController/Views/BrowserViewController.swift | Removes the previousUrl state previously used to suppress duplicate tracking. |
| firefox-ios/Client/Frontend/Browser/BrowserViewController/Extensions/BrowserViewController+WebViewDelegates.swift | Updates the delegate flow to call the simplified navigation tracking helper. |
| firefox-ios/Client/Ecosia/Extensions/BrowserViewController+EcosiaNavigationHandling.swift | Simplifies tracking eligibility: always track Ecosia search vertical navigations except .backForward. |
You can also share your feedback on Copilot code review. Take the survey.
d4r1091
approved these changes
Mar 19, 2026
Member
d4r1091
left a comment
d4r1091
left a comment
There was a problem hiding this comment.
If I understand correctly, we moved the "previous url" tracking as well 👍 .
All good!
lucaschifino
force-pushed
the
ls-mob-4254-same-url-search-event
branch
from
8e049c3 to
177c950
Compare
lucaschifino
added a commit
that referenced
this pull request
Mar 24, 2026
* [MOB-4254] Handle tab restore and navigation type * [MOB-4254] Add new test on AnalyticsSpyTests * [MOB-4254] Remove URL check
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
MOB-4254
Context
The
previousUrlcheck was introduced to prevent tab switching from re-firing the inapp search event. In practice, tab switching never reachesdidCommit— WKWebView handles it at the view level withouttriggering navigation delegates. The check was solving a phantom problem, and as a side effect was incorrectly suppressing legitimate same-URL navigations (e.g. tapping a search vertical you're already on).
Depends on #1083 being merged first ❗
Approach
Remove url check and simplified logic.
The only guard retained is
!isBackForward, matching web behaviour where bfcache keeps the page mounted across back/forward so Vue never refires.Before merging
Checklist
// Ecosia:helper comments where needed