Closed Bug 1178855 Opened 5 years ago Closed 4 years ago

Hide password visibility checkbox when users close the capture doorhanger

Categories

(Toolkit :: Password Manager, defect, P4)

defect

Tracking

()

RESOLVED FIXED
mozilla50
Tracking Status
firefox50 --- fixed

People

(Reporter: rfeeley, Assigned: saadq)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [fxprivacy])

Attachments

(2 files)

When users close the capture password door hanger by clicking on the close icon or by selecting not now, the door hanger should close and the key icon should not appear. If there are saved logins for the site, the key icon can remain.
(In reply to Ryan Feeley from comment #0)
> When users close the capture password door hanger by clicking on the close
> icon or by selecting not now, the door hanger should close and the key icon
> should not appear.

I think the behavior of "not now" in door hangers is always expected to be a way for the user to temporarily dismiss the panel without having to make a decision immediately, so I think what you see as a bug was actually by design.

But anyway, this isn't a 'Device Permissions' issue, moving to 'Password Manager'.
Component: Device Permissions → Password Manager
Product: Firefox → Toolkit
(In reply to Florian Quèze [:florian] [:flo] from comment #1)
> (In reply to Ryan Feeley from comment #0)
> > When users close the capture password door hanger by clicking on the close
> > icon or by selecting not now, the door hanger should close and the key icon
> > should not appear.
> 
> I think the behavior of "not now" in door hangers is always expected to be a
> way for the user to temporarily dismiss the panel without having to make a
> decision immediately, so I think what you see as a bug was actually by
> design.
> 

+1.  The user should be able to ignore the key temporarily and get back to it if they want to when they are finished with whatever they are reading on the page.
That's what click elsewhere would continue to do. But "not now" or "x" is a signal IMHO of "not until next time" (not "leave me alone"). We need a way to expose the saved logins, and this is in the way.
(In reply to Ryan Feeley from comment #3)
> That's what click elsewhere would continue to do. But "not now" or "x" is a
> signal IMHO of "not until next time" (not "leave me alone"). We need a way
> to expose the saved logins, and this is in the way.

Does that mean we are keeping the x?
For now, yes.
User testing last year confirmed a few things:
1. When users click "x" or "not now" they assume that the credentials are not saved. They assume it is a "close" action and not "collapse" action as it currently behaves. 
2. They do not notice the key icon after they have "closed" the save password door hanger
3. While they are on a site where they have decided not to save the login, exposing their credentials is only two-clicks away from someone with physical access.
4. Users like being able to show the password, but it needs to be more explicit, and not risk #3.

Therefore I would like to see this prioritized highly as it paves the way for future password management work.
Flags: needinfo?(edwong)
Making this block the meta bug for Password Manager Security, since 3 below could cause a local attacker (friend, family member, etc) to learn about a password that the user never wanted to save.

(In reply to Ryan Feeley [:rfeeley] from comment #6)
> 3. While they are on a site where they have decided not to save the login,
> exposing their credentials is only two-clicks away from someone with
> physical access.
> 4. Users like being able to show the password, but it needs to be more
> explicit, and not risk #3.
>
Blocks: 1118400
Blocks: 1193404
Priority: -- → P4
Whiteboard: [fxprivacy] [triage] → [fxprivacy]
Attached image dont-save.png
We have a lot of footage from user testing that proves that users assume when they click the X that it means "don't save" and not "collapse" as is the current behaviour. We really need to align with user expectation when it comes to security.
Low effort to align the behavior with user expectations.
Flags: needinfo?(jmoradi)
Ryan, I think we talked before about addressing #3 by removing the checkbox (from bug 1217134) after dismissal (regardless of the dismissal method). This avoids interfering with the doorhanger plans and having this prompt have inconsistent UX compared to others.

Note that the visibility toggle in the doorhanger is Nightly-only because of follow-ups like this but I'm hoping to get those wrapped up soon so we can ship it.

Is my proposal to remove the checkbox after the initial dismissal fine for now until the larger doorhanger changes happen.
Flags: needinfo?(rfeeley)
Blocks: 1270321
OK. It still seems a bit odd to keep the key there when users think they have closed (and not collapsed) it, but this removes the password exposure concern.

How are you liking the SHOW functionality? I find myself using it quite a bit.
Flags: needinfo?(rfeeley) → needinfo?(MattN+bmo)
Flags: needinfo?(jmoradi)
Flags: needinfo?(MattN+bmo)
Assignee: nobody → saad
Status: NEW → ASSIGNED
Summary: Close door hanger when users do not want to remember a password (via not now or close icon) → Hide password visibility checkbox when users close the capture doorhanger
Comment on attachment 8761813 [details]
Bug 1178855 - Hide password visibility toggle when capture doorhanger is closed.

https://reviewboard.mozilla.org/r/58878/#review56258

Looks good! Please do a try push of mochitest-Browser before requesting checkin.

::: toolkit/components/passwordmgr/test/browser/browser_capture_doorhanger.js:574
(Diff revision 1)
> +    info("Clicking on anchor to reshow popup.")
> +    let promiseShown = BrowserTestUtils.waitForEvent(panel, "popupshown");
> +    notif.anchorElement.click();
> +    yield promiseShown;
> +
> +    let passwordVisiblityToggle = panel.querySelector('#password-notification-visibilityToggle');

Nit: Use double-quotes
Attachment #8761813 - Flags: review?(MattN+bmo) → review+
Comment on attachment 8761813 [details]
Bug 1178855 - Hide password visibility toggle when capture doorhanger is closed.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/58878/diff/1-2/
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/fx-team/rev/dd3113ee2260
Hide password visibility toggle when capture doorhanger is closed. r=MattN
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/dd3113ee2260
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
Depends on: 1546638
You need to log in before you can comment on or make changes to this bug.