Flip prefs to disable login autofill on HTTP and enable the warning on insecure login fields

VERIFIED FIXED in Firefox 52

Status

()

Toolkit
Password Manager
P1
normal
VERIFIED FIXED
2 years ago
2 months ago

People

(Reporter: tanvi, Assigned: tanvi, NeedInfo)

Tracking

(Depends on: 8 bugs, Blocks: 5 bugs, {dev-doc-complete, site-compat, user-doc-needed})

44 Branch
mozilla53
dev-doc-complete, site-compat, user-doc-needed
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox51 wontfix, firefox52- verified, firefox53 verified)

Details

(Whiteboard: security:passwords, [fxprivacy])

Attachments

(2 attachments, 5 obsolete attachments)

(Assignee)

Description

2 years ago
Filling a bug specifically for disabling autofill on HTTP pages.  If the user has a saved password on an HTTP page, the Password Manager should not fill it automatically.  Instead, the user should have to first type in or select their username.  At that point (after user interaction) the Password Manager can fill the password.

Given bug https://bugzilla.mozilla.org/show_bug.cgi?id=1179961 has landed, Firefox will be sending mixed messages if we both warn the user that the password could be compromised and fill it in for them.  Hence, it's time to finally stop autofilling passwords on HTTP pages!

+++ This bug was initially created as a clone of Bug #1118511 +++

When passwords are autofilled by the Password Manager, javascript on the page can read the filled in passwords even when the user has no intent to login to the page.  Hence, pages that are vulnerable to javascript injection (via XSS or MITM) are also subject to password exfiltration attacks[1] (in some cases without any indication to the user that such an attack has taken place).  To protect against this, we should not autofill passwords without user interaction / user intent to login.  This protects users from a number of the drive by attacks and protects users who never planned to login.

From a usability perspective, it is debatable whether or not autofill is a good user experience.  It allows for an easy and frictionless login, but can also cause confusion (how did my password get filled in?). Discussion about a more active login experience (rather than a passive one) are underway with UX.

If we do decide that we need to continue with an autofill experience, then there are cases where we definitely should not autofill.  (Note that this will make a less consistent experience for users and won't protect us on a page with an XSS vulnerability.)  In the below cases, we MUST not autofill:
* For http sites (IE 11 has this security feature)
* https sites that have mixed active content
* https sites that require a cert override (chrome does this)
* in iframed sites (safari does this for non-same origin, chrome does this for all frames).
* Invisible form fields (visibility and opacity, although this isn't going to prevent clickjacking attacks to autofill the passwords)

Previous bugs have been filed to address subsets of this problem.  I've added them to the dependent list.

[1] https://www.usenix.org/conference/usenixsecurity14/technical-sessions/presentation/silver
(Assignee)

Comment 1

2 years ago
Is this as simple as adding && !hasInsecureLoginForms here:

http://mxr.mozilla.org/mozilla-central/source/toolkit/components/passwordmgr/LoginManagerContent.jsm#501

let autofillForm = gAutofillForms && !PrivateBrowsingUtils.isContentWindowPrivate(doc.defaultView) && !hasInsecureLoginForms

Paolo, does that look right?
Flags: needinfo?(paolo.mozmail)

Updated

2 years ago
Whiteboard: security:passwords, [fxprivacy] → security:passwords, [fxprivacy] [triage]

Comment 2

2 years ago
We probably want to check whether the specific page where we are autofilling is secure or not, ignoring subframes. In any case, insecure subframes or sibling frames (in the mixed content case) may load asynchronously after we have already autofilled.

If an HTTPS frame is loaded by an HTTP page, is it safe to autofill in the HTTPS frame? I guess so, since they are considered different origins, correct? In that case, we don't need to consider parent frames also.
Flags: needinfo?(paolo.mozmail)

Updated

2 years ago
Whiteboard: security:passwords, [fxprivacy] [triage] → security:passwords, [fxprivacy]

Comment 3

2 years ago
This is NOT a good idea.  My Password Manager database includes 32 entries for HTTP sites.  None of these contain any information about me that would create a vulnerability regarding my identity or finances.  Primarily, I have accounts at these sites that merely contain my preferences for formatting the displays.

Comment 4

2 years ago
If we're telling them in the address bar their password is insecure we shouldn't at the same time be autofilling the password field. It's a mixed message to the user.

Updated

2 years ago
Priority: P2 → P3

Updated

2 years ago
Blocks: 1216897
No longer blocks: 1217142
(Assignee)

Updated

2 years ago
Blocks: 1188121

Updated

2 years ago
No longer blocks: 1216897
(Assignee)

Updated

2 years ago
Blocks: 1217142
Reviewed at the team planning meeting.  Determined to be non-MVP work and moved to our maintenance meta.
Blocks: 1216897
No longer blocks: 1188121
(Assignee)

Comment 6

a year ago
Since we now show an insecure password warning for login forms over HTTP in both Dev Edition and Nightly, we are sending a mixed message by autofilling on HTTP.  We need to stop autofilling as soon as someone has the cycles to pick up this bug.

https://blog.mozilla.org/tanvi/2016/01/28/no-more-passwords-over-http-please/

Matt, you may be interested after your work on https://bugzilla.mozilla.org/show_bug.cgi?id=667233 is complete.

Comment 7

a year ago
I strongly disagree with comment #6.  There are still a number of HTTP Web sites that require logins, most of which involve no security risks.  Instead, the act of logging-in merely retrieves the user's preferred configuration of the site.  We waited a long time for other bugs to be implemented to autofill user IDs and passwords, even for HTTP sites.  Please do not undo that.
Duplicate of this bug: 1269514
Assignee: nobody → jhofmann
Status: NEW → ASSIGNED
In my opinion, bug 1289913, bug 1120037, and bug 376668 are blockers to shipping this since we need the be able to help the users complete the task at hand which is logging in to the website. Users will just type their passwords in manually if he don't help them so providing fallback fill UI isn't a major security drawback, especially once we have contextual warning UI (bug 1217162).
Blocks: 1217162
Depends on: 1289913, 1120037, 376668
(Assignee)

Comment 10

9 months ago
(In reply to Matthew N. [:MattN] from comment #9)
> In my opinion, bug 1289913, bug 1120037, and bug 376668 are blockers to
> shipping this since we need the be able to help the users complete the task
> at hand which is logging in to the website. Users will just type their
> passwords in manually if he don't help them so providing fallback fill UI
> isn't a major security drawback, especially once we have contextual warning
> UI (bug 1217162).

I'm not sure all of these are blockers, but bug 376668 is the most important of the 3 in my opinion - show the autocomplete drop down on focus of the username field.
Is there anyway to determine usage of password manager for http sites, specifically?
(In reply to Tanvi Vyas [:tanvi] from comment #10)
> (In reply to Matthew N. [:MattN] from comment #9)
> > In my opinion, bug 1289913, bug 1120037, and bug 376668 are blockers to
> > shipping this since we need the be able to help the users complete the task
> > at hand which is logging in to the website. Users will just type their
> > passwords in manually if he don't help them so providing fallback fill UI
> > isn't a major security drawback, especially once we have contextual warning
> > UI (bug 1217162).
> 
> I'm not sure all of these are blockers, but bug 376668 is the most important
> of the 3 in my opinion - show the autocomplete drop down on focus of the
> username field.

Without fixing all three we will have some form of UX regression for affected sites.

(In reply to Peter Dolanjski [:pdol] from comment #11)
> Is there anyway to determine usage of password manager for http sites,
> specifically?

Yes, we should have recorded at form submission time whether a form was secure (like I suggested in bug 1174333 comment 7) but instead only measured the existence of forms so we don't currently have that data.

I know many self-hosted forum sites (e.g. phpBB/SMF) don't offer HTTPS and I suspect they are a large chunk of the 35% of insecure login forms and I would hate to break them (especially since there is no indication Chrome will at the moment).
So maybe this got lost but we would put this behind a pref anyway. We'd leave it off for now and I would add a telemetry probe to the patch that collects data on how many HTTP pages would get skipped, no matter the pref.

Sounds fine?
Depends on: 1302474
(In reply to Johann Hofmann [:johannh] from comment #13)
> So maybe this got lost but we would put this behind a pref anyway.

I filed bug 1302474 to add the pref so we don't need to change the whole dependency tree.

> We'd leave it off for now and I would add a telemetry probe to the patch that
> collects data on how many HTTP pages would get skipped, no matter the pref.

Adding the reason to `const AUTOFILL_RESULT` makes sense but that should only record the real reason for not filling meaning that it should honour the pref. I don't think it's necessary to add a probe to know how often we wouldn't fill since it doesn't sound like it will affect our decision to eventually disable this (and provide manual fill methods such as autocomplete) but I also won't stop you from adding a new probe.
Assignee: jhofmann → nobody
Status: ASSIGNED → NEW
Summary: Don't autofill passwords on HTTP pages with the Password Manager → Add a pref to not autofill passwords on HTTP pages with the Password Manager
Summary: Add a pref to not autofill passwords on HTTP pages with the Password Manager → Don't autofill passwords on HTTP pages with the Password Manager
(Assignee)

Updated

8 months ago
Blocks: 1304224
(Assignee)

Updated

8 months ago
No longer blocks: 1217162
(Assignee)

Updated

8 months ago
Assignee: nobody → tanvi
(Assignee)

Comment 15

7 months ago
Marking as P1 because this needs to land in Firefox 52.
Priority: P3 → P1
This bug could also flip the security.insecure_field_warning.contextual.enabled pref too.
(Assignee)

Comment 17

7 months ago
Created attachment 8809221 [details] [diff] [review]
Bug1217152-11-09-16.patch

We still have to wait for a few more bugs to land, but I wanted to get this patch and review ready for when the dependencies land.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=957be548837f50ae54639992e1f674267d37f1d6
Attachment #8809221 - Flags: review?(MattN+bmo)
Comment on attachment 8809221 [details] [diff] [review]
Bug1217152-11-09-16.patch

Review of attachment 8809221 [details] [diff] [review]:
-----------------------------------------------------------------

::: modules/libpref/init/all.js
@@ +4343,5 @@
>  // Login Manager prefs
>  pref("signon.rememberSignons",              true);
>  pref("signon.rememberSignons.visibilityToggle", true);
>  pref("signon.autofillForms",                true);
> +pref("signon.autofillForms.http",           false);

This will disable HTTP autofill for all toolkit applications but I'll need to think about whether the other applications will get the fallback UI (autocomplete) for free or whether we should just override this to false in firefox.js
Comment on attachment 8809221 [details] [diff] [review]
Bug1217152-11-09-16.patch

Review of attachment 8809221 [details] [diff] [review]:
-----------------------------------------------------------------

::: modules/libpref/init/all.js
@@ +4343,5 @@
>  // Login Manager prefs
>  pref("signon.rememberSignons",              true);
>  pref("signon.rememberSignons.visibilityToggle", true);
>  pref("signon.autofillForms",                true);
> +pref("signon.autofillForms.http",           false);

I think it's fine for the toolkit defaults to be secure and another app can override this to true if they want a less secure application.
Attachment #8809221 - Flags: review?(MattN+bmo) → review+

Comment 20

6 months ago
I am confused.  Will there be an option to still get autofill of passwords on HTTP sites?  

There are a number of Web sites that are still not HTTPS that require logins.  For me, those sites contain NO personal information.  The login merely causes those sites to be formatted in a way I previously setup.
(In reply to David E. Ross from comment #20)
> I am confused.  Will there be an option to still get autofill of passwords
> on HTTP sites?  
> 
> There are a number of Web sites that are still not HTTPS that require
> logins.  For me, those sites contain NO personal information.  The login
> merely causes those sites to be formatted in a way I previously setup.

You can still fill logins via the context menu (right click). You could also simply flip the "signon.autofillForms.http" pref to true in your personal about:config (once we change it to false by default).
(In reply to Johann Hofmann [:johannh] from comment #21)
> (In reply to David E. Ross from comment #20)
> > I am confused.  Will there be an option to still get autofill of passwords
> > on HTTP sites?  
> > 
> > There are a number of Web sites that are still not HTTPS that require
> > logins.  For me, those sites contain NO personal information.  The login
> > merely causes those sites to be formatted in a way I previously setup.
> 
> You can still fill logins via the context menu (right click). 

and (more obviously) the autocomplete popup.
status-firefox44: affected → ---
status-firefox51: --- → wontfix
status-firefox52: --- → affected
status-firefox53: --- → affected
(Assignee)

Updated

6 months ago
Status: NEW → ASSIGNED
[Tracking Requested - why for this release]: Enable the new feature

Adrian, can you do a QE pass on the whole insecure field contextual warning and login manager autocomplete changes when this lands or by flipping the two prefs yourself.

Known bug dependencies: https://mnoorenberghe.github.io/bz-dependency-buglist/?list=1304224
Tracked work: https://docs.google.com/spreadsheets/d/155k3Ja06QVgRj9F_DnAJxC8wfH_lWM4N9eZal4llefk/edit#gid=1492590763
tracking-firefox52: --- → ?
Flags: qe-verify+
Flags: needinfo?(adrian.florinescu)
Summary: Don't autofill passwords on HTTP pages with the Password Manager → Flip prefs to disable login autofill on HTTP and enable the warning on insecure login fields
(Assignee)

Comment 24

5 months ago
Created attachment 8821689 [details] [diff] [review]
Bug1217152-selective-tests-WIP.patch

Disabling insecure password form field warning and enabling HTTP autofill on selective tests.  This got to be a bit excessive, so Matt suggested we just flip the prefs for the whole directory until HTTPS mochitests are supported.  There are a handful that don't need them.  I went through everything in http://searchfox.org/mozilla-central/source/toolkit/components/passwordmgr/test/mochitest/ up to test_prompt_http.html.

Will provide a new patch that just updates the whole directory.
(Assignee)

Comment 25

5 months ago
Created attachment 8821691 [details] [diff] [review]
Bug1217152-pwmgrcommon-tests.patch

Disable insecure password warning and enable autofill on HTTP pages for password manager tests using pwmgr_common.js
Attachment #8821691 - Flags: review?(MattN+bmo)
(Assignee)

Comment 26

5 months ago
Push to try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=bb466892361f4671691ebd1a88758d03cf416fc5
(Assignee)

Comment 27

5 months ago
Created attachment 8821698 [details] [diff] [review]
Bug1217152-12-23-16.patch

Rebased and update comment.  Carrying over r+.
Attachment #8809221 - Attachment is obsolete: true
Attachment #8821698 - Flags: review+
(Assignee)

Comment 28

5 months ago
Created attachment 8821699 [details] [diff] [review]
Bug1217152-pwmgrcommon-tests.patch

Fixed trailing whitespace.
Attachment #8821691 - Attachment is obsolete: true
Attachment #8821691 - Flags: review?(MattN+bmo)
Attachment #8821699 - Flags: review?(MattN+bmo)
(Assignee)

Comment 29

5 months ago
Matt, I'm out next week.  Can you review the test patch, and if the patch and try looks good, mark this checkin-needed and request uplift?
Flags: needinfo?(MattN+bmo)
(Assignee)

Comment 30

5 months ago
Try shows that updating https://treeherder.mozilla.org/#/jobs?repo=try&revision=bb466892361f4671691ebd1a88758d03cf416fc5&selectedJob=33416806 pwmgr_common.js doesn't work so well, since some tests need to change the pref values.  Perhaps the code in pwmgr_common.js is getting called after the individual tests change the pref to the value they desire.

Anyway, we might have to go with the individual test approach after all.  So I've pushed my WIP test patch (which is attached to this bug) to try to see what remaining tests we need to individually update:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=267ec9cd52f3d832079c2daa19a9512232f39849
(Assignee)

Comment 31

5 months ago
Created attachment 8821714 [details] [diff] [review]
Bug1217152-selective-tests-WIP-12-23-16B.patch

Fixing the trailing whitespace again.
Attachment #8821689 - Attachment is obsolete: true
(Assignee)

Comment 32

5 months ago
Try shows 2 remaining tests that need to be updated:

TEST-UNEXPECTED-FAIL | toolkit/components/passwordmgr/test/mochitest/test_recipe_login_fields.html | Test timed out.
TEST-UNEXPECTED-FAIL | toolkit/components/passwordmgr/test/test_master_password.html | Test timed out.

Something may also have to be done for this one:
TEST-UNEXPECTED-FAIL | toolkit/components/passwordmgr/test/mochitest/test_basic_form_autocomplete.html 

Matt, hoping you can take this during the week!
(In reply to Matthew N. [:MattN] (PM me if requests are blocking you) from comment #23)
> [Tracking Requested - why for this release]: Enable the new feature
> 
> Adrian, can you do a QE pass on the whole insecure field contextual warning
> and login manager autocomplete changes when this lands or by flipping the
> two prefs yourself.
> 
> Known bug dependencies:
> https://mnoorenberghe.github.io/bz-dependency-buglist/?list=1304224
> Tracked work:
> https://docs.google.com/spreadsheets/d/
> 155k3Ja06QVgRj9F_DnAJxC8wfH_lWM4N9eZal4llefk/edit#gid=1492590763

Hello Matt, few questions below:

1. Are the preferences in discussion: (a)"security.insecure_field_warning.contextual.enabled" and (b)"signon.autofillForms.http"? Their defaults today (12.27) in Nightly53 are: (a) - false and (b)- true.
The defaults when this lands will be a-true and b -false as the title of this issue suggests?
2. I'm not sure what exactly you mean by the "login manager autocomplete changes" - could you specify a list of (meta)bugs that cover this? Would this also cover the email tread with the password autocomplete? guessing this would be it bug 1318203?
3. I see that the tracking is set for 52/53, so I'll treat this work as a bug verification work(exploratory testing and bug fix verification), but I'd prefer if the time allows to treat it as a fully fledged feature (this means test plan, test casing, etc). So, please let me know how are we faring on the time chapter. Meanwhile, I'll go with the 1st version - bug verification work(exploratory testing and bug fix verification).
Comment on attachment 8821699 [details] [diff] [review]
Bug1217152-pwmgrcommon-tests.patch

Review of attachment 8821699 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with the clearUserPref lines removed

::: toolkit/components/passwordmgr/test/pwmgr_common.js
@@ +443,4 @@
>    SimpleTest.registerCleanupFunction(() => {
>      SpecialPowers.popPrefEnv();
> +    SpecialPowers.clearUserPref("signon.autofillForms.http");
> +    SpecialPowers.clearUserPref("security.insecure_field_warning.contextual.enabled");

The reason this failed is because pushPrefEnv is finicky about other code touching the prefs it's controlling and the popPrefEnv() line above is supposed to do this for you. Removing these two lines makes the tests pass with and without e10s for me.
Attachment #8821699 - Flags: review?(MattN+bmo) → review+
Pushing to inbound now with the two lines removed as the following are passing locally without them:
> ./mach mochitest toolkit/components/passwordmgr/test/ --disable-e10s
> ./mach mochitest toolkit/components/passwordmgr/test/
Flags: needinfo?(MattN+bmo)
After bug 1286312 lands we can revert the test changes and make affected tests https except for ones testing the warning.

Comment 37

5 months ago
Pushed by mozilla@noorenberghe.ca:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f0d146fe7317
Flip a pref to disable autofilling saved password on HTTP pages. Flip a pref to enable showing insecure password warnings in the password field autocomplete drop down. r=MattN
https://hg.mozilla.org/integration/mozilla-inbound/rev/bc1c406e17c8
disable insecure password warning and enable autofill on HTTP pages for password manager tests using pwmgr_common.js. r=MattN
(In reply to Adrian Florinescu [:AdrianSV] [No PTO left this year - NI me if required] from comment #33)
> (In reply to Matthew N. [:MattN] (PM me if requests are blocking you) from
> comment #23)
> > [Tracking Requested - why for this release]: Enable the new feature
> > 
> > Adrian, can you do a QE pass on the whole insecure field contextual warning
> > and login manager autocomplete changes when this lands or by flipping the
> > two prefs yourself.
> > 
> > Known bug dependencies:
> > https://mnoorenberghe.github.io/bz-dependency-buglist/?list=1304224
> > Tracked work:
> > https://docs.google.com/spreadsheets/d/
> > 155k3Ja06QVgRj9F_DnAJxC8wfH_lWM4N9eZal4llefk/edit#gid=1492590763
> 
> Hello Matt, few questions below:
> 
> 1. Are the preferences in discussion:
> (a)"security.insecure_field_warning.contextual.enabled" and
> (b)"signon.autofillForms.http"? Their defaults today (12.27) in Nightly53
> are: (a) - false and (b)- true.
> The defaults when this lands will be a-true and b -false as the title of
> this issue suggests?

Yes, correct. See https://hg.mozilla.org/integration/mozilla-inbound/rev/f0d146fe7317

> 2. I'm not sure what exactly you mean by the "login manager autocomplete
> changes" - could you specify a list of (meta)bugs that cover this? Would
> this also cover the email tread with the password autocomplete? guessing
> this would be it bug 1318203?

I mean refactorings like bug 1296638 and bug 1294502 (plus follow-ups) that this work built on top of. I would recommend getting ideas from the spreadsheet and I've added these two bugs there: https://docs.google.com/spreadsheets/d/155k3Ja06QVgRj9F_DnAJxC8wfH_lWM4N9eZal4llefk/edit#gid=1492590763

Yes, it's related to the firefox-dev thread about password autocomplete so anything related to autocomplete of usernames, passwords or even non-login data (e.g. using datalist or regular form history) were affected. 

> 3. I see that the tracking is set for 52/53, so I'll treat this work as a
> bug verification work(exploratory testing and bug fix verification), but I'd
> prefer if the time allows to treat it as a fully fledged feature (this means
> test plan, test casing, etc). So, please let me know how are we faring on
> the time chapter. Meanwhile, I'll go with the 1st version - bug verification
> work(exploratory testing and bug fix verification).

Yeah, I think if you have time it would be good to treat it as a feature affecting autocomplete (and autofill on insecure pages). There are only a few more bugs to land and uplift in the spreadsheet so 52 is still attainable IMO.
Attachment #8821714 - Attachment is obsolete: true
Comment on attachment 8821698 [details] [diff] [review]
Bug1217152-12-23-16.patch

Approval Request Comment
[Feature/Bug causing the regression]: Insecure login field contextual warning feature
[User impact if declined]: Users won't be warned in a login field when it's insecure
[Is this code covered by automated tests?]: Yes, mochitests and unit tests
[Has the fix been verified in Nightly?]: Adrian is working on verification. 
[Needs manual test from QE? If yes, steps to reproduce]: See above
[List of other uplifts needed for the feature/fix]: Depends on bug 1324918 getting uplifted to avoid issues on Linux
[Is the change risky?]: Not really
[Why is the change risky/not risky?]: It's easy to pref off again and riskier dependency parts have been already riding the trains.
[String changes made/needed]: None
Attachment #8821698 - Flags: approval-mozilla-aurora?
Comment on attachment 8821699 [details] [diff] [review]
Bug1217152-pwmgrcommon-tests.patch

Approval Request Comment: See above
Attachment #8821699 - Flags: approval-mozilla-aurora?

Comment 41

5 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/f0d146fe7317
https://hg.mozilla.org/mozilla-central/rev/bc1c406e17c8
Status: ASSIGNED → RESOLVED
Last Resolved: 5 months ago
status-firefox53: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
(Assignee)

Comment 42

5 months ago
Created attachment 8823490 [details] [diff] [review]
Bug1217152-pwmgrcommon-tests2.patch

Updating patch so the right one is in the bug for aurora uplift.
Attachment #8821699 - Attachment is obsolete: true
Attachment #8821699 - Flags: approval-mozilla-aurora?
Attachment #8823490 - Flags: review+
(Assignee)

Comment 43

5 months ago
Comment on attachment 8823490 [details] [diff] [review]
Bug1217152-pwmgrcommon-tests2.patch

Approval Request Comment

See comment 39
Attachment #8823490 - Flags: approval-mozilla-aurora?
Comment on attachment 8823490 [details] [diff] [review]
Bug1217152-pwmgrcommon-tests2.patch

disable password autofill on http for aurora52
Attachment #8823490 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 8821698 [details] [diff] [review]
Bug1217152-12-23-16.patch

disable password autofill on http for aurora52
Attachment #8821698 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/872f6e1948f5
https://hg.mozilla.org/releases/mozilla-aurora/rev/a51cdeb88cbf
status-firefox52: affected → fixed
Don't forget that there is a contextual menu for autofill as well. Currently this says (No Login Suggestions), but I think it should show the list of what's saved.
Flags: needinfo?(tanvi)
tracking-firefox52: ? → -
Matt, 27-30 december last year, I’ve explored around on Nightly by switching the two preferences as discussed in comment 33 point 1. The behavior I got then was that any HTTP login would not offer at all any auto-fill/autocomplete/auto-suggest options for the saved users/passwords (behaviorA).  
If I redo the same actions today, (for HTTP) the autocomplete/auto-suggest and also the context menu will show my usernames/passwords as saved in the password manager(behaviorB). Going back to the 27-29 dec. builds, I get no trace of the behavior A and everything works as BehaviorB. I think we can exclude the example sites, machines and OS’es since I used more than one. 

Other than asking if you have any thought of how the above might be possible, I’d like to confirm that today’s behavior (B) is the correct and expected result of Password manager:

- turning off “signon.autofillForms.http” - automatic autofill for http will be disabled, while manual autofill is still enabled 
Automatic autofill = user,password are filled in on page load (as the current https behaves)
Manual autofill = user, password will be filled on user action (clicking on username will trigger the username list to choose from, etc).

(In reply to Ryan Feeley [:rfeeley] from comment #47)
> Don't forget that there is a contextual menu for autofill as well. Currently
> this says (No Login Suggestions), but I think it should show the list of
> what's saved.
The context menu today (behaviorB) shows the login suggestions (for password as well). Though I remember it did show "No Login Suggestions" on behaviorA. Ryan, could you share the http login you've seen this result on?
Flags: needinfo?(MattN+bmo)
(Assignee)

Comment 49

5 months ago
(In reply to Ryan Feeley [:rfeeley] from comment #47)
> Don't forget that there is a contextual menu for autofill as well. Currently
> this says (No Login Suggestions), but I think it should show the list of
> what's saved.

Hey Ryan, I'm not sure what you are referring to.  Can you elaborate?
Flags: needinfo?(rfeeley)

Updated

5 months ago
Depends on: 1329356
(In reply to Adrian Florinescu [:AdrianSV] from comment #48)
> Matt, 27-30 december last year, I’ve explored around on Nightly by switching
> the two preferences as discussed in comment 33 point 1. The behavior I got
> then was that any HTTP login would not offer at all any
> auto-fill/autocomplete/auto-suggest options for the saved users/passwords
> (behaviorA).  
> If I redo the same actions today, (for HTTP) the autocomplete/auto-suggest
> and also the context menu will show my usernames/passwords as saved in the
> password manager(behaviorB). Going back to the 27-29 dec. builds, I get no
> trace of the behavior A and everything works as BehaviorB. I think we can
> exclude the example sites, machines and OS’es since I used more than one. 

I don't really know what to say about the cause but behaviourA would have been a bug. It would have been useful to have password manager debug logging from when that happened: https://wiki.mozilla.org/Toolkit:Password_Manager/Debugging

> Other than asking if you have any thought of how the above might be
> possible, I’d like to confirm that today’s behavior (B) is the correct and
> expected result of Password manager:
> 
> - turning off “signon.autofillForms.http” - automatic autofill for http will
> be disabled, while manual autofill is still enabled 
> Automatic autofill = user,password are filled in on page load (as the
> current https behaves)
> Manual autofill = user, password will be filled on user action (clicking on
> username will trigger the username list to choose from, etc).

BehaviourB is what is expected and it sounds like what you're saying above matches that.
Flags: needinfo?(MattN+bmo)
Depends on: 1330561
(In reply to Tanvi Vyas - behind on bugmail [:tanvi] from comment #49)
> (In reply to Ryan Feeley [:rfeeley] from comment #47)
> > Don't forget that there is a contextual menu for autofill as well. Currently
> > this says (No Login Suggestions), but I think it should show the list of
> > what's saved.
> 
> Hey Ryan, I'm not sure what you are referring to.  Can you elaborate?

When you right-click the username/password field, you can also fill that way. Not as discoverable as our new UI, but still needs to reflect state accurately.
Flags: needinfo?(rfeeley)
Depends on: 1330953

Updated

4 months ago
Keywords: dev-doc-needed, site-compat, user-doc-needed
Depends on: 1331617
Depends on: 1331655
Depends on: 1331926
Depends on: 1331934
Depends on: 1331959
Depends on: 1331979
Depends on: 1332306
Depends on: 1332342
Posted the site compatibility doc for web developers: https://www.fxsitecompat.com/en-CA/docs/2017/insecure-login-forms-now-disable-autofill-show-warning-beneath-input-control/ (not actually a compatibility stuff though)
Depends on: 1332618
No longer depends on: 1331979
Flags: needinfo?(adrian.florinescu)

Updated

4 months ago
Depends on: 1338167
No longer depends on: 1338167
Reproduced the issue on 53.0a1 20161227030213.
I can confirm the bug is fixed on 53.0a2 (2017-02-27) and 52.0b9 (20170223185858), using Windows 10 x64, Ubuntu 16.04 x64 and Mac OS X 10.11.6.
Setting the corresponding flags.
Status: RESOLVED → VERIFIED
status-firefox52: fixed → verified
status-firefox53: fixed → verified
Flags: qe-verify+
Documented; see details at 

https://bugzilla.mozilla.org/show_bug.cgi?id=1319119#c17
Keywords: dev-doc-needed → dev-doc-complete
You need to log in before you can comment on or make changes to this bug.