Closed Bug 1120037 Opened 10 years ago Closed 8 years ago

Username autocompletion isn't attached initially when the username field with autocomplete=off is focused with <body onload="field.focus()">

Categories

(Toolkit :: Password Manager, defect, P1)

Product:
Toolkit
Component:
Password Manager
Type:
defect

Tracking

()

Status:
RESOLVED DUPLICATE of bug 1311301

People

(Reporter: MattN, Assigned: daleharvey)

References

(Blocks 1 open bug)

Details

(Keywords: regression, testcase)

Attachments

(1 file)

Testcase
(deleted), text/html
Details
Attached file Testcase (deleted) —
<body onLoad='document.forms[0].user.focus();'> with <input type='text' name="user" id="user" autocomplete=off > doesn't get the username autocompletion attached upon page load but will it will if you tab to the next field and move the focus back. This seems to only have with autocomplete=off so I think there is code still bailing when it's off (perhaps a race condition). I actually thought we previously didn't attach the autocomplete dropdown when @autocomplete=off was on the username field.
Flags: qe-verify-
Flags: firefox-backlog+
Blocks: passwords-2015-Q1
Priority: -- → P1
Assignee: nobody → ally
I played around with it, and it's fairly reliable, but not totally reproducible, which suggests a race per MattN's hypothesis. I think we are running afoul of http://mxr.mozilla.org/mozilla-central/source/toolkit/components/passwordmgr/LoginManagerContent.jsm#690 and possibly the asynchronousity of http://mxr.mozilla.org/mozilla-central/source/toolkit/components/passwordmgr/LoginManagerContent.jsm#203
What exactly is going wrong here? Pressing down-arrow in the username field doesn't offer the autocomplete dropdown menu (to select a username)? Will it offer satchel history instead? It would be useful to know 2 things: 1) is the markAsLoginManagerField() in _fillForm() being run? This should happen while the page is loading (as a result of DOMFormHasPassword invoking _fillForm) 2) what does the code in nsFormFillController.cpp checking mPwmgrInputs think is happening. I think the only other code involved are the DOMAutoComplete and blur listeners in content.js... Are those firing when they should be?
FYI: This may end up being a dupe of bug 424646.
See Also: → https://bugzilla.mozilla.org/show_bug.cgi?id=424646
Assignee: ally → nobody
Whiteboard: UI-improvement
Blocks: 1193404
Whiteboard: UI-improvement → UI-improvement [fxprivacy]
Priority: P1 → P2
Priority: P2 → P4
Priority: P4 → P1
Whiteboard: UI-improvement [fxprivacy]
No longer blocks: 1193404
As I see recent activity on this ticket: This bug seems resolved to me. On current FF 41.0, I can no longer reproduce the issue - neither for my usecase (roundcube login) nor for the attached test case.
I can still reproduce the bug with the testcase (and recently in the wild) with the latest Nightly on OS X. Remember that this bug is about autocomplete, not autofill so you need to have multiple saved logins for the site or be in a private window.
Blocks: 1217152
Blocks: 1304224
Im gonna take a look at this shortly
Assignee: nobody → dale
Dale, do you have an update here? We need to figure out what is going on here and fix in Firefox 52.
Flags: needinfo?(dale)
Hi Tanvi, apologies I was a little early picking this up while I was finishing other work, however Mike mentioned that this is turning into a priority so looking into it asap
Flags: needinfo?(dale)
A little confused about the actual bug report here, I can reproduce the behaviour fairly reliably, however the title suggests that its a bug that the autocomplete form only shows once the username has been blurred + refocused, isnt the bug that the autocomplete is attached at all? shouldnt autocomplete=off be making sure it does not get attached?
Flags: needinfo?(MattN+bmo)
(In reply to Dale Harvey (:daleharvey) from comment #9) > A little confused about the actual bug report here, I can reproduce the > behaviour fairly reliably, however the title suggests that its a bug that > the autocomplete form only shows once the username has been blurred + > refocused, isnt the bug that the autocomplete is attached at all? No. > shouldnt > autocomplete=off be making sure it does not get attached? We ignore autocomplete=off for username/passwords fields: https://developer.mozilla.org/en-US/docs/Web/Security/Securing_your_site/Turning_off_form_autocompletion#The_autocomplete_attribute_and_login_fields
Flags: needinfo?(MattN+bmo)
Ok thanks Matt, I hadnt noticed that part of the docs Just a little update on debugging, looks like Justin was on the right track > what does the code in nsFormFillController.cpp checking mPwmgrInputs think is happening. Currently onload nsFormFillController is failing the check @ http://searchfox.org/mozilla-central/source/toolkit/components/satchel/nsFormFillController.cpp#918 because it doesnt think the input is controlled by password manager, figuring out why now
ok yeh, so |onDOMFormHasPassword| gets hit before the input gets focus, but then it sends a message to find the logins @ https://dxr.mozilla.org/mozilla-central/source/toolkit/components/passwordmgr/LoginManagerContent.jsm#406 and by the time the parent sends back the found logins the input has focus but has not been marked as a password manager input so autocomplete stays disabled. Easiest fix would seem to be to trigger |nsFormFillController::MaybeStartControllingInput| once |fillForm| has succeeded? or we could move |MarkAsLoginManagerField| to happen before sending the message to look for logins, Matt what do you think is the best option here?
Flags: needinfo?(MattN+bmo)
(In reply to Dale Harvey (:daleharvey) from comment #12) > ok yeh, so |onDOMFormHasPassword| gets hit before the input gets focus, but > then it sends a message to find the logins @ > https://dxr.mozilla.org/mozilla-central/source/toolkit/components/ > passwordmgr/LoginManagerContent.jsm#406 and by the time the parent sends > back the found logins the input has focus but has not been marked as a > password manager input so autocomplete stays disabled. > > Easiest fix would seem to be to trigger > |nsFormFillController::MaybeStartControllingInput| once |fillForm| has > succeeded? Yeah, I think that's similar to what https://reviewboard.mozilla.org/r/87632/diff/2 (bug 1311301) is doing. It's calling ShowPopup (perhaps it should call MaybeStartControllingInput) from MarkAsLoginManagerField if that field is already focused. Do you think your suggestion is better than :jkt's patch now that you've dug into the code? If you like jkt's approach, can you help review it? Perhaps the bugs end up being duped together. I think his depends on bug 376668. > or we could move |MarkAsLoginManagerField| to happen before > sending the message to look for logins The current ordering is intentional since we currently show form history autocomplete results when there are no matching login results. If the field was marked as a login manager field then we would no longer show form history results in this case. Ideally we would always intelligently combine form history and usernames in the dropdown (bug 1166112) but I don't think we've found good UX for that yet. I think we should keep the current behaviour for now to avoid other changes that I mention which aren't necessary to fix this.
Flags: needinfo?(MattN+bmo)
See Also: → https://bugzilla.mozilla.org/show_bug.cgi?id=1311301
Closing as a dupe since I am working on 1311301 and it contains the fix for this
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: