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

Support Autofill Domains with Port Number Suffixes #2602

Merged
merged 6 commits into from
Apr 15, 2024

Conversation

aataraxiaa
Copy link
Contributor

@aataraxiaa aataraxiaa commented Apr 11, 2024

Task/Issue URL: https://app.asana.com/0/1199230911884351/1206023722645078/f

Description:
This PR includes the following changes:

  • Temporarily points BSK reference at commit for related BSK work
  • Adds a URLExtension method which fixes an new issue where the Autofill dialog didn’t open with the right account highlighted, when the domain had a port suffix
  • Adds tests for above

Steps to test this PR:

  1. Test Autofilling on domain with port
  • Navigate to domain with port (private domain shared on MM, owner requested not to share here)
  • Attempt a login, and save it
  • Reload page, ensure Autofill suggests saved login
  1. Test Autofilling on domain with port and multiple domains
  • Follow test 1 steps above, but adding multiple logins
  • Ensure we can select any of them, and that most recent used works as expected
  1. Test ‘Manage Passwords’ Autofill action opens Autofill with relevant item highlighted
  • On any login page with a saved login, select ‘Manage passwords’ from the Autofill prompt menu
  • Observe the Autofill dialog opens with the correct entry selected/highlighted
  1. Smoke Test Autofill
  • Using fill.dev or other site with login, smoke test general autofill usage

Internal references:

Pull Request Review Checklist
Software Engineering Expectations
Technical Design Template
Pull Request Documentation

… password manager from Autofill 'Manage passwords' action. Add tests. Point at BSK commit.
@aataraxiaa aataraxiaa marked this pull request as ready for review April 11, 2024 13:53
@aataraxiaa aataraxiaa requested a review from amddg44 April 11, 2024 13:53
Copy link
Contributor

@amddg44 amddg44 left a comment

Choose a reason for hiding this comment

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

LGTM! Works perfectly including smoke tests of all pre-existing behaviours 🚀

@@ -1,7 +1,7 @@
<?xml version="1.0" encoding="UTF-8"?>
<Scheme
LastUpgradeVersion = "1530"
version = "1.8">
version = "1.7">
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if this change should be part of this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @amddg44, I’ll remove

@aataraxiaa aataraxiaa merged commit ef05acd into main Apr 15, 2024
17 checks passed
@aataraxiaa aataraxiaa deleted the pete/autofill-ports-support branch April 15, 2024 12:55
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