Skip to content

Commit

Permalink
New autofill save & update password prompt pixels for alignment with …
Browse files Browse the repository at this point in the history 
…iOS (#2801)

Task/Issue URL: https://app.asana.com/0/72649045549333/1207361449520263/f
Tech Design URL:
CC:

Description:
New pixels to bring macOS password management pixel reporting up to date with iOS
  • Loading branch information
@amddg44
amddg44 authored May 23, 2024
1 parent 03bc5dc commit d826ec2
Show file tree
Hide file tree
Showing 3 changed files with 149 additions and 1 deletion.
104 changes: 104 additions & 0 deletions DuckDuckGo/SecureVault/View/SaveCredentialsViewController.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,12 @@ final class SaveCredentialsViewController: NSViewController {
@IBOutlet var fireproofCheck: NSButton!
@IBOutlet weak var fireproofCheckDescription: NSTextFieldCell!

private enum Action {
case displayed
case confirmed
case dismissed
}

weak var delegate: SaveCredentialsDelegate?

private var credentials: SecureVaultModels.WebsiteCredentials?
Expand Down Expand Up @@ -139,6 +145,9 @@ final class SaveCredentialsViewController: NSViewController {
// Only use the non-editable state if a credential was automatically saved and it didn't already exist.
let condition = credentials.account.id != nil && !(credentials.account.username?.isEmpty ?? true) && automaticallySaved
updateViewState(editable: !condition)

let existingCredentials = getExistingCredentialsFrom(credentials)
evaluateCredentialsAndFirePixels(for: .displayed, credentials: existingCredentials)
}

private func updateViewState(editable: Bool) {
Expand Down Expand Up @@ -208,6 +217,7 @@ final class SaveCredentialsViewController: NSViewController {
domain: domainLabel.stringValue)
account.id = credentials?.account.id
let credentials = SecureVaultModels.WebsiteCredentials(account: account, password: passwordData)
let existingCredentials = getExistingCredentialsFrom(credentials)

do {
if passwordManagerCoordinator.isEnabled {
Expand All @@ -231,6 +241,8 @@ final class SaveCredentialsViewController: NSViewController {
PixelKit.fire(DebugEvent(GeneralPixel.secureVaultError(error: error)))
}

evaluateCredentialsAndFirePixels(for: .confirmed, credentials: existingCredentials)

PixelKit.fire(GeneralPixel.autofillItemSaved(kind: .password))

if passwordManagerCoordinator.isEnabled {
Expand All @@ -250,6 +262,9 @@ final class SaveCredentialsViewController: NSViewController {

@IBAction func onDontUpdateClicked(_ sender: Any) {
delegate?.shouldCloseSaveCredentialsViewController(self)

let existingCredentials = getExistingCredentialsFrom(credentials)
evaluateCredentialsAndFirePixels(for: .dismissed, credentials: existingCredentials)
}

@IBAction func onNotNowSegmentedControlClicked(_ sender: Any) {
Expand Down Expand Up @@ -280,6 +295,9 @@ final class SaveCredentialsViewController: NSViewController {
delegate?.shouldCloseSaveCredentialsViewController(self)
}

let existingCredentials = getExistingCredentialsFrom(credentials)
evaluateCredentialsAndFirePixels(for: .dismissed, credentials: existingCredentials)

guard DataClearingPreferences.shared.isLoginDetectionEnabled else {
notifyDelegate()
return
Expand Down Expand Up @@ -365,4 +383,90 @@ final class SaveCredentialsViewController: NSViewController {
}
}

private func getExistingCredentialsFrom(_ credentials: SecureVaultModels.WebsiteCredentials?) -> SecureVaultModels.WebsiteCredentials? {
guard let credentials = credentials, let id = credentials.account.id else {
return nil
}

var existingCredentials: SecureVaultModels.WebsiteCredentials?

if passwordManagerCoordinator.isEnabled {
guard !passwordManagerCoordinator.isLocked else {
os_log("Failed to access credentials: Password manager is locked")
return existingCredentials
}

passwordManagerCoordinator.websiteCredentialsFor(accountId: id) { credentials, _ in
existingCredentials = credentials
}
} else {
if let idInt = Int64(id) {
existingCredentials = try? AutofillSecureVaultFactory.makeVault(reporter: SecureVaultReporter.shared).websiteCredentialsFor(accountId: idInt)
}
}

return existingCredentials
}

private func isUsernameUpdated(credentials: SecureVaultModels.WebsiteCredentials) -> Bool {
if credentials.account.username != self.usernameField.stringValue.trimmingWhitespace() {
return true
}
return false
}

private func isPasswordUpdated(credentials: SecureVaultModels.WebsiteCredentials) -> Bool {
if credentials.password != self.passwordData {
return true
}
return false
}

private func evaluateCredentialsAndFirePixels(for action: Action, credentials: SecureVaultModels.WebsiteCredentials?) {
switch action {
case .displayed:
if let credentials = credentials {
if isPasswordUpdated(credentials: credentials) {
PixelKit.fire(GeneralPixel.autofillLoginsUpdatePasswordInlineDisplayed)
} else {
PixelKit.fire(GeneralPixel.autofillLoginsUpdateUsernameInlineDisplayed)
}
} else {
if usernameField.stringValue.trimmingWhitespace().isEmpty {
PixelKit.fire(GeneralPixel.autofillLoginsSavePasswordInlineDisplayed)
} else {
PixelKit.fire(GeneralPixel.autofillLoginsSaveLoginInlineDisplayed)
}
}
case .confirmed, .dismissed:
if let credentials = credentials {
if isUsernameUpdated(credentials: credentials) {
firePixel(for: action,
confirmedPixel: GeneralPixel.autofillLoginsUpdateUsernameInlineConfirmed,
dismissedPixel: GeneralPixel.autofillLoginsUpdateUsernameInlineDismissed)
}
if isPasswordUpdated(credentials: credentials) {
firePixel(for: action,
confirmedPixel: GeneralPixel.autofillLoginsUpdatePasswordInlineConfirmed,
dismissedPixel: GeneralPixel.autofillLoginsUpdatePasswordInlineDismissed)
}
} else {
if usernameField.stringValue.trimmingWhitespace().isEmpty {
firePixel(for: action,
confirmedPixel: GeneralPixel.autofillLoginsSavePasswordInlineConfirmed,
dismissedPixel: GeneralPixel.autofillLoginsSavePasswordInlineDismissed)
} else {
firePixel(for: action,
confirmedPixel: GeneralPixel.autofillLoginsSaveLoginInlineConfirmed,
dismissedPixel: GeneralPixel.autofillLoginsSaveLoginInlineDismissed)
}
}
}
}

private func firePixel(for action: Action, confirmedPixel: PixelKitEventV2, dismissedPixel: PixelKitEventV2) {
let pixel = action == .confirmed ? confirmedPixel : dismissedPixel
PixelKit.fire(pixel)
}

}
44 changes: 44 additions & 0 deletions DuckDuckGo/Statistics/GeneralPixel.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,27 @@ enum GeneralPixel: PixelKitEventV2 {
case formAutofilled(kind: FormAutofillKind)
case autofillItemSaved(kind: FormAutofillKind)

case autofillLoginsSaveLoginInlineDisplayed
case autofillLoginsSaveLoginInlineConfirmed
case autofillLoginsSaveLoginInlineDismissed

case autofillLoginsSavePasswordInlineDisplayed
case autofillLoginsSavePasswordInlineConfirmed
case autofillLoginsSavePasswordInlineDismissed

case autofillLoginsSaveLoginModalExcludeSiteConfirmed
case autofillLoginsSettingsResetExcludedDisplayed
case autofillLoginsSettingsResetExcludedConfirmed
case autofillLoginsSettingsResetExcludedDismissed

case autofillLoginsUpdatePasswordInlineDisplayed
case autofillLoginsUpdatePasswordInlineConfirmed
case autofillLoginsUpdatePasswordInlineDismissed

case autofillLoginsUpdateUsernameInlineDisplayed
case autofillLoginsUpdateUsernameInlineConfirmed
case autofillLoginsUpdateUsernameInlineDismissed

case bitwardenPasswordAutofilled
case bitwardenPasswordSaved

Expand Down Expand Up @@ -361,6 +377,20 @@ enum GeneralPixel: PixelKitEventV2 {
case .autofillItemSaved(kind: let kind):
return "m_mac_save_\(kind)"

case .autofillLoginsSaveLoginInlineDisplayed:
return "m_mac_autofill_logins_save_login_inline_displayed"
case .autofillLoginsSaveLoginInlineConfirmed:
return "m_mac_autofill_logins_save_login_inline_confirmed"
case .autofillLoginsSaveLoginInlineDismissed:
return "m_mac_autofill_logins_save_login_inline_dismissed"

case .autofillLoginsSavePasswordInlineDisplayed:
return "m_mac_autofill_logins_save_password_inline_displayed"
case .autofillLoginsSavePasswordInlineConfirmed:
return "m_mac_autofill_logins_save_password_inline_confirmed"
case .autofillLoginsSavePasswordInlineDismissed:
return "m_mac_autofill_logins_save_password_inline_dismissed"

case .autofillLoginsSaveLoginModalExcludeSiteConfirmed:
return "m_mac_autofill_logins_save_login_exclude_site_confirmed"
case .autofillLoginsSettingsResetExcludedDisplayed:
Expand All @@ -370,6 +400,20 @@ enum GeneralPixel: PixelKitEventV2 {
case .autofillLoginsSettingsResetExcludedDismissed:
return "m_mac_autofill_settings_reset_excluded_dismissed"

case .autofillLoginsUpdatePasswordInlineDisplayed:
return "m_mac_autofill_logins_update_password_inline_displayed"
case .autofillLoginsUpdatePasswordInlineConfirmed:
return "m_mac_autofill_logins_update_password_inline_confirmed"
case .autofillLoginsUpdatePasswordInlineDismissed:
return "m_mac_autofill_logins_update_password_inline_dismissed"

case .autofillLoginsUpdateUsernameInlineDisplayed:
return "m_mac_autofill_logins_update_username_inline_displayed"
case .autofillLoginsUpdateUsernameInlineConfirmed:
return "m_mac_autofill_logins_update_username_inline_confirmed"
case .autofillLoginsUpdateUsernameInlineDismissed:
return "m_mac_autofill_logins_update_username_inline_dismissed"

case .bitwardenPasswordAutofilled:
return "m_mac_bitwarden_autofill_password"

Expand Down
2 changes: 1 addition & 1 deletion UnitTests/Tab/ViewModel/TabViewModelTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ final class TabViewModelTests: XCTestCase {
let filteredCases = DefaultZoomValue.allCases.filter { $0 != AccessibilityPreferences.shared.defaultPageZoom }
let randomZoomLevel = filteredCases.randomElement()!
AccessibilityPreferences.shared.updateZoomPerWebsite(zoomLevel: randomZoomLevel, url: hostURL)
var tab = Tab(url: url)
let tab = Tab(url: url)
var tabVM = TabViewModel(tab: tab)

// WHEN
Expand Down

0 comments on commit d826ec2

Please sign in to comment.