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

RESOLVED DUPLICATE of bug 1311301

Status

()

Toolkit
Password Manager
P1
normal
RESOLVED DUPLICATE of bug 1311301
3 years ago
9 months ago

People

(Reporter: MattN, Assigned: daleharvey)

Tracking

(Blocks: 2 bugs, {regression, testcase})

unspecified
regression, testcase
Points:
---
Dependency tree / graph
Bug Flags:
firefox-backlog +
qe-verify -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Created attachment 8546961 [details]
Testcase

<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: 1118955
Keywords: regression
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: → bug 424646
Assignee: ally → nobody

Updated

2 years ago
Whiteboard: UI-improvement

Updated

2 years ago
Blocks: 1193404
Whiteboard: UI-improvement → UI-improvement [fxprivacy]

Updated

2 years ago
Priority: P1 → P2

Updated

2 years ago
Priority: P2 → P4
Priority: P4 → P1
Whiteboard: UI-improvement [fxprivacy]

Updated

2 years ago
No longer blocks: 1193404

Comment 4

2 years ago
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
(Assignee)

Comment 6

11 months ago
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)
(Assignee)

Comment 8

10 months ago
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)
(Assignee)

Comment 9

10 months ago
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)
(Assignee)

Comment 11

10 months ago
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
(Assignee)

Comment 12

10 months ago
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: → bug 1311301
(Assignee)

Comment 14

9 months ago
Closing as a dupe since I am working on 1311301 and it contains the fix for this
Status: NEW → RESOLVED
Last Resolved: 9 months ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1311301
You need to log in before you can comment on or make changes to this bug.