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

NEW
Unassigned

Status

()

Toolkit
Password Manager
P2
enhancement
2 years ago
29 days ago

People

(Reporter: MattN, Unassigned, Mentored)

Tracking

(Blocks: 2 bugs)

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

Firefox Tracking Flags

(Not tracked)

Details

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

Attachments

(2 attachments, 2 obsolete attachments)

Created attachment 8641502 [details]
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

Updated

2 years ago
Blocks: 1193404
Priority: -- → P1
Whiteboard: [fxprivacy]

Updated

2 years ago
Assignee: nobody → rchtara

Updated

2 years ago
Status: NEW → ASSIGNED

Updated

2 years ago
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?
Created attachment 8656851 [details]
screenshot.png

Updated

2 years ago
Iteration: 43.2 - Sep 7 → 43.3 - Sep 21
Created attachment 8660986 [details]
MozReview Request: Bug 1189618 - Update the style of the password manager autocomplete popup

Bug 1189618 - Update the style of the password manager autocomplete popup

Updated

2 years ago
Iteration: 43.3 - Sep 21 → 44.1 - Oct 5

Updated

2 years ago
Iteration: 44.1 - Oct 5 → ---

Updated

2 years ago
Priority: P1 → --

Updated

11 months ago
Assignee: riadh.chtara → mconley

Comment 6

11 months ago
Created attachment 8771511 [details]
Bug 1189618 [WIP] - Restyle password manager autocomplete popup.

Review commit: https://reviewboard.mozilla.org/r/64630/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/64630/

Comment 7

11 months ago
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 8

11 months ago
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)

Comment 9

11 months ago
(Note that the styling is super-preliminary right now - only works on OS X).

Updated

10 months ago
Attachment #8771511 - Flags: feedback?(MattN+bmo)

Comment 10

10 months ago
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)

Comment 11

10 months ago
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.

Comment 12

10 months ago
(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)

Comment 13

10 months ago
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).

Comment 15

10 months ago
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 16

10 months ago
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
Comment hidden (obsolete)

Updated

10 months ago
Depends on: 1294502

Comment 19

10 months ago
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.

Updated

9 months ago
Depends on: 1296638
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.
Blocks: 1166112
Depends on: 1129629
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@mozilla.com
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
You need to log in before you can comment on or make changes to this bug.