Only show View Saved Logins dropdown via arrow-down on empty password fields with no login suggestions
Categories
(Toolkit :: Password Manager, defect, P1)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr60 | --- | unaffected |
firefox66 | --- | unaffected |
firefox67 | --- | verified |
firefox68 | --- | verified |
People
(Reporter: rfeeley, Assigned: MattN)
References
(Regression)
Details
(Keywords: parity-chrome, parity-safari, regression)
Attachments
(2 files)
(deleted),
text/x-phabricator-request
|
pascalc
:
approval-mozilla-beta+
|
Details |
(deleted),
image/png
|
Details |
I noticed that on blank password fields where no password is saved, the View Saved Logins appears on focus.
I think it should behave more like a typical input text and only reveal when the user has arrowed down or double clicked.
Assignee | ||
Comment 1•6 years ago
|
||
From talking with Ryan, I think his main concern is when we show only the footer in a password field upon focus (which is only when it's empty).
Summarizing the problem environment:
- Password field is empty
- There is no saved password suggestion for this form/site
- The user clicks in the password field
Currently we auto-open the autocomplete popup with only the "View Saved Logins" footer showing. Other browsers don't automatically open in this case though Safari instead has an in-field key icon to click on.
Sandy, do you agree with Ryan that we shouldn't auto-show in this case? It ends up overlapping site content and I know people we even annoyed with the auto-opening for the insecure login field warning. It would be great to get an answer for this ASAP before we uplift bug 1534442.
(Marking 67 affected as it will be affected once we uplift bug 1534442)
If there are other cases where we want to show the popup less we can handle those in separate bugs.
Comment 2•6 years ago
|
||
All good points. In my mind the current flow is intentional: when there are no passwords available/saved there is a path to go find the desired login for that form. That said, it's not my goal to irritate our users. Let's proceed as Ryan has requested. We'll then be able to track if people are able to use the footer entry (or not) to access saved logins and can introduce ways to make this more discoverable (eg a key icon).
It would be really great to make some option in settings to disable that footer completely. It is very annoying.
Assignee | ||
Comment 4•6 years ago
|
||
Once we fix this bug will you still find it annoying? After this bug it would only show up on a password field when the user explicitly opens it with the down arrow or double-click (varies by OS).
If you find it annoying on username fields then that would be a separate issue.
Assignee | ||
Updated•6 years ago
|
Assignee | ||
Updated•6 years ago
|
Once we fix this bug will you still find it annoying?
It will be much less annoying after that.
If you find it annoying on username fields then that would be a separate issue.
I think it should be approached the same way.
And one more thing:
I don't want this popup to appear if I've disabled saving passwords for this website (from a little popup in address-bar), and I don't have any logins saved.
Assignee | ||
Comment 6•6 years ago
|
||
(In reply to Revertron from comment #5)
Once we fix this bug will you still find it annoying?
It will be much less annoying after that.
OK, that's good.
If you find it annoying on username fields then that would be a separate issue.
I think it should be approached the same way.
OK, but there are some different factors to consider and the implementation of a change is quite different so please file a separate bug for this giving specific steps to reproduce the annoyance.
And one more thing:
I don't want this popup to appear if I've disabled saving passwords for this website (from a little popup in address-bar), and I don't have any logins saved.
OK, I would need to do some more research into how we handle autocomplete suggestions… there are also mixed feelings by users about how our password manager checkbox should work… (e.g. should it be about saving or filling?). Also, the interaction with logins saved on other subdomains is worth considering. Please file another bug for this too.
Thanks
Assignee | ||
Comment 7•6 years ago
|
||
I'm looking into our options for the password field case.
Assignee | ||
Comment 8•6 years ago
|
||
Normally autocomplete results are cached based upon the search string but to get the desired behaviour we want two different sets of results for the same search string depending on how the autocomplete search was started:
a) Via automatically focusing a password field.
b) Every other method of starting an autocomplete search.
In order to not have cached results used, the result code for case (a) [an empty result] will be RESULT_FAILURE
and I've updated the autocomplete code to not re-use an error result.
In the coming months we may be rewriting our content autocomplete code but that would be too risky to uplift to 67 so for now I'm tracking when satchel automatically opens the popup upon focus and then using that state in the autocomplete result creation code to know whether to include the footer.
Created separate bugs for other cases:
https://bugzilla.mozilla.org/show_bug.cgi?id=1539754
https://bugzilla.mozilla.org/show_bug.cgi?id=1539756
Comment 10•6 years ago
|
||
Comment 11•6 years ago
|
||
bugherder |
Comment hidden (off-topic) |
Comment hidden (off-topic) |
Comment hidden (off-topic) |
Comment 15•6 years ago
|
||
Verified that we don't show the footer on the password field on focus when there's nothing to show on Ubuntu 16.04 / Windows 10 / Mac 10.14 using 68.0a1 2019-04-01.
Assignee | ||
Comment 16•6 years ago
|
||
Comment on attachment 9054094 [details]
Bug 1538952 - Don't automatically open the password autocomplete popup when we only have the footer to show. r=jaws!,mak!
Beta/Release Uplift Approval Request
- Feature/Bug causing the regression: Bug 1189618
- User impact if declined: The autocomplete popup containing only the "View Saved Logins" footer will be shown when focusing a password field and annoying some users.
- Is this code covered by automated tests?: Yes
- Has the fix been verified in Nightly?: Yes
- Needs manual test from QE?: Yes
- If yes, steps to reproduce: Adrian already knows how to verify
- List of other uplifts needed: None
- Risk to taking this patch: Medium
- Why is the change risky/not risky? (and alternatives if risky): C++ change so some chance of a crash though I don't see it as a current Nightly top crasher and we have automated tests that would likely catch crashes.
- String changes made/needed: None
Assignee | ||
Updated•6 years ago
|
Assignee | ||
Updated•6 years ago
|
Updated•6 years ago
|
Updated•6 years ago
|
Comment 17•6 years ago
|
||
Comment on attachment 9054094 [details]
Bug 1538952 - Don't automatically open the password autocomplete popup when we only have the footer to show. r=jaws!,mak!
Fix with tests for a regression introduced by Bug 1189618 which is a 67 feature, verified by QA on Nightly, uplift accepted for 67 beta 9, thanks!
Updated•6 years ago
|
Comment 18•6 years ago
|
||
bugherder uplift |
Updated•6 years ago
|
Assignee | ||
Updated•6 years ago
|
Assignee | ||
Comment 19•6 years ago
|
||
Looks like I missed allowing this to work with double-click. Adrian will file an enhancement bug for that.
Updated•6 years ago
|
Comment 20•6 years ago
|
||
With the double click work tracked in bug 1546719, marking this as also verified on 67.0b13 on Windows 10, Ubuntu 16.04, Mac 10.14.
Updated•6 years ago
|
Updated•3 years ago
|
Description
•