password field staying selected under doorhanger

VERIFIED FIXED in Firefox 51

Status

()

P1
normal
VERIFIED FIXED
3 years ago
2 years ago

People

(Reporter: kjozwiak, Assigned: rittme)

Tracking

(Blocks: 1 bug)

42 Branch
mozilla51
Points:
---
Dependency tree / graph
Bug Flags:
firefox-backlog +
qe-verify +

Firefox Tracking Flags

(firefox40 unaffected, firefox41 wontfix, firefox42+ wontfix, firefox43+ wontfix, firefox44 unaffected, firefox51 verified)

Details

(Whiteboard: [fxprivacy])

Attachments

(6 attachments, 1 obsolete attachment)

(Reporter)

Description

3 years ago
Created attachment 8643141 [details]
issue1.png

This is related to bug # 1186123.. instead of the password being visible when going through the STR, the "SHOW" string will disappear and the password field will stay selected.

- added two screenshots to illustrate the problem

* Win 8.1 x64 -> Reproduced
* Win 10 x64 -> Reproduced
* OSX 10.10.4 -> Reproduced

STR:

- load gmail.com
- type in your moz username and select "Next"
- type in your moz credentials under mozilla.okta.com  and select "Sign In"
- when the doorhanger appears, you'll need to quickly click on the password field than anywhere on the doorhanger a few times before the page finishes loading. You'll notice that after a 1-2 tries, the "SHOW" string will disappear and the password field will stay selected with the blue border.

Used the following build:
- https://ftp.mozilla.org/pub/firefox/nightly/2015-08-04-03-02-04-mozilla-central/
(Reporter)

Comment 1

3 years ago
Created attachment 8643142 [details]
issue2.png
Assignee: nobody → MattN+bmo
Status: NEW → ASSIGNED
Iteration: --- → 42.3 - Aug 10
Points: --- → 2
Flags: qe-verify+
Flags: firefox-backlog+
Created attachment 8644140 [details]
MozReview Request: Bug 1190938 - Remove @focused from the capture password <xul:textbox> upon dismissal/removal.

Bug 1190938 - Remove @focused from the capture password <xul:textbox> upon dismissal/removal.
Kamil, can you test a try build with a possible fix: https://treeherder.mozilla.org/#/jobs?repo=try&revision=221854776b22 (click a "B" when it goes green then click the link beside "Build:" to find the download)

I still can't reproduce this on OS X with the instructions you provided.

Thanks.
Flags: needinfo?(kjozwiak)
(Reporter)

Comment 4

3 years ago
Matt, so I went through the two try builds on Win 10 x64 and OSX 10.10.4 x64, got the following results:

The password field can still get into the state where it always appears selected with SHOW missing from the field. However now when you dismiss the doorhanger, the selection is cleared and SHOW is restored when you bring back the doorhanger via the lock icon.

The next issues is that the username is now staying selected. I got into a case where both the fields were being selected at the same time (issue1.png). Once that happened, I dismissed the doorhanger and brought it back via the lock icon. The password field was cleared but the username was still appearing selected.

For the STR, maybe try these: (I have to try this a few times before I can get it going)

- login into gmail using moz credentials
- once the doorhanger appears, quickly click between the username field/password field while the page is loading
- you'll notice that both fields will end up being selected
Flags: needinfo?(kjozwiak)

Updated

3 years ago
Blocks: 1193404
Iteration: 42.3 - Aug 10 → 43.1 - Aug 24
Priority: -- → P1
Whiteboard: [fxprivacy]
(Assignee)

Comment 5

3 years ago
I looked a bit into it and figured out this problem is probably related to bug 627547.
The problem here seems to be that when the form action page loads the doorhanger is reloaded. During this reloading the field focus seems to get into a strange state. 

I created a page where we can easily reproduce this problem:
http://doorhanger-test.herokuapp.com/5000 (5000 is the number of milliseconds before the server responds and can be changed)

STR:
- open the page, fill the form and submit.
- click at the password field or username field.
- wait for the action page to load.
(Assignee)

Updated

3 years ago
Assignee: MattN+bmo → bernardo

Updated

3 years ago
Iteration: 43.1 - Aug 24 → 43.2 - Sep 7
status-firefox40: --- → unaffected
status-firefox41: --- → affected

Comment 6

3 years ago
Problem: an attacker or family member can drive by your machine and can see the password by clicking on the key icon if password cap/fill is still active.  The plan - as agreed upon 9/1/15 - do not display the 'show' button after the door hanger has been dismissed.

Updated

3 years ago
Iteration: 43.2 - Sep 7 → 43.3 - Sep 21

Comment 7

3 years ago
to document - :rittme is going to work on this when he settles in.
Merge day and GTB for Beta 1 is Sept 21st for Firefox 42.0. Can this land prior to Beta 1, please?

Updated

3 years ago
Flags: needinfo?(bernardo)
Tracking for 42 as it is something we are going to communicate on.
status-firefox41: affected → wontfix
status-firefox43: --- → affected
tracking-firefox42: --- → +
(Assignee)

Comment 10

3 years ago
Created attachment 8663790 [details]
MozReview Request: Bug 1190938 - Remove focus from password manager capture doorhanger textboxes when opening. r=MattN

Bug 1190938 - Remove focus from password manager capture doorhanger textboxes when opening. r=MattN
Attachment #8663790 - Flags: review?(MattN+bmo)
(Assignee)

Comment 11

3 years ago
I'll soon be pushing the tests for this.
Flags: needinfo?(bernardo)
Comment on attachment 8663790 [details]
MozReview Request: Bug 1190938 - Remove focus from password manager capture doorhanger textboxes when opening. r=MattN

https://reviewboard.mozilla.org/r/19857/#review17865
Attachment #8663790 - Flags: review?(MattN+bmo) → review+

Updated

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

Updated

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

Updated

3 years ago
Priority: P1 → --
Matt, Bernado, that is the plan about this issue? We are in the middle of the beta cycle here.
Flags: needinfo?(bernardo)
Flags: needinfo?(MattN+bmo)
No action here. I am going to untrack it for 42.
status-firefox42: affected → wontfix
I disabled the SHOW button for 42 like we did for 41.
See bug 1169702 comment 24.
Flags: needinfo?(MattN+bmo)
status-firefox44: --- → affected
tracking-firefox43: --- → ?
(Reporter)

Comment 16

3 years ago
Created attachment 8676416 [details]
fieldSelected.png

I went through bug # 1169702 comment # 25 and made sure things are still working correctly under fx42 with "SHOW" being removed. However there's still an issue under Windows. Seems like the the blue border around the password field isn't being correctly cleared when clicking away from the password field. Once this happens, logging into any other website will show the password field as being selected via the blue border.

* attached a screenshot to illustrate the problem (password field isn't being selected but still has the blue border around the field)

STR:

- login into facebook.com using Windows (tested using Win 7 & 10)
- once the password doorhanger is visible, click on the "password" field
- close the password doorhanger
- re-open the password doorhanger by selecting the "key" icon and you should notice the password field will still be selected via the blue border
- open a new tab and login into another website (tumblr, twitter, Gmail etc..)
- you'll notice that the password field appears to be selected via the blue border even though the user hasn't pressed on the password field yet
Tracking for 43+, Bernardo, are you working on this? If not let's find someone else to have a look at this bug. MattN, what do you think?
tracking-firefox43: ? → +
tracking-firefox44: --- → +
Flags: needinfo?(MattN+bmo)
Wontfixed twice in a row, no activity for a month, wontfixing again for 43.
status-firefox43: affected → wontfix
Bernardo, please let me know if you are still planning to complete this bug, or if I should work with MattN to find a new owner.
(Assignee)

Comment 20

3 years ago
I'm sorry, I was away for a while.
I'm back now and I'll complete this bug asap.
Flags: needinfo?(bernardo)
Flags: needinfo?(MattN+bmo)
(In reply to Bernardo Rittmeyer [:rittme] from comment #20)
> I'm sorry, I was away for a while.
> I'm back now and I'll complete this bug asap.

Thank you.
Bernardo, Matt: I am trying to follow up on 44+ tracked bugs and wondering if this is something we can fix soon for Beta44. Please let me know.
Flags: needinfo?(bernardo)
Flags: needinfo?(MattN+bmo)
This no longer needs tracking for 44 since the feature was put behind a Nightly-only flag (in bug 1230391) until follow-ups like this are completed.
status-firefox44: affected → unaffected
tracking-firefox44: + → ---
Flags: needinfo?(bernardo)
Flags: needinfo?(MattN+bmo)
After bug 1217134 we will want to re-test this.
Hey Kamil, could you help verify that the two issues of this bug report are now gone in Nightly? Since "SHOW" is gone I'm mostly interested in the password field staying focused.

This is the last remaining issue blocking bug 1270321 so if this issue isn't a problem anymore and you don't have other blocking issues then we will likely ship the feature in Fx50.
Flags: needinfo?(kjozwiak)
Assignee: bernardo → nobody
Status: ASSIGNED → UNCONFIRMED
Points: 2 → ---
Ever confirmed: false
(Reporter)

Comment 26

2 years ago
(In reply to Matthew N. [:MattN] from comment #25)
> Hey Kamil, could you help verify that the two issues of this bug report are
> now gone in Nightly? Since "SHOW" is gone I'm mostly interested in the
> password field staying focused.

It looks like the issue is still occurring with the latest nightly [1]. I used the steps from comment #16 to reproduce. I've added videos to illustrate both the password and username fields staying selected when dismissing the password manager:

* https://youtu.be/Kjl-OsUTDeM (username field staying selected)
** reproduced on both OSX 10.11.5 x64 & Win 10 x64 VM

* https://youtu.be/vsR9AGx7fU8 (password field staying selected)
** reproduced on both OSX 10.11.5 x64 & Win 10 x64 VM

* https://youtu.be/5GeV7NOOaiQ (both username and password field staying selected)
** reproduced on both OSX 10.11.5 x64 & Win 10 x64 VM

OS's Used:

[1] fx 50.0a1, buildId: 20160718030454, changeset: 0fbdcd21fad7
Flags: needinfo?(kjozwiak) → needinfo?(MattN+bmo)

Comment 27

2 years ago
The checkbox is hidden because of 
https://github.com/mozilla/gecko-dev/blob/master/toolkit/components/passwordmgr/nsLoginManagerPrompter.js#L958

Which `wasDismissed` value is never set back.

Will check why we need `wasDismissed` value
Assignee: nobody → gasolin
Flags: needinfo?(MattN+bmo)
Comment hidden (mozreview-request)

Comment 29

2 years ago
Comment on attachment 8780469 [details]
Bug 1190938 - SHOW disappearing & password field staying selected under doorhanger;

remove `wasDismissed` value work in real case

Though the test case seems not trigger `dismissed` event when panel.hidePopup() is called, therefore the passwordVisiblityToggle is still not hidden when popup is closed

https://github.com/mozilla/gecko-dev/blob/master/toolkit/components/passwordmgr/test/browser/browser_capture_doorhanger.js#L568
Attachment #8780469 - Flags: feedback?(MattN+bmo)

Comment 30

2 years ago
`wasDismissed` value is used in PopupNotifications.jsm , so we'd better keep it.

But I found the test assertion is wrong, because the toggle field should be shown after hidePop > showPopup

https://github.com/mozilla/gecko-dev/blob/master/toolkit/components/passwordmgr/test/browser/browser_capture_doorhanger.js#L577
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment on attachment 8780469 [details]
Bug 1190938 - SHOW disappearing & password field staying selected under doorhanger;

https://reviewboard.mozilla.org/r/71172/#review69168

::: toolkit/components/passwordmgr/test/browser/browser_capture_doorhanger.js:571
(Diff revision 3)
>      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");
> -    is(passwordVisiblityToggle.hidden, true, "Check that the Show Password field is Hidden");
> +    ok(!passwordVisiblityToggle.hidden, "Check that the Show Password field is shown");

I think the test was correct and you are regressing the fix from bug 1178855
Attachment #8780469 - Flags: review?(MattN+bmo) → review-

Comment 34

2 years ago
Ooh I got it, the toggle should be hidden when user tap x button, so the lefted issue here is the input field is still get `focused`

I think we just need land Bernardo Rittmeyer's patch (r+'d) and ask kamil to varify it again.
Keywords: checkin-needed

Updated

2 years ago
Summary: "SHOW" disappearing & password field staying selected under doorhanger → password field staying selected under doorhanger

Comment 35

2 years ago
Created attachment 8781823 [details]
screenshot

clear PR to make sheriff pick the right patch
Attachment #8780469 - Attachment is obsolete: true

Comment 36

2 years ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/f1d89ebd1567
Remove focus from password manager capture doorhanger textboxes when opening. r=MattN
Keywords: checkin-needed

Comment 37

2 years ago
kamil I think focus issue has been solved, could you help verify it?
Flags: needinfo?(kjozwiak)
Fred, were you able to reproduce the bug with the steps in comment 16 and videos in comment 26 without the patch? If so, did that patch resolve the issue for you?
Flags: needinfo?(gasolin)

Comment 39

2 years ago
Yes I've test it locally and the username/password field is not focused after tap x button
Flags: needinfo?(gasolin)

Comment 40

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/f1d89ebd1567
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox51: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Assignee: gasolin → bernardo
(Reporter)

Comment 41

2 years ago
Reproduced the original issue using the STR from comment#16 using the following build:
* fx51.0a1, buildId: 20160817030202, changeset: fe895421dfbe
* https://archive.mozilla.org/pub/firefox/nightly/2016/08/2016-08-17-03-02-02-mozilla-central/

Went through verification using the following builds:
* fx51.0a1, buildId: 20160818030226, changeset: 97a52326b06a
* https://archive.mozilla.org/pub/firefox/nightly/2016/08/2016-08-18-03-02-26-mozilla-central/

Platforms Used:
* macOS 10.11.6 x64 - PASSED
* Win 10 x64 VM - PASSED
* Ubuntu 14.04.5 LTS VM - PASSED

Test Cases Used:

* went through the original STR from comment#16
* selected each of the fields, username & password and ensured that it didn't stay selected when
** dismissing the doorhanger via the "X"
** dismissing the doorhanger by clicking anywhere on the page
** dismissing the doorhanger via "Not Now" under the drop menu
** dismissing the doorhanger via pressing "ESC"
status-firefox51: fixed → verified
Flags: needinfo?(kjozwiak)

Updated

2 years ago
Iteration: --- → 51.2 - Aug 29
Priority: -- → P1
Awesome, thanks Fred for investigating and Kamil for the great QE work. It looks like this feature should now be able to ride the trains in bug 1270321!
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.