1186123 - sometimes password visible rather than hidden under the doorhanger

Status: VERIFIED FIXED

(Toolkit :: Password Manager, defect)

Description

[Kamil Jozwiak [:kjozwiak]]

There's times when the password is completely visible under the doorhanger once you've logged into a website. I've ran into this issue at least 5-6 times but unfortunately I can't get reliable STR.

Every time I've seen this happen, I was going through the following work flow: (I had this happen to me at least twice when I was trying to get reliable STR for this bug)

- installed the latest version of m-c
- visited gmail.com and typed in my personal username and clicked "Next"
- inserted my password in the next screen and clicked "Sign In"
- clicked "Remember" after the doorhanger appeared with my username and password
- inserted my 2FA and selected "Verify"
- once logged in, I selected "Add Account" under Gmail
- typed in my mozilla username and selected "Next"
- dismissed the doorhanger that appeared using the "X" at the top right of the doorhanger (which had my previous gmail credentials listed)
- typed in my LDAP username and password and selected "Sign In"

Once I signed in the doorhanger appeared for the second time, this time having my mozilla credentials filled in but the password was completely visible. Clicking on the doorhanger or dismissing the doorhanger wouldn't hide the password. The only way it would go back to the hidden state would be if I click on the username field, than the password field and at that point click anywhere on the doorhanger.

Comment 1

[Matthew N. [:MattN]]

Can you reproduce this in beta 40? Was it only OS X where you've noticed this?

I have a simple speculative fix but I haven't been able to repro this.

Comment 2

[Matthew N. [:MattN]]

Attachment: MozReview Request: Bug 1186123 - Ensure the password field is masked by default in the capture doorhanger. r=rittme

Bug 1186123 - Ensure the password field is masked by default in the capture doorhanger. r=rittme

* The `type` setter won't work if the <xul:textbox> lost its binding before the blur occurs so switch to setAttribute.
* For good measure, set the type attribute to "password" before setting the value.

Comment 3

[Bernardo Rittmeyer [:rittme]]

Comment on attachment 8640919 [details]
MozReview Request: Bug 1186123 - Ensure the password field is masked by default in the capture doorhanger. r=rittme

https://reviewboard.mozilla.org/r/14397/#review13013

::: toolkit/components/passwordmgr/nsLoginManagerPrompter.js:871
(Diff revision 1)
> -               .setAttribute("value", login.password);
> +      // Ensure the type is reset so we the field is masked.

"so we the field is masked."

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/b0ea220ccfb3

Comment 5

[Matthew N. [:MattN]]

Comment on attachment 8640919 [details]
MozReview Request: Bug 1186123 - Ensure the password field is masked by default in the capture doorhanger. r=rittme

Approval Request Comment
[Feature/regressing bug #]: Bug 1169702
[User impact if declined]: A user's password may be exposed in plaintext on the screen after submitting a login form or re-opening the remember password panel. This would be a problem if someone else was watching the screen.
[Describe test coverage new/current, TreeHerder]: Straight-forward speculative fix that can't be tested by me since I can't reproduce the problem but I'm fairly sure the 2 fixes will address the issue.
[Risks and why]: Low risk - switching to a more durable method of switching the type for XBL elements & proactively making sure the type is correct whenever the panel opens.
[String/UUID change made/needed]: None

Comment 6

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8640919 [details]
MozReview Request: Bug 1186123 - Ensure the password field is masked by default in the capture doorhanger. r=rittme

I reviewed this request with Matt and team. We're going to pref this feature off in 40 so the fix isn't required. This fix is required for 41 but it should both land on m-c and be verified before uplift. Beta-

Comment 7

[Matthew N. [:MattN]]

Kamil, can you confirm that this is fixed in tomorrow's Nightly or you can use an fx-team build like http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/fx-team-macosx64/1438237842/firefox-42.0a1.en-US.mac.dmg for OS X

Comment 8

[[Back July 11] Ryan VanderMeulen]

https://hg.mozilla.org/mozilla-central/rev/b0ea220ccfb3

Comment 9

[Kamil Jozwiak [:kjozwiak]]

I ended up finding a way to reproduce the original issue that I mentioned in comment #0 which made verification a lot easier. This is a pretty bad bug. When you end up in the situation where the password is being shown and not being hidden, every other login session would make the password visible under the doorhangers until you either fix it by closing the browser session or making the password hidden once again by clicking on the username, the password field and than somewhere on the doorhanger.

Here are more reliable STR that can be used rather than the STR in comment #0:

- load gmail.com
- type in your moz username and select "Next"
- type in your moz credentials under mozilla.okta.com 
- once you typed in your credentials, select "Sign In"
- when the doorhanger appears, quickly click on the password field than anywhere on the doorhanger (you need to do this very quickly a few times before the page finishes loading)

Win 8.1 x64 -> PASSED (still reproducible on m-a)
Win 10 x64 -> PASSED (still reproducible on m-a)
OSX 10.10.4 -> PASSED (still reproducible on m-a)

However now there's a new bug that replaced this one. Rather than making the password visible when going through the above STR, the "SHOW" string in the password field will disappear and the password field will stay selected. I created bug # 1190938.

Comment 10

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8640919 [details]
MozReview Request: Bug 1186123 - Ensure the password field is masked by default in the capture doorhanger. r=rittme

Let's uplift to Aurora as this fix has been verified on Nightly.

Comment 11

[Matthew N. [:MattN]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/a6bb84ddeed3

Comment 12

[Kamil Jozwiak [:kjozwiak]]

This also affects fx40 when signon.rememberSignons.visibilityToggle;true however it's disabled by default. Do we want to push this into fx40 or let it ride the trains from m-a?

Comment 13

[Kamil Jozwiak [:kjozwiak]]

Reproduced the issue using the following builds:
- https://ftp.mozilla.org/pub/firefox/nightly/2015-08-03-00-40-02-mozilla-aurora/

Went through verification using the following build:
- https://ftp.mozilla.org/pub/firefox/nightly/2015-08-06-00-40-06-mozilla-aurora/

OS's Used:

- OSX 10.10.4 x64 -> PASSED
- Win 10 x64 -> PASSED
- Win 8.1 x64 (VM) -> PASSED
- Ubuntu 14.04.2 x64 -> PASSED