Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DeviceInterface] Call closeautofillparent only after openManage calls #718

Merged
merged 1 commit into from
Dec 13, 2024

Conversation

dbajpeyi
Copy link
Collaborator

@dbajpeyi dbajpeyi commented Dec 12, 2024

Reviewer: @GioSensation
Asana: https://app.asana.com/0/1203822806345703/1208958786488368/f

Description

On windows IWebView disposes after closeAutofillParent more efficiently (since https://github.com/duckduckgo/windows-browser/pull/2658/files#diff-e9708afe1dc71c217b0379bc7531fd02309a7ce40e69ce7e60e3bed7edde1160) now causing openManagePasswords call to be late. This causes native password manager UI to not open.

This PR changes the sequence of closeAutofillParent and openPasswordManager calls. Note that this will also affect MacOS, but so far there doesn't seem to be any regression with manual testing.

Steps to test

  1. Go to windows-browser repo,
  2. Update duckduckgo-autofill submodule to use dbajpeyi/fix/manage-password-race-condition
  3. Build the browser,
  4. Go to https://autofill.me/form/registration-email and try to open password manager from the autofill prompt.
  5. The manage prompt should open!

@dbajpeyi dbajpeyi changed the title wip: split manage calls for apple/windows wip: split closeautofillparent calls for apple/windows Dec 12, 2024
@dbajpeyi dbajpeyi force-pushed the dbajpeyi/fix/manage-password-race-condition branch 2 times, most recently from bf95ad5 to d09d037 Compare December 12, 2024 14:57
@dbajpeyi dbajpeyi changed the title wip: split closeautofillparent calls for apple/windows Call closeautofillparent only after the manage UI has opened Dec 12, 2024
@dbajpeyi dbajpeyi changed the title Call closeautofillparent only after the manage UI has opened [DeviceInterface] Call closeautofillparent only after the manage UI has opened Dec 13, 2024
@dbajpeyi dbajpeyi force-pushed the dbajpeyi/fix/manage-password-race-condition branch from d09d037 to feccf1e Compare December 13, 2024 11:08
@dbajpeyi dbajpeyi marked this pull request as ready for review December 13, 2024 11:12
default:
// noop
}
this.removeTooltip();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Doesn't seem to affect MacOS - would like to avoid multiple/split implementation between platforms unless really necessary (doesn't seem like it).

@dbajpeyi dbajpeyi changed the title [DeviceInterface] Call closeautofillparent only after the manage UI has opened [DeviceInterface] Call closeautofillparent only after openManage calls Dec 13, 2024
Copy link
Member

@GioSensation GioSensation left a comment

Choose a reason for hiding this comment

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

Looking good. Haven't checked on a real build, but seems fine. Thanks for reacting quickly!

@dbajpeyi dbajpeyi merged commit e14501e into main Dec 13, 2024
1 check passed
@dbajpeyi dbajpeyi deleted the dbajpeyi/fix/manage-password-race-condition branch December 13, 2024 17:14
CDRussell pushed a commit to duckduckgo/Android that referenced this pull request Dec 18, 2024
Task/Issue URL:
https://app.asana.com/0/1209009001482464/1209009001482464
Autofill Release:
https://github.com/duckduckgo/duckduckgo-autofill/releases/tag/16.1.0


## Description
Updates Autofill to version
[16.1.0](https://github.com/duckduckgo/duckduckgo-autofill/releases/tag/16.1.0).

### Autofill 16.1.0 release notes
## What's Changed
* [DeviceInterface] Call closeautofillparent only after openManage calls
by @dbajpeyi in
duckduckgo/duckduckgo-autofill#718
* [CredentialsImport] Call credentials flow call first, and then close
parent by @dbajpeyi in
duckduckgo/duckduckgo-autofill#723
* [FeatureToggle] Add new autofill config for partial save trigger by
@dbajpeyi in duckduckgo/duckduckgo-autofill#724


**Full Changelog**:
duckduckgo/duckduckgo-autofill@16.0.0...16.1.0

## Steps to test
This release has been tested during autofill development. For smoke test
steps see [this
task](https://app.asana.com/0/1198964220583541/1200583647142330/f).

Co-authored-by: dbajpeyi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants