[MOB-3221] Native SRPV Analytics

[lucaschifino]

MOB-3221

(reopening #870 after it got closed due to the main branch migration)

Context

We want to have Native SRPV we can use to base retention metrics of.

Approach

• Investigate what's the best place to track any web navigation (i.e. change in url) - see section below
• Create Analytics unstructured event and extract info from url using utils methods
• Put event behind a feature flag so we can control who receives it

Finding where to track

Finding out exactly where in code is the best place to track this url can was trickier than expected, as the tab flow is quite complex and changes might happens on multiple places. Here are some attempts worth documenting:

1. The first attempt was using webView(_:decidePolicyFor:decisionHandler:) as that is the closest we have from a direct webview delegate method. This did in fact trigger whenever we wanted, but also triggered extra times like with different web redirects or, more problematically, multiple times at once like on launch.
2. Once 1 did not fit, a number of other candidates where attempted like for example using the observed URL value change which in turn triggered the tab event handler. They seem great but in the end have the same core issue of being triggered multiple times from different places.
3. To solve the multiple triggers, I resorted to the main place where I could guarantee no events would be double fired: the url bar view setter. There I can actually compare the new and old values and guarantee that the event is only tracked once (even if this is also triggered multiple time). This is also where we currently ecosify our urls, so we can combo both changes together.

Some things that still trigger the event (if the url matches) but there's not much way around without over-complicating:

• Back and forward button (But only if url changes, this is an extra validation from us and why refreshes do not trigger)
• Launching the app (Resume doesn't trigger though)
• Switching tabs

Other

URL Tests

Took the opportunity to organise them and mark, besides adding the new ones.

Flaky tests

Skipped some more flaky tests that are troubling us for a while and added them to MOB-3030 so they can be checked later.

Before merging

Checklist

• I performed some relevant testing on a real device and/or simulator
• I wrote Unit Tests that confirm the expected behaviour
• I added the // Ecosia: helper comments where needed
• I made sure that any change to the Analytics events included in PR won't alter current analytics (e.g. new users, upgrading users)

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 50b66e7)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Comment Clarity The new URL setter includes commented-out code and inline comments that may be confusing. Consider cleaning up or clarifying comments to ensure maintainability. /* Ecosia: Change setter to update URL accordingly and track search event when needed locationView.url = newURL */ let oldURL = currentURL var updatedUrl = newURL // Ecosify if needed if updatedUrl?.shouldEcosify() ?? false { updatedUrl = newURL?.ecosified(isIncognitoEnabled: isPrivate) } locationView.url = updatedUrl // Track search if url changed and is Ecosia's vertical // (this has to be done after ecosifying so we properly track only if changed) if let updatedUrl = updatedUrl, oldURL != updatedUrl, updatedUrl.isEcosiaSearchVertical() { Analytics.shared.inappSearch(url: updatedUrl) } // Update constraints if needed if !inOverlayMode { URL Query Handling The addition of new enums and helper methods for query parameters alters URL processing. Extra care should be taken to validate that existing URL behavior remains consistent. import Foundation extension URL { public enum EcosiaQueryItemName: String { case page = "p", query = "q", typeTag = "tt", userId = "_sp" } public enum EcosiaSearchVertical: String, CaseIterable { case search case images case news case videos init?(path: String) { let pathWithNoLeadingSlash = String(path.dropFirst()) self.init(rawValue: pathWithNoLeadingSlash) } } public static func ecosiaSearchWithQuery(_ query: String, urlProvider: URLProvider = Environment.current.urlProvider) -> URL { var components = URLComponents(url: urlProvider.root, resolvingAgainstBaseURL: false)! components.path = "/search" components.queryItems = [item(name: .query, value: query), item(name: .typeTag, value: "iosapp")] return components.url! } /// Check whether the URL being browsed will present the SERP out of a search or a search suggestion public func isEcosiaSearchQuery(_ urlProvider: URLProvider = Environment.current.urlProvider) -> Bool { guard isEcosia(urlProvider), let components = URLComponents(url: self, resolvingAgainstBaseURL: false) else { return false } return components.path == "/search" } public func isEcosiaSearchVertical(_ urlProvider: URLProvider = Environment.current.urlProvider) -> Bool { getEcosiaSearchVerticalPath(urlProvider) != nil } public func getEcosiaSearchVerticalPath(_ urlProvider: URLProvider = Environment.current.urlProvider) -> String? { guard isEcosia(urlProvider), let components = components else { return nil } return EcosiaSearchVertical(path: components.path)?.rawValue } public func getEcosiaSearchQuery(_ urlProvider: URLProvider = Environment.current.urlProvider) -> String? { guard isEcosia(urlProvider), let components = components else { return nil } return components.queryItems?.first(where: { $0.name == EcosiaQueryItemName.query.rawValue })?.value } public func getEcosiaSearchPage(_ urlProvider: URLProvider = Environment.current.urlProvider) -> Int? { guard isEcosia(urlProvider), let components = components else { return nil } if let pageNumber = components.queryItems?.first(where: { $0.name == EcosiaQueryItemName.page.rawValue })?.value { return Int(pageNumber) } return nil } /// Check whether the URL should be Ecosified. At the moment this is true for every Ecosia URL. public func shouldEcosify(_ urlProvider: URLProvider = Environment.current.urlProvider) -> Bool { return isEcosia(urlProvider) } public func ecosified(isIncognitoEnabled: Bool, urlProvider: URLProvider = Environment.current.urlProvider) -> URL { guard isEcosia(urlProvider), var components = components else { return self } components.queryItems?.removeAll(where: { $0.name == EcosiaQueryItemName.userId.rawValue }) var items = components.queryItems ?? .init() /* The `sendAnonymousUsageData` is set by the native UX component in settings that determines whether the app would send the events to Snowplow. To align the business logic, this parameter will also function as a condition that decides whether we would send our AnalyticsID as query paramter for searches. In this scenario thuogh, the naming is a bit misleanding, thus checking for the negative evaluation of it. */ let shouldAnonymizeUserId = isIncognitoEnabled || !User.shared.hasAnalyticsCookieConsent || !User.shared.sendAnonymousUsageData let userId = shouldAnonymizeUserId ? UUID(uuid: UUID_NULL).uuidString : User.shared.analyticsId.uuidString items.append(Self.item(name: .userId, value: userId)) components.queryItems = items return components.url! } public var policy: Scheme.Policy { (scheme .flatMap(Scheme.init(rawValue:)) ?? .other) .policy } private subscript(_ itemName: EcosiaQueryItemName) -> String? { components?.queryItems?.first { $0.name == itemName.rawValue }?.value } private func isEcosia(_ urlProvider: URLProvider = Environment.current.urlProvider) -> Bool { guard let domain = urlProvider.domain else { return false } let isBrowser = scheme.flatMap(Scheme.init(rawValue:))?.isBrowser == true let hasURLProviderDomainSuffix = host?.hasSuffix(domain) == true return isBrowser && hasURLProviderDomainSuffix } private var components: URLComponents? { URLComponents(url: self, resolvingAgainstBaseURL: false) } private static func item(name: EcosiaQueryItemName, value: String) -> URLQueryItem { .init(name: name.rawValue, value: value) } Build Configuration The changes switching the code sign style to Manual and adding empty keys for DEVELOPMENT_TEAM and PROVISIONING_PROFILE_SPECIFIER should be reviewed to ensure they align with the build and deployment workflows. CLANG_WARN_UNGUARDED_AVAILABILITY = YES_AGGRESSIVE; CLANG_WARN_UNREACHABLE_CODE = YES; CLANG_WARN__DUPLICATE_METHOD_MATCH = YES; CODE_SIGN_STYLE = Manual; COPY_PHASE_STRIP = NO; CURRENT_PROJECT_VERSION = 1; DEBUG_INFORMATION_FORMAT = dwarf; DEVELOPMENT_TEAM = ""; ENABLE_STRICT_OBJC_MSGSEND = YES;

[lucaschifino]

Had to change this to run EcosiaTests locally since it was with Automatic signing enabled.

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 50b66e7
Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Add missing variant property Implement a variant property similar to other settings (e.g. UnleashAPNConsent) to enable proper feature toggling. firefox-ios/Client/Ecosia/Settings/EcosiaDebugSettings.swift [227-235] final class UnleashNativeSRPVAnalyticsSetting: UnleashVariantResetSetting { override var titleName: String? { "Native SRPV Analytics" + } + + override var variant: Unleash.Variant? { + Unleash.getVariant(.nativeSRPVAnalytics) } override var unleashEnabled: Bool? { Unleash.isEnabled(.nativeSRPVAnalytics) } } Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies that the UnleashNativeSRPVAnalyticsSetting is missing a variant property like its counterpart (UnleashAPNConsent), and the improved code accurately integrates this change, enhancing feature toggling.	Medium
	Configure code signing details Supply valid DEVELOPMENT_TEAM and PROVISIONING_PROFILE_SPECIFIER values when using manual code signing to prevent build errors. firefox-ios/Client.xcodeproj/project.pbxproj [25529-25560] CODE_SIGN_STYLE = Manual; -DEVELOPMENT_TEAM = ""; -PROVISIONING_PROFILE_SPECIFIER = ""; +DEVELOPMENT_TEAM = "YourTeamID"; +PROVISIONING_PROFILE_SPECIFIER = "YourProvisioningProfile"; Suggestion importance[1-10]: 6 __ Why: The suggestion points out that leaving DEVELOPMENT_TEAM and PROVISIONING_PROFILE_SPECIFIER empty might lead to build issues; however, the improved code provides placeholder values that require further manual customization per project needs.	Low

---

Previous suggestions

Suggestions up to commit e48eed8

Category	Suggestion	Impact
General	Clean commented setter code Remove or refactor the block comment and any unused commented-out code in the setter to improve code clarity. firefox-ios/Client/Frontend/Toolbar+URLBar/URLBarView.swift [251-271] set { - /* Ecosia: Change setter to update URL accordingly and track search event when needed - locationView.url = newURL - */ let oldURL = currentURL var updatedUrl = newURL - // Ecosify if needed if updatedUrl?.shouldEcosify() ?? false { updatedUrl = newURL?.ecosified(isIncognitoEnabled: isPrivate) } locationView.url = updatedUrl - // Track search if url changed and is Ecosia's vertical if let updatedUrl = updatedUrl, oldURL != updatedUrl, updatedUrl.isEcosiaSearchVertical() { Analytics.shared.inappSearch(url: updatedUrl) } - // Update constraints if needed if !inOverlayMode { setNeedsUpdateConstraints() } } Suggestion importance[1-10]: 3 __ Why: The suggestion improves code clarity by removing commented-out code in the setter, though its impact is minor.	Low

[github-actions]

Persistent review updated to latest commit 50b66e7

[d4r1091]

All good and I highlighted some of the implementation and polishment here and there! Asked some clarifying comments before giving final 👍 💪
Thanks!

[d4r1091]

Should we use our standard // Ecosia: and /* Ecosia */ patter here and below or is there a special case for this series of comments in this snippet?

[lucaschifino]

Above this code, on the first line of the setter, we already have

/* Ecosia: Change setter to update URL accordingly and track search event when needed
  locationView.url = newURL
*/

Everything below that then is already Ecosia code. This is already similar to what we had before but I added more code there and included some explaining comments. To try and make this more visible I left all of the code together with no separating blank lines. Do yo think that's clear enough? Or would you suggest we have multiple Ecosia comments?

[d4r1091]

Thanks for the explanation!
I thought there was a reason and the second part of that comment made me feel so 👌 .
The only "challenge" I see is from a person willing to contribute (beyond us today with an history on making such contribution) and reading the Ecosia comment section. If in the next upgrade for any reason it won't be us working on it, the fellow Ecosian may discard such comments as not following the pattern and seeing the "conflict" raised as a reason to, by default, discard it and follow Firefox's approach.

[lucaschifino]

Yeah, you're right, will add them even if there's the one on top 🙂

[lucaschifino]

Done! ✅

[d4r1091]

Thanks for the updates! 🙏 🚀
Will fix the production build issues then will be available for being merged 💪