Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions iOS/Core/Pixel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ public struct PixelParameters {

public static let count = "count"
public static let source = "source"
public static let browsingMode = "browsing_mode"
public static let authVersion = "authVersion"
public static let lastUsed = "last_used"

Expand Down
36 changes: 36 additions & 0 deletions iOS/Core/PixelEvent.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1733,6 +1733,24 @@ extension Pixel {
case webExtensionAdBlockingInstalled
case webExtensionAdBlockingUpgraded
case webExtensionAdBlockingInstallError

// MARK: - Fire Mode
case fireModeNTPPromotionShown
case fireModeNTPPromotionDismissed
case fireModeNTPPromotionEngaged
case fireModeMenuPromotionShown
case fireModeMenuPromotionEngaged
case fireModeSwitched
case fireModeBurnExecuted
case normalModeBurnExecuted
case fireModeDataCleared
case normalModeDataCleared
case fireModeLastTabClosedBurn
case fireModeEmptyStateNewTab
case linkLongPressMenuShown
case linkLongPressNewTab
case linkLongPressBackgroundTab
case linkLongPressFireTab
}

}
Expand Down Expand Up @@ -3377,6 +3395,24 @@ extension Pixel.Event {
case .webExtensionAdBlockingInstalled: return "m_web_extension_ad_blocking_installed"
case .webExtensionAdBlockingUpgraded: return "m_web_extension_ad_blocking_upgraded"
case .webExtensionAdBlockingInstallError: return "m_web_extension_ad_blocking_install_error"

// MARK: - Fire Mode
case .fireModeNTPPromotionShown: return "m_fire-mode_ntp-promotion_shown"
case .fireModeNTPPromotionDismissed: return "m_fire-mode_ntp-promotion_dismissed"
case .fireModeNTPPromotionEngaged: return "m_fire-mode_ntp-promotion_engaged"
case .fireModeMenuPromotionShown: return "m_fire-mode_menu-promotion_shown"
case .fireModeMenuPromotionEngaged: return "m_fire-mode_menu-promotion_engaged"
case .fireModeSwitched: return "m_fire-mode_switched"
case .fireModeBurnExecuted: return "m_fire-mode_burn_executed"
case .normalModeBurnExecuted: return "m_normal-mode_burn_executed"
case .fireModeDataCleared: return "m_fire-mode_data-cleared"
case .normalModeDataCleared: return "m_normal-mode_data-cleared"
case .fireModeLastTabClosedBurn: return "m_fire-mode_last-tab-closed_burn"
case .fireModeEmptyStateNewTab: return "m_fire-mode_empty-state_new-tab"
case .linkLongPressMenuShown: return "m_link-long-press_menu-shown"
case .linkLongPressNewTab: return "m_link-long-press_new-tab"
case .linkLongPressBackgroundTab: return "m_link-long-press_background-tab"
case .linkLongPressFireTab: return "m_link-long-press_fire-tab"
}
}
}
Expand Down
20 changes: 20 additions & 0 deletions iOS/DuckDuckGo/BrowsingMode.swift
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,24 @@ public enum BrowsingMode: Int, CaseIterable {
return false
}
}

var pixelParamValue: String {
switch self {
case .fire:
return "fire"
case .normal:
return "normal"
}
}
}

extension Tab {
var pixelParamValue: String {
switch fireTab {
case true:
return "fire"
case false:
return "normal"
}
}
Comment thread
cursor[bot] marked this conversation as resolved.
}
10 changes: 6 additions & 4 deletions iOS/DuckDuckGo/Fire/FireExecutor.swift
Original file line number Diff line number Diff line change
Expand Up @@ -397,9 +397,10 @@ class FireExecutor: FireExecuting {
switch scope {
case .tab:
return TimedPixel(.singleTabDataCleared)
case .fireMode, .normalMode:
// TODO: - return new pixel
return nil
case .fireMode:
return TimedPixel(.fireModeDataCleared)
case .normalMode:
return TimedPixel(.normalModeDataCleared)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a note here that we’ll need to make adjustments to our dashboards as this will stop sending the forgetAllDataCleared pixel for users who have the fire mode enabled.
Also - I see the mode param is added, so is it necessary to have a separate pixel for each mode?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Synced on this, and we agreed to leave it as is. Added a comment, though, to clarify the intention of the normalMode scope.

case .all:
return TimedPixel(.forgetAllDataCleared)
}
Expand All @@ -413,7 +414,8 @@ class FireExecutor: FireExecuting {
let tabType = viewModel.tab.isAITab ? "ai" : "web"
return [
PixelParameters.tabType: tabType,
PixelParameters.domainsCount: "\(domains?.count ?? 0)"
PixelParameters.domainsCount: "\(domains?.count ?? 0)",
PixelParameters.browsingMode: viewModel.tab.pixelParamValue
]
case .fireMode:
tabsModel = self.tabManager.tabsModel(for: .fire)
Expand Down
10 changes: 5 additions & 5 deletions iOS/DuckDuckGo/FireMode/FireModePromotionsCoordinator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -114,17 +114,17 @@ final class FireModePromotionsCoordinator: FireModePromotionCoordinating {
if firstSeenDate == nil {
firstSeenDate = Date()
}
// TODO: fire promotion shown pixel
DailyPixel.fireDailyAndCount(pixel: .fireModeNTPPromotionShown)
}

func markNTPPromotionDismissed() {
isDismissed = true
// TODO: fire promotion dismissed pixel
Pixel.fire(pixel: .fireModeNTPPromotionDismissed)
}

func markNTPPromotionEngaged() {
isEngaged = true
// TODO: fire promotion engaged pixel
Pixel.fire(pixel: .fireModeNTPPromotionEngaged)
}

// MARK: - Menu Promotion
Expand Down Expand Up @@ -153,12 +153,12 @@ final class FireModePromotionsCoordinator: FireModePromotionCoordinating {
menuPromotionFirstShownDate = Date()
}
menuPromotionShownCount += 1
// TODO: fire menu promotion shown pixel
DailyPixel.fireDailyAndCount(pixel: .fireModeMenuPromotionShown)
}

func markMenuPromotionEngaged() {
menuPromotionEngaged = true
// TODO: fire menu promotion engaged pixel
Pixel.fire(pixel: .fireModeMenuPromotionEngaged)
}

// MARK: - Private
Expand Down
79 changes: 48 additions & 31 deletions iOS/DuckDuckGo/MainViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1548,6 +1548,7 @@
let bucket = HomePageDisplayDailyPixelBucket(favoritesCount: favoritesCount)
DailyPixel.fire(pixel: .newTabPageDisplayedDaily, withAdditionalParameters: [
"FavoriteCount": bucket.value,
PixelParameters.browsingMode: tabManager.currentBrowsingMode.pixelParamValue
])
}

Expand Down Expand Up @@ -1577,8 +1578,9 @@
)
}

Pixel.fire(pixel: .forgetAllPressedBrowsing)
DailyPixel.fire(pixel: .forgetAllPressedBrowsingDaily)
let browsingModeParam = [PixelParameters.browsingMode: tabManager.currentBrowsingMode.pixelParamValue]
Pixel.fire(pixel: .forgetAllPressedBrowsing, withAdditionalParameters: browsingModeParam)
DailyPixel.fire(pixel: .forgetAllPressedBrowsingDaily, withAdditionalParameters: browsingModeParam)

performActionIfAITab { DailyPixel.fireDailyAndCount(pixel: .aiChatFireButtonTapped) }

Expand Down Expand Up @@ -3029,8 +3031,8 @@
action()
}

func navigateToFireMode() {
tabManager.setBrowsingMode(.fire)
func navigateToFireMode(source: FireModeSwitchSource = .internal) {
tabManager.setBrowsingMode(.fire, source: source)
showTabSwitcher()
}
}
Expand Down Expand Up @@ -3401,14 +3403,15 @@

tab.didLaunchBrowsingMenu()

let modeParam = [PixelParameters.browsingMode: tabManager.currentBrowsingMode.pixelParamValue]
switch context {
case .newTabPage:
Pixel.fire(pixel: .browsingMenuOpenedNewTabPage)
case .aiChatTab:
Pixel.fire(pixel: .browsingMenuOpened)
Comment thread
cursor[bot] marked this conversation as resolved.
Pixel.fire(pixel: .browsingMenuOpened, withAdditionalParameters: modeParam)
DailyPixel.fireDailyAndCount(pixel: .aiChatSettingsMenuOpened)
case .website:
Pixel.fire(pixel: .browsingMenuOpened)
Pixel.fire(pixel: .browsingMenuOpened, withAdditionalParameters: modeParam)

if tab.isError {
Pixel.fire(pixel: .browsingMenuOpenedError)
Expand Down Expand Up @@ -3565,16 +3568,17 @@
aiChat: .keyboardGoWhileOnAIChat)
}

func fireControllerAwarePixel(ntp: Pixel.Event, serp: Pixel.Event, website: Pixel.Event, aiChat: Pixel.Event) {
func fireControllerAwarePixel(ntp: Pixel.Event, serp: Pixel.Event, website: Pixel.Event, aiChat: Pixel.Event,
additionalParameters: [String: String] = [:]) {
if newTabPageViewController != nil {
Pixel.fire(pixel: ntp)
Pixel.fire(pixel: ntp, withAdditionalParameters: additionalParameters)
} else if let currentTab {
if currentTab.isAITab == true {
Pixel.fire(pixel: aiChat)
Pixel.fire(pixel: aiChat, withAdditionalParameters: additionalParameters)
} else if currentTab.url?.isDuckDuckGoSearch == true {
Pixel.fire(pixel: serp)
Pixel.fire(pixel: serp, withAdditionalParameters: additionalParameters)
} else {
Pixel.fire(pixel: website)
Pixel.fire(pixel: website, withAdditionalParameters: additionalParameters)
}
}
}
Expand All @@ -3599,7 +3603,7 @@
if featureFlagger.internalUserDecider.isInternalUser || isDebugBuild {
segueToDebugSettings()
} else {
segueToSettings()

Check failure on line 3606 in iOS/DuckDuckGo/MainViewController.swift

View workflow job for this annotation

GitHub Actions / SwiftLint

Function parameters should be aligned vertically if they're in multiple lines in a declaration (vertical_parameter_alignment)

Check failure on line 3606 in iOS/DuckDuckGo/MainViewController.swift

View workflow job for this annotation

GitHub Actions / SwiftLint

Function parameters should be aligned vertically if they're in multiple lines in a declaration (vertical_parameter_alignment)

Check failure on line 3606 in iOS/DuckDuckGo/MainViewController.swift

View workflow job for this annotation

GitHub Actions / SwiftLint

Function parameters should be aligned vertically if they're in multiple lines in a declaration (vertical_parameter_alignment)

Check failure on line 3606 in iOS/DuckDuckGo/MainViewController.swift

View workflow job for this annotation

GitHub Actions / SwiftLint

Function parameters should be aligned vertically if they're in multiple lines in a declaration (vertical_parameter_alignment)
}
}

Expand Down Expand Up @@ -3649,21 +3653,21 @@
}

private func newTabShortcutAction() {
Pixel.fire(pixel: .tabSwitchLongPressNewTab)
Pixel.fire(pixel: .tabSwitchLongPressNewTab, withAdditionalParameters: [
PixelParameters.browsingMode: tabManager.currentBrowsingMode.pixelParamValue
])
performCancel()
newTab()
}

private func newFireTabLongPressMenuAction() {
// TODO: - Add pixel
tabManager.setBrowsingMode(.fire)
tabManager.setBrowsingMode(.fire, source: .longPressTabsIcon)
performCancel()
newTab()
}

private func newNormalTabLongPressMenuAction() {
// TODO: - Add pixel
tabManager.setBrowsingMode(.normal)
tabManager.setBrowsingMode(.normal, source: .longPressTabsIcon)
performCancel()
newTab()
}
Expand All @@ -3687,10 +3691,12 @@
}

if tapped {
let modeParam = [PixelParameters.browsingMode: tabManager.currentBrowsingMode.pixelParamValue]
fireControllerAwarePixel(ntp: .addressBarClickOnNTP,
serp: .addressBarClickOnSERP,
website: .addressBarClickOnWebsite,
aiChat: .addressBarClickOnAIChat)
aiChat: .addressBarClickOnAIChat,
additionalParameters: modeParam)
}

guard newTabPageViewController == nil else { return }
Expand Down Expand Up @@ -3938,10 +3944,12 @@

// MARK: - Experimental Address Bar (pixels only)
func onExperimentalAddressBarTapped() {
let modeParam = [PixelParameters.browsingMode: tabManager.currentBrowsingMode.pixelParamValue]
fireControllerAwarePixel(ntp: .addressBarClickOnNTP,
serp: .addressBarClickOnSERP,
website: .addressBarClickOnWebsite,
aiChat: .addressBarClickOnAIChat)
aiChat: .addressBarClickOnAIChat,
additionalParameters: modeParam)
}

func onExperimentalAddressBarClearPressed() {
Expand Down Expand Up @@ -3969,7 +3977,9 @@

/// Delegate method called when the omnibar branding area is tapped while in AI Chat mode.
func onAIChatBrandingPressed() {
Pixel.fire(pixel: .addressBarClickOnAIChat)
Pixel.fire(pixel: .addressBarClickOnAIChat, withAdditionalParameters: [
PixelParameters.browsingMode: tabManager.currentBrowsingMode.pixelParamValue
])
viewCoordinator.omniBar.beginEditing(animated: true, forTextEntryMode: .aiChat)
}

Expand Down Expand Up @@ -4049,7 +4059,7 @@
}

func onFireModeRequested() {
navigateToFireMode()
navigateToFireMode(source: .ntpPromotion)
}

func isCurrentTabFireTab() -> Bool {
Expand Down Expand Up @@ -4171,7 +4181,7 @@
}

func newTabPageDidRequestFireMode(_ controller: NewTabPageViewController) {
navigateToFireMode()
navigateToFireMode(source: .ntpPromotion)
}
}

Expand All @@ -4186,7 +4196,7 @@
}

func tabDidRequestFireMode(tab: TabViewController) {
navigateToFireMode()
navigateToFireMode(source: .menuPromotion)
}

var isAIChatEnabled: Bool {
Expand Down Expand Up @@ -4272,7 +4282,7 @@
func tab(_ tab: TabViewController,
didRequestNewFireTabForUrl url: URL,
inheritingAttribution attribution: AdClickAttributionLogic.State?) {
tabManager.setBrowsingMode(.fire)
tabManager.setBrowsingMode(.fire, source: .longPressLink)
loadUrlInNewTab(url, inheritedAttribution: attribution)
}

Expand Down Expand Up @@ -4714,8 +4724,11 @@
}

func showTabSwitcher(_ button: TabSwitcherButton) {
Pixel.fire(pixel: .tabBarTabSwitcherOpened)
DailyPixel.fireDaily(.tabSwitcherOpenedDaily, withAdditionalParameters: TabSwitcherOpenDailyPixel().parameters(with: tabManager.allTabsModel.tabs))
Pixel.fire(pixel: .tabBarTabSwitcherOpened,
withAdditionalParameters: [PixelParameters.browsingMode: tabManager.currentBrowsingMode.pixelParamValue])
var openedDailyParams = TabSwitcherOpenDailyPixel().parameters(with: tabManager.allTabsModel.tabs)
openedDailyParams[PixelParameters.browsingMode] = tabManager.currentBrowsingMode.pixelParamValue
DailyPixel.fireDaily(.tabSwitcherOpenedDaily, withAdditionalParameters: openedDailyParams)

performActionIfAITab { DailyPixel.fireDailyAndCount(pixel: .aiChatTabSwitcherOpened) }

Expand Down Expand Up @@ -4949,6 +4962,7 @@
extension MainViewController: TabManagerFireModeDelegate {

func tabManagerDidCloseLastFireTab() {
DailyPixel.fireDailyAndCount(pixel: .fireModeLastTabClosedBurn)
Task {
let request = FireRequest(options: [.data, .aiChats],
trigger: .fireModeAutoClear,
Expand Down Expand Up @@ -4983,20 +4997,23 @@

private func firePixels(for request: FireRequest) {
let tabType = tabManager.viewModelForCurrentTab()?.tab.isAITab == true ? "ai" : "web"
let browsingMode = tabManager.currentBrowsingMode.pixelParamValue
let params: [String: String] = [
PixelParameters.source: request.source.rawValue,
PixelParameters.tabType: tabType
PixelParameters.tabType: tabType,
PixelParameters.browsingMode: browsingMode
]

switch request.scope {
case .all:
Pixel.fire(pixel: .forgetAllExecuted, withAdditionalParameters: params)
DailyPixel.fire(pixel: .forgetAllExecutedDaily, withAdditionalParameters: params)
case .tab:
DailyPixel.fireDailyAndCount(pixel: .singleTabBurnExecuted, withAdditionalParameters: params)
case .fireMode, .normalMode:
// TODO: - Add fire mode burn pixel
break
case .fireMode:
DailyPixel.fireDailyAndCount(pixel: .fireModeBurnExecuted, withAdditionalParameters: params)
case .normalMode:
DailyPixel.fireDailyAndCount(pixel: .normalModeBurnExecuted, withAdditionalParameters: params)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Browsing mode may be stale for scoped burns

Medium Severity

In firePixels(for:), tabManager.currentBrowsingMode is read at fire time. For .fireMode scoped burns triggered by tabManagerDidCloseLastFireTab, the browsing mode has likely already switched to .normal (since the last fire tab closed). This means fireModeBurnExecuted fires with browsingMode=normal, which is misleading — the burn is clearly a fire-mode operation. The mode should be derived from the request.scope rather than queried from tabManager.currentBrowsingMode.


Please tell me if this was useful or not with a 👍 or 👎.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 2ff6b0a. Configure here.

}
}

Expand Down
1 change: 0 additions & 1 deletion iOS/DuckDuckGo/NewTabPageMessagesModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,6 @@ final class NewTabPageMessagesModel: ObservableObject {
// MARK: - Fire Mode Promotion Actions

func firePromotionDidAppear() {
// TODO: fire promotion shown pixel
fireModePromotionEligibility?.markNTPPromotionShown()
}

Expand Down
Loading
Loading