Skip to content

Commit

Permalink
[Form] Trigger partialSave on username/email only form submit (#702)
Browse files Browse the repository at this point in the history
* feat: add API call

* feat: allow partial save in form submission

* fix: don't need additional API

* feat: update heuristics for save and update tests

* feat: prompt when autofilled

* chore: rebuild assets

* fix: narrow down identities for partial save

* chore: address PR comments

* fix: make credentials storing more liberal

* feat: preserve autosubmit

* refactor: simply value storage

* style: minor PR comments
  • Loading branch information
dbajpeyi authored Dec 6, 2024
1 parent ca1722c commit 88982a3
Show file tree
Hide file tree
Showing 18 changed files with 312 additions and 182 deletions.
61 changes: 33 additions & 28 deletions dist/autofill-debug.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

59 changes: 32 additions & 27 deletions dist/autofill.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions integration-test/helpers/pages/loginPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,12 @@ export function loginPage(page, opts = {}) {
expect(mockCalls.length).toBe(0);
}

async shouldPromptToSave() {
let mockCalls = [];
mockCalls = await mockedCalls(page, { names: ['storeFormData'] });
expect(mockCalls.length).toBeGreaterThan(0);
}

/**
* This is used mostly to avoid false negatives when we check for something _not_ happening.
* Basically, you check that a specific call hasn't happened but the rest of the script ran just fine.
Expand Down
4 changes: 2 additions & 2 deletions integration-test/tests/save-prompts.android.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,11 +81,11 @@ test.describe('Android Save prompts', () => {
await login.submitPasswordOnlyForm(credentials);
await login.assertWasPromptedToSave(credentials);
});
test('with username only (should NOT prompt)', async ({ page }) => {
test('with username only (should prompt)', async ({ page }) => {
const { login } = await setup(page);
const credentials = { username: '123456' };
await login.submitUsernameOnlyForm(credentials.username);
await login.promptWasNotShown();
await login.shouldPromptToSave();
});
});
});
Expand Down
4 changes: 2 additions & 2 deletions integration-test/tests/save-prompts.ios.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,12 +105,12 @@ test.describe('iOS Save prompts', () => {
await login.assertWasPromptedToSave(credentials);
});

test('username only (should NOT prompt)', async ({ page }) => {
test('username only (should prompt)', async ({ page }) => {
const login = await setup(page);

const credentials = { username: '123456' };
await login.submitUsernameOnlyForm(credentials.username);
await login.shouldNotPromptToSave();
await login.shouldPromptToSave();
});
});

Expand Down
4 changes: 2 additions & 2 deletions integration-test/tests/save-prompts.macos.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ test.describe('macos', () => {
await login.submitPasswordOnlyForm(credentials);
await login.assertWasPromptedToSave(credentials);
});
test('username only (should NOT prompt)', async ({ page }) => {
test('username only (should prompt)', async ({ page }) => {
// enable in-terminal exceptions
await forwardConsoleMessages(page);

Expand All @@ -76,7 +76,7 @@ test.describe('macos', () => {
const login = loginPage(page);
await login.navigate();
await login.submitUsernameOnlyForm(credentials.username);
await login.shouldNotPromptToSave();
await login.shouldPromptToSave();
});
});
});
Expand Down
8 changes: 6 additions & 2 deletions src/DeviceInterface/InterfacePrototype.js
Original file line number Diff line number Diff line change
Expand Up @@ -799,14 +799,18 @@ class InterfacePrototype {
postSubmit(values, form) {
if (!form.form) return;
if (!form.hasValues(values)) return;
const checks = [form.shouldPromptToStoreData && !form.submitHandlerExecuted, this.passwordGenerator.generated];

const isUsernameOnly = Object.keys(values?.credentials || {}).length === 1 && values?.credentials?.username;
const checks = [form.shouldPromptToStoreData && !form.submitHandlerExecuted, this.passwordGenerator.generated, isUsernameOnly];
if (checks.some(Boolean)) {
const formData = appendGeneratedKey(values, {
password: this.passwordGenerator.password,
username: this.emailProtection.lastGenerated,
});
this.storeFormData(formData, 'formSubmission');

// If credentials has only username field, and no password field, then trigger is a partialSave
const trigger = isUsernameOnly ? 'partialSave' : 'formSubmission';
this.storeFormData(formData, trigger);
}
}

Expand Down
8 changes: 4 additions & 4 deletions src/Form/Form.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,8 @@ describe('Test the form class reading values correctly', () => {
<input type="password" value="" autocomplete="new-password" />
<button type="submit">Sign up</button>
</form>`,
expHasValues: false,
expValues: { credentials: undefined },
expHasValues: true,
expValues: { credentials: { username: 'testUsername' } },
},
{
testCase: 'form where the password is <=3 characters long',
Expand All @@ -95,8 +95,8 @@ describe('Test the form class reading values correctly', () => {
<input type="password" value="abc" autocomplete="new-password" />
<button type="submit">Sign up</button>
</form>`,
expHasValues: false,
expValues: { credentials: undefined },
expHasValues: true,
expValues: { credentials: { username: 'testUsername' } },
},
{
testCase: 'form with hidden email field',
Expand Down
Loading

0 comments on commit 88982a3

Please sign in to comment.