Skip to content

Commit

Permalink
Increase ratio of complete form saves (#3649)
Browse files Browse the repository at this point in the history
Task/Issue URL: https://app.asana.com/0/0/1206048666874235/f

**Description**:

Increase ratio of complete credentials saves (i.e. that include the
username) by checking for recently filled emails/usernames when a form
save event is incomplete.

Approach is to have a new form save events for username/email-only forms
that stays in memory for 3 minutes. If a password-only form save is
detected for the same domain, we can complete the form with the previous
partial data. This would also cover mobile where we don't have
identities autofill – and in general in all cases where the user inputs
data manually.

**Optional E2E tests**:
- [ ] Run PIR E2E tests
Check this to run the Personal Information Removal end to end tests. If
updating CCF, or any PIR related code, tick this.

**Steps to test this PR**:
https://app.asana.com/0/1202926619870900/1208866651723703/f

<!--
Tagging instructions
If this PR isn't ready to be merged for whatever reason it should be
marked with the `DO NOT MERGE` label (particularly if it's a draft)
If it's pending Product Review/PFR, please add the `Pending Product
Review` label.

If at any point it isn't actively being worked on/ready for
review/otherwise moving forward (besides the above PR/PFR exception)
strongly consider closing it (or not opening it in the first place). If
you decide not to close it, make sure it's labelled to make it clear the
PRs state and comment with more information.
-->

**Definition of Done**:

* [ ] Does this PR satisfy our [Definition of
Done](https://app.asana.com/0/1202500774821704/1207634633537039/f)?

---
###### Internal references:
[Pull Request Review
Checklist](https://app.asana.com/0/1202500774821704/1203764234894239/f)
[Software Engineering
Expectations](https://app.asana.com/0/59792373528535/199064865822552)
[Technical Design
Template](https://app.asana.com/0/59792373528535/184709971311943)
[Pull Request
Documentation](https://app.asana.com/0/1202500774821704/1204012835277482/f)
  • Loading branch information
graeme authored Dec 9, 2024
1 parent c718fdb commit 65bfa97
Show file tree
Hide file tree
Showing 13 changed files with 38 additions and 15 deletions.
2 changes: 1 addition & 1 deletion DuckDuckGo.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -15225,7 +15225,7 @@
repositoryURL = "https://github.com/duckduckgo/BrowserServicesKit";
requirement = {
kind = exactVersion;
version = 218.1.0;
version = 219.0.0;
};
};
9FF521422BAA8FF300B9819B /* XCRemoteSwiftPackageReference "lottie-spm" */ = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@
"kind" : "remoteSourceControl",
"location" : "https://github.com/duckduckgo/BrowserServicesKit",
"state" : {
"revision" : "a82c14b991f8cbef46d4b7d8e613762f1592fcd2",
"version" : "218.1.0"
"revision" : "20df9e22d5a69acfc25204fb0228a9d6f79b721f",
"version" : "219.0.0"
}
},
{
Expand Down
3 changes: 3 additions & 0 deletions DuckDuckGo/Autofill/ContentOverlayViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,12 @@ public final class ContentOverlayViewController: NSViewController, EmailManagerR
return emailManager
}()

lazy var featureFlagger = NSApp.delegateTyped.featureFlagger

lazy var vaultManager: SecureVaultManager = {
let manager = SecureVaultManager(passwordManager: PasswordManagerCoordinator.shared,
includePartialAccountMatches: true,
shouldAllowPartialFormSaves: featureFlagger.isFeatureOn(.autofillPartialFormSaves),
tld: ContentBlocking.shared.tld)
manager.delegate = self
return manager
Expand Down
19 changes: 13 additions & 6 deletions DuckDuckGo/SecureVault/View/SaveCredentialsViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -487,13 +487,16 @@ final class SaveCredentialsViewController: NSViewController {
}

private func evaluateCredentialsAndFirePixels(for action: Action, credentials: SecureVaultModels.WebsiteCredentials?) {
let backfilledKey = GeneralPixel.AutofillParameterKeys.backfilled
switch action {
case .displayed:
if let credentials = credentials {
if isPasswordUpdated(credentials: credentials) {
PixelKit.fire(GeneralPixel.autofillLoginsUpdatePasswordInlineDisplayed)
let backfilled = credentials.password.flatMap { String(data: $0, encoding: .utf8) }.isNilOrEmpty
PixelKit.fire(GeneralPixel.autofillLoginsUpdatePasswordInlineDisplayed, withAdditionalParameters: [backfilledKey: String(describing: backfilled)])
} else {
PixelKit.fire(GeneralPixel.autofillLoginsUpdateUsernameInlineDisplayed)
let backfilled = credentials.account.username.isNilOrEmpty
PixelKit.fire(GeneralPixel.autofillLoginsUpdateUsernameInlineDisplayed, withAdditionalParameters: [backfilledKey: String(describing: backfilled)])
}
} else {
if usernameField.stringValue.trimmingWhitespace().isEmpty {
Expand All @@ -505,14 +508,18 @@ final class SaveCredentialsViewController: NSViewController {
case .confirmed, .dismissed:
if let credentials = credentials {
if isUsernameUpdated(credentials: credentials) {
let backfilled = credentials.account.username.isNilOrEmpty
firePixel(for: action,
confirmedPixel: GeneralPixel.autofillLoginsUpdateUsernameInlineConfirmed,
dismissedPixel: GeneralPixel.autofillLoginsUpdateUsernameInlineDismissed)
dismissedPixel: GeneralPixel.autofillLoginsUpdateUsernameInlineDismissed,
withAdditionalParameters: [backfilledKey: String(describing: backfilled)])
}
if isPasswordUpdated(credentials: credentials) {
let backfilled = credentials.password.flatMap { String(data: $0, encoding: .utf8) }.isNilOrEmpty
firePixel(for: action,
confirmedPixel: GeneralPixel.autofillLoginsUpdatePasswordInlineConfirmed,
dismissedPixel: GeneralPixel.autofillLoginsUpdatePasswordInlineDismissed)
dismissedPixel: GeneralPixel.autofillLoginsUpdatePasswordInlineDismissed,
withAdditionalParameters: [backfilledKey: String(describing: backfilled)])
}
} else {
if usernameField.stringValue.trimmingWhitespace().isEmpty {
Expand All @@ -528,9 +535,9 @@ final class SaveCredentialsViewController: NSViewController {
}
}

private func firePixel(for action: Action, confirmedPixel: PixelKitEventV2, dismissedPixel: PixelKitEventV2) {
private func firePixel(for action: Action, confirmedPixel: PixelKitEventV2, dismissedPixel: PixelKitEventV2, withAdditionalParameters parameters: [String: String]? = nil) {
let pixel = action == .confirmed ? confirmedPixel : dismissedPixel
PixelKit.fire(pixel)
PixelKit.fire(pixel, withAdditionalParameters: parameters)
}

}
4 changes: 4 additions & 0 deletions DuckDuckGo/Statistics/GeneralPixel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1442,4 +1442,8 @@ enum GeneralPixel: PixelKitEventV2 {
}

}

enum AutofillParameterKeys {
static var backfilled = "backfilled"
}
}
3 changes: 3 additions & 0 deletions DuckDuckGo/Tab/TabExtensions/AutofillTabExtension.swift
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,12 @@ final class AutofillTabExtension: TabExtension {
return emailManager
}

static var featureFlagger = NSApp.delegateTyped.featureFlagger

static var vaultManagerProvider: (SecureVaultManagerDelegate) -> AutofillSecureVaultDelegate = { delegate in
let manager = SecureVaultManager(passwordManager: PasswordManagerCoordinator.shared,
includePartialAccountMatches: true,
shouldAllowPartialFormSaves: featureFlagger.isFeatureOn(.autofillPartialFormSaves),
tld: ContentBlocking.shared.tld)
manager.delegate = delegate
return manager
Expand Down
2 changes: 1 addition & 1 deletion LocalPackages/DataBrokerProtection/Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ let package = Package(
targets: ["DataBrokerProtection"])
],
dependencies: [
.package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "218.1.0"),
.package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "219.0.0"),
.package(path: "../SwiftUIExtensions"),
.package(path: "../AppKitExtensions"),
.package(path: "../XPCHelper"),
Expand Down
2 changes: 1 addition & 1 deletion LocalPackages/FeatureFlags/Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ let package = Package(
targets: ["FeatureFlags"]),
],
dependencies: [
.package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "218.1.0"),
.package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "219.0.0"),
],
targets: [
// Targets are the basic building blocks of a package, defining a module or a test suite.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ public enum FeatureFlag: String, CaseIterable {

case isPrivacyProLaunchedROW
case isPrivacyProLaunchedROWOverride

case autofillPartialFormSaves
}

extension FeatureFlag: FeatureFlagDescribing {
Expand All @@ -59,6 +61,8 @@ extension FeatureFlag: FeatureFlagDescribing {
return true
case .maliciousSiteProtectionErrorPage:
return true
case .autofillPartialFormSaves:
return true
case .debugMenu,
.sslCertificatesBypass,
.appendAtbToSerpQueries,
Expand Down Expand Up @@ -101,6 +105,8 @@ extension FeatureFlag: FeatureFlagDescribing {
return .remoteReleasable(.subfeature(PrivacyProSubfeature.isLaunchedROW))
case .isPrivacyProLaunchedROWOverride:
return .remoteReleasable(.subfeature(PrivacyProSubfeature.isLaunchedROWOverride))
case .autofillPartialFormSaves:
return .remoteReleasable(.subfeature(AutofillSubfeature.partialFormSaves))
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion LocalPackages/NetworkProtectionMac/Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ let package = Package(
.library(name: "VPNAppLauncher", targets: ["VPNAppLauncher"]),
],
dependencies: [
.package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "218.1.0"),
.package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "219.0.0"),
.package(url: "https://github.com/airbnb/lottie-spm", exact: "4.4.3"),
.package(path: "../AppLauncher"),
.package(path: "../UDSHelper"),
Expand Down
2 changes: 1 addition & 1 deletion LocalPackages/NewTabPage/Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ let package = Package(
targets: ["NewTabPage"]),
],
dependencies: [
.package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "218.1.0"),
.package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "219.0.0"),
.package(path: "../WebKitExtensions"),
.package(path: "../Utilities"),
],
Expand Down
2 changes: 1 addition & 1 deletion LocalPackages/SubscriptionUI/Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ let package = Package(
targets: ["SubscriptionUI"]),
],
dependencies: [
.package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "218.1.0"),
.package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "219.0.0"),
.package(path: "../SwiftUIExtensions"),
.package(path: "../FeatureFlags")
],
Expand Down
2 changes: 1 addition & 1 deletion LocalPackages/WebKitExtensions/Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ let package = Package(
),
],
dependencies: [
.package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "218.1.0"),
.package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "219.0.0"),
.package(path: "../AppKitExtensions")
],
targets: [
Expand Down

0 comments on commit 65bfa97

Please sign in to comment.