Closed
Bug 1329631
Opened 8 years ago
Closed 8 years ago
Autofill not dismissed after second username selection
Categories
(Toolkit :: Password Manager, defect, P1)
Toolkit
Password Manager
Tracking
()
VERIFIED
FIXED
mozilla53
People
(Reporter: aflorinescu, Assigned: MattN)
References
(Blocks 1 open bug)
Details
(Whiteboard: [fxprivacy])
Attachments
(2 files, 1 obsolete file)
226.51 KB,
video/mp4
|
Details | |
59 bytes,
text/x-review-board-request
|
daleharvey
:
review+
jcristau
:
approval-mozilla-aurora+
|
Details |
[OS:] All. [Versions tested upon:] 52.0a2 20170109004008 53.0a1 20170108030212 [Description:] Autocomplete form not dismissed after second username selection. Steps: 1. Open a login form (http/https) 2. Save 2 passoword manager entries. 3. Refresh the login form. 4. Select an username. 5. Click username field and re-select the username. [Actual Result:] After the 2nd username selection, the autofill is opened again. (see screencast.) - each new username selection will reopen the autofill. In case of using http, selecting the username or clicking on the contextual warning has the same result: the username autofill is opened again. [Expected Result:] When the username is selected the username selection autofill is dismissed and the username/ password are populated. Note: I believe this is a bug introduced by the recent autocomplete changes, maybe by bug 1311301?
Reporter | ||
Updated•8 years ago
|
status-firefox52:
--- → affected
Reporter | ||
Updated•8 years ago
|
Whiteboard: [fxprivacy]
Assignee | ||
Comment 1•8 years ago
|
||
Hey Dale, any chance you have time to look into this? I can repro on OS X too.
Flags: needinfo?(dale)
Comment 4•8 years ago
|
||
Dale, we need to get this into 53 and uplift to aurora/52 asap.
Priority: -- → P1
Updated•8 years ago
|
tracking-firefox52:
--- → ?
Comment 6•8 years ago
|
||
So yup this is caused by https://bugzilla.mozilla.org/show_bug.cgi?id=1311301, nsFormFillController::MarkAsLoginManagerField is being called when the autocomplete entry is clicked which it doesnt expect to be so, so its just opening the popup as if it were being called on page load. This was part of Jonathans original patch but it wasnt said what it was needed for, if this seems good ill get a test written up for it.
Attachment #8825855 -
Flags: feedback?(MattN+bmo)
Comment hidden (mozreview-request) |
Assignee | ||
Comment 8•8 years ago
|
||
Comment on attachment 8825855 [details] [diff] [review] Bug 1329631 - Dont showpopup if called after user click Review of attachment 8825855 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/components/satchel/nsFormFillController.cpp @@ +277,5 @@ > mPwmgrInputs.Put(node, true); > node->AddMutationObserverUnlessExists(this); > > nsFocusManager *fm = nsFocusManager::GetFocusManager(); > + if (fm && !aUserTriggered) { The point of the ShowPopup code is to handle the case where the field is focused before the first MarkAsLoginManagerField occurs. Did you consider only running this code if the field is not already marked instead? ::: toolkit/components/satchel/nsIFormFillController.idl @@ +40,5 @@ > * Autocomplete requests will be handed off to the password manager, and will > * not be stored in form history. > * > * @param aInput - The HTML <input> element to tag > + * @param aUserTriggered - a bool if the user tiggered the filling Nit: typo on "triggered"
Assignee | ||
Comment 9•8 years ago
|
||
mozreview-review |
Comment on attachment 8826019 [details] Bug 1329631 - Only automatically open login manager autocomplete upon first marking. https://reviewboard.mozilla.org/r/104060/#review104788 ::: toolkit/components/passwordmgr/test/mochitest/test_autofocus_js.html:74 (Diff revision 1) > + let formFillController = SpecialPowers.Cc["@mozilla.org/satchel/form-fill-controller;1"]. > + getService(SpecialPowers.Ci.nsIFormFillController); > +let usernameField = iframeDoc.getElementById("form-basic-username"); > + listenForUnexpectedPopupShown() > + formFillController.markAsLoginManagerField(usernameField); I couldn't get the bug to happen using doKey or synthesizeKey for some reason (only when I manually hit enter) so this ended up being a unit test which I figured is better than nothing.
Comment 10•8 years ago
|
||
Comment on attachment 8825855 [details] [diff] [review] Bug 1329631 - Dont showpopup if called after user click Clearing since alternate patch
Attachment #8825855 -
Flags: feedback?(MattN+bmo)
Comment 11•8 years ago
|
||
Comment on attachment 8826019 [details] Bug 1329631 - Only automatically open login manager autocomplete upon first marking. This is much cleaner thanks, I should have thought of that first time round.
Attachment #8826019 -
Flags: review?(dale) → review+
Assignee | ||
Comment 12•8 years ago
|
||
Comment on attachment 8825855 [details] [diff] [review] Bug 1329631 - Dont showpopup if called after user click Thanks
Attachment #8825855 -
Attachment is obsolete: true
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Comment 13•8 years ago
|
||
Dale: needs review + in mozreview so that we can use autoland for this checkin.
Flags: needinfo?(dale)
Keywords: checkin-needed
Comment 14•8 years ago
|
||
mozreview-review |
Comment on attachment 8826019 [details] Bug 1329631 - Only automatically open login manager autocomplete upon first marking. https://reviewboard.mozilla.org/r/104060/#review104926 Looks great, thanks
Assignee | ||
Updated•8 years ago
|
Flags: needinfo?(dale)
Comment 15•8 years ago
|
||
Pushed by mozilla@noorenberghe.ca: https://hg.mozilla.org/integration/autoland/rev/e86a5e67f82e Only automatically open login manager autocomplete upon first marking. r=daleharvey
Comment 16•8 years ago
|
||
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/autoland/rev/14f20d0a83a0 Fix ESLint failures in test_autofocus_js.html on a CLOSED TREE.
Assignee | ||
Updated•8 years ago
|
Assignee: dale → MattN+bmo
Status: NEW → ASSIGNED
Comment 17•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/e86a5e67f82e https://hg.mozilla.org/mozilla-central/rev/14f20d0a83a0
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Assignee | ||
Comment 19•8 years ago
|
||
Comment on attachment 8826019 [details] Bug 1329631 - Only automatically open login manager autocomplete upon first marking. Approval Request Comment [Feature/Bug causing the regression]: Bug 1311301 [User impact if declined]: login autocomplettion re-open when it shouldn't [Is this code covered by automated tests?]: YEs [Has the fix been verified in Nightly?]: Yes [Needs manual test from QE? If yes, steps to reproduce]: Done [List of other uplifts needed for the feature/fix]: None [Is the change risky?]: No [Why is the change risky/not risky?]: Simple early return added [String changes made/needed]: No
Attachment #8826019 -
Flags: approval-mozilla-aurora?
Comment 20•8 years ago
|
||
Comment on attachment 8826019 [details] Bug 1329631 - Only automatically open login manager autocomplete upon first marking. don't show password autofill twice, aurora52+
Attachment #8826019 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 21•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/832fbf29c8a4
Flags: in-testsuite+
Reporter | ||
Comment 22•8 years ago
|
||
Verified as fixed on 52.0a2 20170122004006 on Ubuntu 16.04, Windows 10 and Mac OSX 10.10.
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•