Add a "View Saved Logins" footer to the password manager autocomplete popup

VERIFIED FIXED in Firefox 67

Status

()

P2
enhancement
VERIFIED FIXED
4 years ago
9 days ago

People

(Reporter: MattN, Assigned: prathiksha, Mentored)

Tracking

(Blocks: 2 bugs)

unspecified
mozilla67
Points:
---
Dependency tree / graph
Bug Flags:
firefox-backlog +
qe-verify +

Firefox Tracking Flags

(firefox67 verified)

Details

(Whiteboard: [fxprivacy][passwords:fill-ui], URL)

Attachments

(3 attachments, 2 obsolete attachments)

Posted image Design
1) Add key icons to the left column of usernames
2) Add a footer after a separator which opens the password manager
Flags: qe-verify+
Flags: firefox-backlog+
Duplicate of this bug: 1129629
Blocks: 1193404
Priority: -- → P1
Whiteboard: [fxprivacy]
Assignee: nobody → rchtara
Status: NEW → ASSIGNED
Iteration: --- → 43.2 - Sep 7
@ rfeeley: Hi, I started working on the adding the icons to the password autocomplete  And I was wondering  (with mattN) if the footer should match other footers : search box for ex
I was looking at this design today and wondering if we even need the key icons now that the context menu does not have them.

What is the footer of the search box like? Can you post a screen shot?
Posted image screenshot.png
Iteration: 43.2 - Sep 7 → 43.3 - Sep 21
Bug 1189618 - Update the style of the password manager autocomplete popup
Iteration: 43.3 - Sep 21 → 44.1 - Oct 5
Iteration: 44.1 - Oct 5 → ---
Priority: P1 → --
Assignee: riadh.chtara → mconley
Comment on attachment 8771511 [details]
Bug 1189618 [WIP] - Restyle password manager autocomplete popup.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/64630/diff/1-2/
Comment on attachment 8771511 [details]
Bug 1189618 [WIP] - Restyle password manager autocomplete popup.

How does this approach look, Matt?
Attachment #8771511 - Flags: feedback?(MattN+bmo)
(Note that the styling is super-preliminary right now - only works on OS X).
Attachment #8771511 - Flags: feedback?(MattN+bmo)
Comment on attachment 8771511 [details]
Bug 1189618 [WIP] - Restyle password manager autocomplete popup.

Screenshot of what I've got so far on OS X: https://reviewboard.mozilla.org/r/64630/file/320/
Attachment #8771511 - Flags: feedback?(rfeeley)
Attachment #8771511 - Flags: feedback?(MattN+bmo)
Spoke with Shorlander. Looks good. Copy the search box suggestions for style. If we can't open the Saved Logins into it's own native window, then opening them in about:preferences new tab is fine. Don't open a new browser window.
(In reply to Ryan Feeley [:rfeeley] from comment #11)
> If we can't open the Saved Logins into it's own native window, then
> opening them in about:preferences new tab is fine. Don't open a new browser
> window.

Just so we're clear, my initial patch _will_ open the old-school Saved Logins into its own native window. That's the same mechanism that the context "Fill Login" submenu uses.

I assume we want to eventually make it so that we open about:preferences to #security and show the Saved Logins... dialog inside it. That's currently not possible with how about:preferences works, but I think we can make it happen with some tweaks, but I think I might want to do that in a follow-up.

This assumes that opening about:preferences should be the eventual goal. Is that a correct assumption?
Flags: needinfo?(rfeeley)
The eventual goal is not to bring the user deeper into prefs, but to surface the passwords in new UI (a doorhanger, a toolbar panel and/or a sidebar). For today, the native window is much preferred to Saved Logins in about:preferences#security so this is great news to me.
Flags: needinfo?(rfeeley)
Ryan, last time we talked I think you weren't sure about whether UX wanted the icons. What's the current opinion?

Mike, I mentioned the other day to focus on the footer as I wasn't sure if UX wanted the icons at this time. I'm fine with this bug just being about adding the footer then another can be about the icons. I agree with Ryan that it doesn't make sense to send people to a preferences tab while they're in the middle of a task (logging in).
I think Mike is going to be able to make the icons work, though may require a second SVG. The styling will match how search history in the search box has a history icon (except this will be a key). I think it's a nice signal for the user that it's just just text that will be filled but the password too.
Comment on attachment 8771511 [details]
Bug 1189618 [WIP] - Restyle password manager autocomplete popup.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/64630/diff/2-3/
Attachment #8771511 - Flags: review?(MattN+bmo)
Attachment #8771511 - Flags: feedback?(rfeeley)
Attachment #8771511 - Flags: feedback?(MattN+bmo)
Attachment #8771511 - Flags: review?(MattN+bmo)
Comment on attachment 8771511 [details]
Bug 1189618 [WIP] - Restyle password manager autocomplete popup.

https://reviewboard.mozilla.org/r/64630/#review62406

The big question (ignoring icon stuff) is how we want keyboard accessibilty to work, specifically up/down arrows and tab. I personally think it should be accessible by keyboard as it's an essential part of performing the task of logging in if the saved login doesn't show (e.g. if it's on a subdomain).
Having it keyboard accessible could lead to an alternative (possibly simpler) approach which would be to have the footer as the last result in getResultAt, getStyleAt, getCommentAt, etc. and use getStyleAt to style it differently. I think Riadh looked into both approaches but I'm not sure why his patch didn't go with that approach. Did you consider that approach?

::: browser/base/content/browser.xul:147
(Diff revision 3)
>  
>      <!-- for search and content formfill/pw manager -->
> -    <panel type="autocomplete" id="PopupAutoComplete" noautofocus="true" hidden="true"/>
> +    <panel type="autocomplete" id="PopupAutoComplete" noautofocus="true" hidden="true">
> +      <footer id="LoginAutoCompleteFooter">
> +        <button class="plain" label="&savedLogins.label;"
> +                oncommand="LoginHelper.openPasswordManager(window);"/>

This doesn't seem to be working for me btw.

::: browser/themes/osx/browser.css:1837
(Diff revision 3)
> +.autocomplete-treebody::-moz-tree-image(login) {
> +  padding-inline-start: 8px;
> +  padding-inline-end: 8px;
> +  list-style-image: url(chrome://browser/skin/permissions.svg#login-detailed);
> +}

Lets move the icon stuff to a separate bug (or at least separate commit) if it's not going to slow down this landing

::: browser/themes/osx/browser.css:1843
(Diff revision 3)
> +.autocomplete-treebody::-moz-tree-row(login) {
> +  height: 28px; /* <-- this totally doesn't work, unless I remove
> +                 * the login selector from -moz-tree-row, but then

The approach of the footer being a real row may help?

::: toolkit/components/satchel/AutoCompleteE10S.jsm:146
(Diff revision 3)
>    },
>  
>    // This function is used by the login manager, which uses a single message
>    // to fill in the autocomplete results. See
>    // "RemoteLogins:autoCompleteLogins".
> -  showPopupWithResults: function(browserWindow, rect, results) {
> +  showPopupWithResults: function(browserWindow, rect, results, forLogin=false) {

Nit: spaces around operators
I've filed bug 1294502 to combine the nsFormAutoComplete implementations. This is so that they can share the same AutoCompletePopup logic. Then I'm going to re-jig that AutoCompletePopup to use a richlist instead, and try to figure out a nice story for the footer in this spec.
Part of the design for this is in bug 1307316 - https://bug1307316.bmoattachments.org/attachment.cgi?id=8799132

I'm concerned that the lock with the strikethrough and the key icons are confusing when placed next to each other.
Mike, I'm unassigning you for now due to lack of activity but please feel free to take this again if you'd like.
Assignee: mconley → nobody
Status: ASSIGNED → NEW
Priority: -- → P2
Whiteboard: [fxprivacy] → [fxprivacy][passwords:fill-ui]
Totally fair - thanks. :)
We can use the same approach as the insecure password warning (bug 1217162) using a specially styled richlistitem.
Mentor: MattN+bmo
Summary: Update the style of the password manager autocomplete popup → Add a "View Saved Logins" footer to the password manager autocomplete popup
Attachment #8660986 - Attachment is obsolete: true
Attachment #8771511 - Attachment is obsolete: true

This may depend on bug 1519533 if we want to share the custom element but not if we just share CSS styles.

Assignee: nobody → prathikshaprasadsuman
Status: NEW → ASSIGNED
(Assignee)

Comment 25

a month ago

Add a 'View Saved Logins' footer to the password manager autocomplete popup.

(Assignee)

Updated

27 days ago
Depends on: 1530029

Comment 26

24 days ago
Pushed by prathikshaprasadsuman@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/271d947b5c4b
Add a 'View Saved Logins' footer to the password manager autocomplete popup. r=MattN

Comment 27

24 days ago
bugherder
Status: ASSIGNED → RESOLVED
Last Resolved: 24 days ago
status-firefox67: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla67
Whiteboard: [fxprivacy][passwords:fill-ui] → [fxprivacy][passwords:fill-ui][qa-triaged]

Comment 28

22 days ago

Hi, [Firefox version used: latest nightly 67.0a1]. I've tested this issue on several machines: Windows 10, Mac OS X and Ubuntu 16.04 with multiple Facebook accounts and I've got this key with the rest of data as are shown in the screenshots.

Status: RESOLVED → VERIFIED
status-firefox67: fixed → verified
QA Whiteboard: [qa-triaged]
Whiteboard: [fxprivacy][passwords:fill-ui][qa-triaged] → [fxprivacy][passwords:fill-ui]
(Assignee)

Updated

15 days ago
Depends on: 1533205
No longer depends on: 1533205
(Assignee)

Updated

10 days ago
Depends on: 1534442
You need to log in before you can comment on or make changes to this bug.