1629143 - Password manager not enabled on CostCoPharmacy.ca

Status: VERIFIED FIXED

(Toolkit :: Password Manager: Site Compatibility, defect, P2)

Description

[Jared Wein [:jaws] (please needinfo? me)]

Under "New Patient" on the page, right-click on either of the password inputs and look for a "Use Generated Password..." option.

Debugging information says, "(form ignored -- too many password fields. [ got 4 ])"

Comment 1

[Matthew N. [:MattN]]

Autocomplete and all login context menu items are affected.

The initial password manager code had a limit of 3 password fields and later when it was converted to JS the limited was moved to 4 (maybe an off-by-one error?). There is no rationale explained (other than 1 password field being an assumption it's for login and 2 or 3 treated as registration or change) but I imagine back in those days that people used <form> elements better.

bug 1538026 comment 2 was another example where we hit the same issue but in that case I think it was to intentionally go over our threshold.

I would be fine with trying to bump the limit to 4 or 5 in general.

Comment 2

[:severin Rudie]

Attachment: Bug 1629143 - enable password manager on sites with <= 5 password fields;r?MattN

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/853b0e791775
enable password manager on sites with <= 5 password fields;r=MattN

Comment 4

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/853b0e791775

Comment 5

[Matthew N. [:MattN]]

If there's no fallout it may be worth uplifting since it's [wfh]

Comment 6

[Matthew N. [:MattN]]

Comment on attachment 9141099 [details]
Bug 1629143 - enable password manager on sites with <= 5 password fields;r?MattN

Beta/Release Uplift Approval Request

• User impact if declined: Pages with more than 3 password fields on one page won't have passwords saved/filled/autocomplete/generated. This affects multiple sites but https://www.costcopharmacy.ca/Login.aspx is the focus since it is probably more relevant during Covid-19.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The actual code change is bumping a threshold so by itself is low risk in terms of breaking general functionality in Firefox. It's possible that this means we will now prompt to save/fill/autocomplete on a form where a user wouldn't want us to but they would be able to choose Never Save for the site or ignore the prompt in that case so it would be an annoyance more than a major issue.
• String changes made/needed: None

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9141099 [details]
Bug 1629143 - enable password manager on sites with <= 5 password fields;r?MattN

Fixes a possible password manager papercut bug. Fits well with the general "password manager improvements" theme of 76. Thanks for including automated tests. Approved for 76.0b8.

Comment 8

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/releases/mozilla-beta/rev/423c734549bc

Comment 9

[Timea Cernea [:tbabos][inactive]]

Verified-Fixed on Windows 10, Ubuntu 16.04 and MacOS 10.13 on latest Nightly 77.0a1 (2020-04-22) and Beta 76.0b8.

1. The following context menu items are now displayed and functional in the context menu for all the fields on the page:
   a. Fill Login
   b. Fill Password
   c. Use Generated Password...
2. The autocomplete dropdown is displayed for username/password field

Please note that the username will only be autofilled/autocompleted and captured on the login form from the top of the page. -> will submit a bug for this.
Filling in a username on the Register form will not be captured by the dismissed or save doorhanger. -> will submit this as a separate bug
Password Generation is also not offered in the autocomplete dropdown upon clicking on the New/Confirm password fields -> will also submit it as a false negative case issue.

As for what this patch is supposed to fix (context menu items and autocomplete to be displayed for the fields) considering it as fixed, but will follow up with bugs for what I mentioned above.

Comment 10

[Matthew N. [:MattN]]

(In reply to Timea Cernea [:tbabos] from comment #9)

Please note that the username will only be autofilled/autocompleted and captured on the login form from the top of the page. -> will submit a bug for this.
Filling in a username on the Register form will not be captured by the dismissed or save doorhanger. -> will submit this as a separate bug
Password Generation is also not offered in the autocomplete dropdown upon clicking on the New/Confirm password fields -> will also submit it as a false negative case issue.

I think all of this can be covered by bug 1304002.

Comment 11

[Timea Cernea [:tbabos][inactive]]

Added Costcopharmacy.ca to that bug so we can check it out once it gets fixed. Thank you!