Closed Bug 1190938 Opened 9 years ago Closed 8 years ago

password field staying selected under doorhanger

Categories

(Toolkit :: Password Manager, defect, P1)

42 Branch
defect

Tracking

()

VERIFIED FIXED
mozilla51
Iteration:
51.2 - Aug 29
Tracking Status
firefox40 --- unaffected
firefox41 --- wontfix
firefox42 + wontfix
firefox43 + wontfix
firefox44 --- unaffected
firefox51 --- verified

People

(Reporter: kjozwiak, Assigned: rittme)

References

Details

(Whiteboard: [fxprivacy])

Attachments

(6 files, 1 obsolete file)

Attached image issue1.png (deleted) —
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/
Attached image issue2.png (deleted) —
Assignee: nobody → MattN+bmo
Status: NEW → ASSIGNED
Iteration: --- → 42.3 - Aug 10
Points: --- → 2
Flags: qe-verify+
Flags: firefox-backlog+
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)
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)
Blocks: 1193404
Iteration: 42.3 - Aug 10 → 43.1 - Aug 24
Priority: -- → P1
Whiteboard: [fxprivacy]
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: MattN+bmo → bernardo
Iteration: 43.1 - Aug 24 → 43.2 - Sep 7
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.
Iteration: 43.2 - Sep 7 → 43.3 - Sep 21
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?
Flags: needinfo?(bernardo)
Tracking for 42 as it is something we are going to communicate on.
Bug 1190938 - Remove focus from password manager capture doorhanger textboxes when opening. r=MattN
Attachment #8663790 - Flags: review?(MattN+bmo)
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+
Iteration: 43.3 - Sep 21 → 44.1 - Oct 5
Iteration: 44.1 - Oct 5 → ---
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.
I disabled the SHOW button for 42 like we did for 41. See bug 1169702 comment 24.
Flags: needinfo?(MattN+bmo)
Attached image fieldSelected.png (deleted) —
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?
Flags: needinfo?(MattN+bmo)
Wontfixed twice in a row, no activity for a month, wontfixing again for 43.
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.
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.
Flags: needinfo?(bernardo)
Flags: needinfo?(MattN+bmo)
Blocks: 1270321
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
(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)
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 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)
`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 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-
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
Summary: "SHOW" disappearing & password field staying selected under doorhanger → password field staying selected under doorhanger
Attached image screenshot (deleted) —
clear PR to make sheriff pick the right patch
Attachment #8780469 - Attachment is obsolete: true
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
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)
Yes I've test it locally and the username/password field is not focused after tap x button
Flags: needinfo?(gasolin)
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Assignee: gasolin → bernardo
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"
Flags: needinfo?(kjozwiak)
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.

Attachment

General

Created:
Updated:
Size: