Skip to content
This repository has been archived by the owner on Mar 6, 2024. It is now read-only.

Commit

Permalink
Fixes #3557 The search counts telemetry is recording partial data
Browse files Browse the repository at this point in the history
  • Loading branch information
razvanlitianu committed Nov 11, 2022
1 parent 28d5e6f commit f5dd762
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 28 deletions.
8 changes: 4 additions & 4 deletions Blockzilla/AppDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ class AppDelegate: UIResponder, UIApplicationDelegate {
browserViewController.deactivateUrlBarOnHomeView()
browserViewController.dismissSettings()
browserViewController.dismissActionSheet()
browserViewController.submit(url: url)
browserViewController.submit(url: url, source: .action)
queuedUrl = nil
} else if let text = queuedString {
Telemetry.default.recordEvent(category: TelemetryEventCategory.action, method: TelemetryEventMethod.openedFromExtension, object: TelemetryEventObject.app)
Expand All @@ -245,9 +245,9 @@ class AppDelegate: UIResponder, UIApplicationDelegate {
browserViewController.dismissActionSheet()

if let fixedUrl = URIFixup.getURL(entry: text) {
browserViewController.submit(url: fixedUrl)
browserViewController.submit(url: fixedUrl, source: .action)
} else {
browserViewController.submit(text: text)
browserViewController.submit(text: text, source: .action)
}

queuedString = nil
Expand Down Expand Up @@ -284,7 +284,7 @@ class AppDelegate: UIResponder, UIApplicationDelegate {
browserViewController.resetBrowser(hidePreviousSession: true)
browserViewController.ensureBrowsingMode()
browserViewController.deactivateUrlBarOnHomeView()
browserViewController.submit(url: url)
browserViewController.submit(url: url, source: .action)
Telemetry.default.recordEvent(category: TelemetryEventCategory.action, method: TelemetryEventMethod.siri, object: TelemetryEventObject.openFavoriteSite)
GleanMetrics.Siri.openFavoriteSite.record()
case "EraseIntent":
Expand Down
38 changes: 23 additions & 15 deletions Blockzilla/BrowserViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class BrowserViewController: UIViewController {
private lazy var webViewController: WebViewController = {
var menuAction = WebMenuAction.live
menuAction.openLink = { url in
self.submit(url: url)
self.submit(url: url, source: .action)
}
return WebViewController(trackingProtectionManager: trackingProtectionManager, webMenuAction: menuAction)
}()
Expand Down Expand Up @@ -286,7 +286,7 @@ class BrowserViewController: UIViewController {
guard shouldEnsureBrowsingMode else { return }
ensureBrowsingMode()
guard let url = initialUrl else { return }
submit(url: url)
submit(url: url, source: .action)
}

override func viewWillAppear(_ animated: Bool) {
Expand Down Expand Up @@ -850,7 +850,7 @@ class BrowserViewController: UIViewController {
shouldEnsureBrowsingMode = false
}

func submit(text: String) {
func submit(text: String, source: Source) {
var url = URIFixup.getURL(entry: text)
if url == nil {
Telemetry.default.recordEvent(category: TelemetryEventCategory.action, method: TelemetryEventMethod.typeQuery, object: TelemetryEventObject.searchBar)
Expand All @@ -861,14 +861,22 @@ class BrowserViewController: UIViewController {
}

if let url = url {
submit(url: url)
submit(url: url, source: source)
}
}

func submit(url: URL) {
fileprivate func recordSearchEvent(_ source: Source) {
let identifier = searchEngineManager.activeEngine.getNameOrCustom()
let source = source.rawValue
GleanMetrics.BrowserSearch.searchCount["\(identifier).\(source)"].add()
}

func submit(url: URL, source: Source) {
// If this is the first navigation, show the browser and the toolbar.
guard isViewLoaded else { initialUrl = url; return }
GleanMetrics.BrowserSearch.searchCount["action"].add()

recordSearchEvent(source)

shortcutsPresenter.shortcutsState = .none
SearchInContentTelemetry.shouldSetUrlTypeSearch = true
if isIPadRegularDimensions {
Expand Down Expand Up @@ -1149,7 +1157,7 @@ extension BrowserViewController: UIDropInteractionDelegate {

self.ensureBrowsingMode()
self.urlBar.fillUrlBar(text: url.absoluteString)
self.submit(url: url)
self.submit(url: url, source: .action)
Telemetry.default.recordEvent(category: TelemetryEventCategory.action, method: TelemetryEventMethod.drop, object: TelemetryEventObject.searchBar)
GleanMetrics.UrlInteraction.dropEnded.record()
}
Expand Down Expand Up @@ -1267,7 +1275,7 @@ extension BrowserViewController: URLBarDelegate {
}
}

func urlBar(_ urlBar: URLBar, didSubmitText text: String) {
func urlBar(_ urlBar: URLBar, didSubmitText text: String, source: Source) {
let text = text.trimmingCharacters(in: .whitespaces)

guard !text.isEmpty else {
Expand All @@ -1287,7 +1295,7 @@ extension BrowserViewController: URLBarDelegate {
Telemetry.default.recordEvent(category: TelemetryEventCategory.action, method: TelemetryEventMethod.typeURL, object: TelemetryEventObject.searchBar)
}
if let urlBarURL = url {
submit(url: urlBarURL)
submit(url: urlBarURL, source: source)
urlBar.url = urlBarURL
}

Expand Down Expand Up @@ -1501,7 +1509,7 @@ extension BrowserViewController: HomeViewControllerDelegate {
deactivateUrlBarOnHomeView()
dismissSettings()
dismissActionSheet()
submit(url: url)
submit(url: url, source: .action)
}

func homeViewControllerDidTapTip(_ controller: HomeViewController, tip: TipManager.Tip) {
Expand Down Expand Up @@ -1541,7 +1549,7 @@ extension BrowserViewController: OverlayViewDelegate {
if searchEngineManager.activeEngine.urlForQuery(query) != nil {
Telemetry.default.recordEvent(category: TelemetryEventCategory.action, method: TelemetryEventMethod.selectQuery, object: TelemetryEventObject.searchBar)
Telemetry.default.recordSearch(location: .actionBar, searchEngine: searchEngineManager.activeEngine.getNameOrCustom())
urlBar(urlBar, didSubmitText: query)
urlBar(urlBar, didSubmitText: query, source: .topsite)
}

urlBar.dismiss()
Expand Down Expand Up @@ -1584,7 +1592,7 @@ extension BrowserViewController: OverlayViewDelegate {
Telemetry.default.recordEvent(category: TelemetryEventCategory.action, method: TelemetryEventMethod.typeURL, object: TelemetryEventObject.searchBar)
}
if let overlayURL = url {
submit(url: overlayURL)
submit(url: overlayURL, source: .suggestion)
urlBar.url = overlayURL
}
urlBar.dismiss()
Expand Down Expand Up @@ -1992,11 +2000,11 @@ extension BrowserViewController: MenuActionable {
}

func showHelp() {
submit(text: "https://support.mozilla.org/en-US/products/focus-firefox/Focus-ios")
submit(text: "https://support.mozilla.org/en-US/products/focus-firefox/Focus-ios", source: .action)
}

func showWhatsNew() {
submit(url: URL(forSupportTopic: .whatsNew))
submit(url: URL(forSupportTopic: .whatsNew), source: .action)
}

func showCopy(url: URL) {
Expand Down Expand Up @@ -2061,7 +2069,7 @@ extension BrowserViewController {
ensureBrowsingMode()
urlBar.url = url
deactivateUrlBarOnHomeView()
submit(url: url)
submit(url: url, source: .shortcut)
GleanMetrics.Shortcuts.shortcutOpenedCounter.add()
}

Expand Down
9 changes: 4 additions & 5 deletions Blockzilla/NavigationPath.swift
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ enum NavigationPath {

private static func handleURL(_ application: UIApplication, url: URL, with browserViewController: BrowserViewController) -> URL? {
if application.applicationState == .active {
browserViewController.submit(url: url)
browserViewController.submit(url: url, source: .action)
}
else { return url }
return nil
Expand All @@ -85,10 +85,9 @@ enum NavigationPath {
private static func handleText(_ application: UIApplication, text: String, with browserViewController: BrowserViewController) -> String? {
if application.applicationState == .active {
if let fixedUrl = URIFixup.getURL(entry: text) {
browserViewController.submit(url: fixedUrl)
}
else {
browserViewController.submit(text: text)
browserViewController.submit(url: fixedUrl, source: .action)
} else {
browserViewController.submit(text: text, source: .action)
}
}
else { return text }
Expand Down
10 changes: 7 additions & 3 deletions Blockzilla/UIComponents/URLBar.swift
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,13 @@ import SnapKit
import Telemetry
import Glean

enum Source: String {
case action, shortcut, suggestion, topsite, widget, none
}

protocol URLBarDelegate: AnyObject {
func urlBar(_ urlBar: URLBar, didEnterText text: String)
func urlBar(_ urlBar: URLBar, didSubmitText text: String)
func urlBar(_ urlBar: URLBar, didSubmitText text: String, source: Source)
func urlBar(_ urlBar: URLBar, didAddCustomURL url: URL)
func urlBarDidActivate(_ urlBar: URLBar)
func urlBarDidDeactivate(_ urlBar: URLBar)
Expand Down Expand Up @@ -462,7 +466,7 @@ class URLBar: UIView {
@objc func pasteAndGo(clipboardString: String) {
isEditing = true
delegate?.urlBarDidActivate(self)
delegate?.urlBar(self, didSubmitText: clipboardString)
delegate?.urlBar(self, didSubmitText: clipboardString, source: .action)

Telemetry.default.recordEvent(category: TelemetryEventCategory.action, method: TelemetryEventMethod.click, object: TelemetryEventObject.pasteAndGo)
GleanMetrics.UrlInteraction.pasteAndGo.record()
Expand Down Expand Up @@ -901,7 +905,7 @@ extension URLBar: AutocompleteTextFieldDelegate {
}
userInputText = nil

delegate?.urlBar(self, didSubmitText: autocompleteTextField.text ?? "")
delegate?.urlBar(self, didSubmitText: autocompleteTextField.text ?? "", source: .action)

if Settings.getToggle(.enableSearchSuggestions) {
Telemetry.default.recordEvent(TelemetryEvent(category: TelemetryEventCategory.action, method: TelemetryEventMethod.click, object: TelemetryEventObject.searchSuggestionNotSelected))
Expand Down
4 changes: 3 additions & 1 deletion Blockzilla/metrics.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -554,7 +554,9 @@ browser_search:
the name of the search engine. If it's a custom search engine (defined:
https://github.com/mozilla-mobile/fenix/issues/1607) the value will be
`custom`.
`source` will be: `action`, `suggestion`.
`source` will be: `action`, `suggestion`, `widget`, `shortcut`, `topsite`
(depending on the source from which the search started). Also added the
`other` option for the source but it should never enter on this case.
send_in_pings:
- metrics
- baseline
Expand Down

0 comments on commit f5dd762

Please sign in to comment.