Last Comment Bug 362576 - autocomplete="off" should prevent filling passwords in addition to remembering passwords
: autocomplete="off" should prevent filling passwords in addition to rememberin...
Status: VERIFIED FIXED
:
Product: Toolkit
Classification: Components
Component: Password Manager (show other bugs)
: Trunk
: All All
: -- normal with 5 votes (vote)
: mozilla1.9
Assigned To: Justin Dolske [:Dolske]
:
Mentors:
http://www.whatwg.org/specs/web-forms...
: 423966 424268 (view as bug list)
Depends on: 389185 433238 434094
Blocks: 368959 373140 443183
  Show dependency treegraph
 
Reported: 2006-12-01 23:59 PST by Biju
Modified: 2012-10-12 05:21 PDT (History)
17 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
First Test Case (1.23 KB, text/html)
2007-02-03 23:03 PST, Robert Chapin
no flags Details
Patch v.1 (work in progress) (4.41 KB, patch)
2008-05-05 13:00 PDT, Justin Dolske [:Dolske]
no flags Details | Diff | Splinter Review
Patch v.2 (14.78 KB, patch)
2008-05-05 17:00 PDT, Justin Dolske [:Dolske]
gavin.sharp: review+
mconnor: ui‑review+
mconnor: approval1.9+
Details | Diff | Splinter Review

Description Biju 2006-12-01 23:59:24 PST
Per 360493 comment #187

If there is autocomplete="off" for a form,
it may not suppress saved password auto filled all the time.

Steps:-
* a site had login form at http://mysite.com/login.html without autocomplete="off" 

* While logging-in User_A saved his password when prompted by PM.

* this setting went for days, and User_A use to get his password auto filled in.

* one day developer at mysite.com learned about autocomplete attr.

* he modified login form to autocomplete="off"

* but User_A will still get password autofilled !!!!

PS: 
for future User_X, PM wont be prompted to save password.
as well as if User_A and User_B have save password early 
firefox wont Auto-fillin
Comment 1 Jesse Ruderman 2006-12-02 02:45:34 PST
This would break the "remember password" bookmarklet.
Comment 2 Biju 2006-12-02 10:05:17 PST
(In reply to comment #1)
> This would break the "remember password" bookmarklet.

Sad... IMO bookmarklet should be always powerful than site authors code.

So this bug is fix should include an about:config option to revert to current way by advanced users.

Also we should decide on what to do with already saved password, remove it for PM?.
I dont like the Idea of keeping saved password in PM, without notifying user.

Comment 3 al_9x 2007-01-12 15:54:19 PST
> 
> So this bug is fix should include an about:config option to revert to current
> way by advanced users.
> 

no, the option should address the issue directly and allow advanced users to ignore autocomplete=off altogether as opposed to restoring current behavior which still requires a js hack.  
Comment 4 Robert Chapin 2007-02-03 23:03:10 PST
Created attachment 253928 [details]
First Test Case

To make Mike happy :)

Biju, good work finding this bug, congrats.
Comment 5 Dotan Cohen 2007-04-16 06:34:49 PDT
No need for a Javascript hack. Look at this (disclaimer: my personal site):
http://dotancohen.com/howto/firefox_password_manager.php
Comment 6 Justin Dolske [:Dolske] 2007-09-05 22:22:01 PDT
Deferring to M9, this can be handled as a bugfix but Jesse had some concerns about it, so crash landing it in M8 doesn't seem fair. May also depend on bug 389185, which also had some objections. :-)
Comment 7 Jesse Ruderman 2007-09-06 02:35:51 PDT
One idea is to remember whether autocomplete=off was present and overridden when the password was saved.  That would help in the bug 389185 case but not in the bookmarklet case.
Comment 8 Dotan Cohen 2007-09-06 04:03:20 PDT
Why do we even want this? The only reason that Firefox supports autocomplete=off was so that the banks would not block the Firefox UA. No end users want autocomplete=off support. Now that we have the banks' support, why make life even harder for end users. I move that this bug be closed as it makes Firefox less usable for end users, with no benefit.
Comment 9 TheSeer 2007-09-28 03:32:22 PDT
There are many very reasonable cases where autocomplete is a problem rather then help. Imagine a page to adjust personal data along with a password field. Firefox will automagically fill in the form fields, overriding stuff provided by the server (?), and breaking the password-change logic in a way the user as to CLEAR the password field manually to not update his password.

Having a server-provided "autocomplete=off" will ease the situation.
Comment 10 Dotan Cohen 2007-09-28 08:38:31 PDT
(In reply to comment #9)
> There are many very reasonable cases where autocomplete is a problem rather
> then help. Imagine a page to adjust personal data along with a password field.
> Firefox will automagically fill in the form fields, overriding stuff provided
> by the server (?), and breaking the password-change logic in a way the user as
> to CLEAR the password field manually to not update his password.
> 
> Having a server-provided "autocomplete=off" will ease the situation.
> 

This should be client side, then, not server-side. When a user wants this functionality, do you think that he will write to the webmaster to request it?!? No, let him enable it from within his browser. A "Do not auto-complete this field" option in the right-click menu would be fine in this case.
Comment 11 TheSeer 2007-09-28 09:22:11 PDT
(In reply to comment #10)
> (In reply to comment #9)
> > There are many very reasonable cases where autocomplete is a problem rather
> > then help. Imagine a page to adjust personal data along with a password field.
> > Firefox will automagically fill in the form fields, overriding stuff provided
> > by the server (?), and breaking the password-change logic in a way the user as
> > to CLEAR the password field manually to not update his password.
> > 
> > Having a server-provided "autocomplete=off" will ease the situation.
> > 
> 
> This should be client side, then, not server-side. When a user wants this
> functionality, do you think that he will write to the webmaster to request
> it?!? No, let him enable it from within his browser. A "Do not auto-complete
> this field" option in the right-click menu would be fine in this case.
> 

Well, that's where I disagree: The Webmaster should have the power to tell the browser how to behave - with the option of the user to OVERRIDE that decision. Not the other way around. Requiring a user to perform an action where, from a logical perspective of the website in question an autocomplete is not desired and thus disabled by the developer, is bad - imho. 

If the user wants to overrule the decision made by the webmaster, that's fine and firefox should support/offer a gui-way to do so. But the default behavior of the browser should be what the html-code suggests.

Comment 12 Dotan Cohen 2007-09-28 12:12:31 PDT
Actually, I don't really care what the default behaviour is as I'm not quite a defaut kind of guy. So long as the behaviour is configurable I'm game. That means, if "autocomplete=off" is specified then I want to be able to override it, even if I have to hack as described here:
http://dotancohen.com/howto/firefox_password_manager.php
Comment 13 Justin Dolske [:Dolske] 2007-10-02 11:15:33 PDT
Bumping to M10, I don't want to put this in M9.
Comment 14 al_9x 2007-10-03 09:17:02 PDT
(In reply to comment #13)
> Bumping to M10, I don't want to put this in M9.
> 

What is the planned "fix" for this?
Comment 15 Dotan Cohen 2008-01-30 01:05:39 PST
> What is the planned "fix" for this?

The fix should be to mark the bug WONTFIX. There is nothing broken to fix, quite the opposite, the bug is suggesting that we deliberately break something.
Comment 16 Justin Dolske [:Dolske] 2008-03-19 15:58:07 PDT
*** Bug 423966 has been marked as a duplicate of this bug. ***
Comment 17 Justin Dolske [:Dolske] 2008-03-21 00:00:25 PDT
*** Bug 424268 has been marked as a duplicate of this bug. ***
Comment 18 Chris Boyle 2008-04-11 06:37:11 PDT
This is particularly obnoxious in combination with bug 364970 in FF3. I see that is WONTFIX so I will say this here: it is now impossible to use input type="password" for anything other than a login form, or to have more than one password field anywhere on the same hostname for different purposes.

The change password use case has already been mentioned. As another example, on some settings pages of my employer's main product, Zeus Extensible Traffic Manager, one user can set the password of another, or a password the device will use to authenticate to a hardware crypto device. This is done using input type="password" autocomplete="off" value="some value", but it is filled in with the (unrelated) password to log in to the UI. Worse, the previous text field, which is some way up the page and unrelated, is silently overwritten with the username.

There is currently nothing I can put in such pages to prevent that behaviour. We and other vendors require a way for the user to enter a password for a purpose other than logging in, and if Firefox 3 is released with its current autofill behaviour, what can we do except tell our customers that we are unable to support Firefox 3?
Comment 19 Dotan Cohen 2008-04-11 08:45:38 PDT
@chris: You should be using unique names for the individual password fields.

<form action="login.cgi">
<input type="password" name="loginPassword" />
</form>

<form action="changepassword.cgi">
<input type="password" name="oldPassword" />
<input type="password" name="newPassword" />
<input type="password" name="newPasswordVerify" />
</form>
Comment 20 Chris Boyle 2008-04-11 08:49:33 PDT
@Dotan: We are. FF3 (beta 5) doesn't seem to care, see bug 364970 and possibly others.
Comment 21 Frédéric Buclin 2008-04-11 09:48:18 PDT
(In reply to comment #19)
> @chris: You should be using unique names for the individual password fields.

No, this doesn't work in Firefox 3 anymore. Firefox 2 was looking at the name of the field to decide if it has to fill it or not. Firefox 3 doesn't care about the field name anymore and will fill it with the stored password in all cases. This also breaks Bugzilla, which is another example of an application concerned by this non-sense change in Firefox 3.

I'm honestly wondering if MoCo guys thought about this problem a bit or not when deciding this change.
Comment 22 Chris Cooper [:coop] 2008-04-11 09:52:10 PDT
(In reply to comment #21)
> I'm honestly wondering if MoCo guys thought about this problem a bit or not
> when deciding this change.

FWIW, I'm a MoCo guy and this bug is biting me too.
Comment 23 Dotan Cohen 2008-04-11 09:54:53 PDT
@Chris:
I see that the problem is that Firefox does not store the name of the fields. Autocomplete="off" would be a workaround, not a solution, to that problem. I will collect real-world use cases and file a descriptive bug report with a proposed solution. Please post the public URL of your application. Thanks.
Comment 24 Dotan Cohen 2008-04-11 10:00:01 PDT
In any case, this bug is not the correct place to discuss this. Please do not hijack this bug, and post all comments in the proper bug.
Comment 25 Chris Boyle 2008-04-11 10:04:28 PDT
(In reply to comment #23)
> Please post the public URL of your application. Thanks.

The application in question is the web UI for a traffic manager, http://www.zeus.com/products/zxtm/ . There isn't a public instance available; you can download a VM if you really want at http://www.zeus.com/downloads/evaluation/desktop/your-details/id/zxtm

Problematic areas (areas where a password field is used other than for logging in) include System -> Users (one may edit a user other than oneself) and System -> Global Settings -> SSL Hardware.
Comment 26 Frédéric Buclin 2008-04-11 10:06:44 PDT
(In reply to comment #24)
> In any case, this bug is not the correct place to discuss this. Please do not
> hijack this bug, and post all comments in the proper bug.

This bug *is* the correct place to discuss this. All other related bugs have been marked as dupes of this one. There is no need to file another bug about this topic.
Comment 27 Chris Cooper [:coop] 2008-04-11 10:13:45 PDT
@Dotan: bug 423966 got duped to this one.

The app in question is https://litmus.mozilla.org/

...and is available from cvs as mozilla/webtools/litmus.

You'll see the workaround here:

http://mxr.mozilla.org/mozilla/source/webtools/litmus/templates/en/default/auth/login.html.tmpl#179
Comment 28 Glenn Linderman 2008-04-17 12:06:35 PDT
Banks have legitimate, and legally required, security concerns, and Firefox should support that.

Users that have adequate physical and internet security for their machines, should also have the right to choose to have Firefox remember their passwords, and override the autocomplete=off issue.

Even if the users physical and internet security is not sufficient, they should have the ability to choose whether or not to take the risk of having Firefox remember their passwords.

Presently there is a bookmarklet that zaps the "autocomplete=off" stuff, it is annoying to have to use it, but at least it works, for most sites.  Some sites outsmart the bookmarklet through the use of complex javascript.  So far, this simply causes me to give my business to banks whose sites don't outsmart the bookmarklet.
Comment 29 Justin Dolske [:Dolske] 2008-05-05 13:00:01 PDT
Created attachment 319447 [details] [diff] [review]
Patch v.1 (work in progress)

I think this bug needs to be fixed, because a some recent bugs have shown that's there's a relatively common issue with this. The current problem is that the new password manager ignores form field names -- deliberately, because some sites were using different field names and accidentally breaking saving logins. However, this means sites don't have a way to prevent filling in logins where they really don't belong. The typical example seems to be:

-----
<h1>Welcome Admin! Adding a new user...</h1>
New Username:     <input name="user">
Initial password: <input name="pass" type="password">
-----

Filling in the admin's login here is unwanted, and sites should have the ability to prevent it from happening.

What this patch does:

When autocomplete=off is found when loading a page, the login is no longer automatically filled in. However, the autocomplete handler is still attached to the username field, so users can still manually select their login from the field's dropdown if they want to. This is what we already do when multiple logins are present -- we don't know which one to fill, so we fill nothing and attach the autocomplete handler to allow the user to choose from the dropdown.
Comment 30 Justin Dolske [:Dolske] 2008-05-05 17:00:49 PDT
Created attachment 319482 [details] [diff] [review]
Patch v.2
Comment 31 :Gavin Sharp [email: gavin@gavinsharp.com] 2008-05-06 18:39:22 PDT
Comment on attachment 319482 [details] [diff] [review]
Patch v.2

>Index: toolkit/components/passwordmgr/src/nsLoginManager.js

>+    _isAutocompleteDisabled :  function (element) {
>+        if (element && element.hasAttribute("autocomplete") &&

nit: this was there before, but getAttribute already does a hasAttribute check, so it isn't necessary.

r=me code wise, but I'm a bit concerned about making this behavior change. It makes things more annoying for users who've used bookmarklets to force the password to be saved, or have passwords saved from before autocomplete=off was added, and the password manager dropdown isn't exactly easy to discover. Not sure how that compares to the annoyance of having fields filled in when they shouldn't be, considering that for this to help there the webmaster needs to be aware - is it common for pages to already have autocomplete=off for these kind of forms? Could we make this behavior conditional on the field names not matching, as well? I thought that's what you suggested when we discussed it on IRC.
Comment 32 Glenn Linderman 2008-05-06 18:54:17 PDT
It is easy to tab to an erroneously filled in field, and put in the right data... likely would have to do that anyway, if it hadn't been erroneously filled in.

It is easy to tab to or click on the blank field and fill in it, but if it had been filled in, that would have been unnecessary.

So my vote is to fill in the fields.
Comment 33 Justin Dolske [:Dolske] 2008-05-06 21:23:27 PDT
(In reply to comment #31)

> r=me code wise, but I'm a bit concerned about making this behavior change. It
> makes things more annoying

True, although it's no more annoying than we currently have with multiple logins (which people commonly hit on sites like Gmail). That's a known problem [c.f. bug 376668, bug 327044, bug 222653], but I'm optimistic that we can improve the UI for that case. And by reducing the autocomplete=off issue to that problem, such an improvement will kill 2 birds with one stone. (Although that's obviously not going to happen for Firefox 3.)

> the password manager dropdown isn't exactly easy to discover.

From a blank field it's not obvious that down-arrow/clicking will show the autocomplete-dropdown, but once you start typing in your username the dropdown is shown with matching logins. It's the same principle as form autocomplete and awesomebar, so I'm not *too* worried about discoverability.

> considering that for this to help there the webmaster needs to be
> aware - is it common for pages to already have autocomplete=off for these kind
> of forms?

Well, the reason for fixing this is that it provides a familiar workaround to sites that encounter this problem (which hopefully isn't often). I've been reluctant to fix this to date, because I didn't want to cripple login manager *or* stop users from using their bookmarklet-saved logins. This seemed like a reasonable compromise that still enables both cases.

> Could we make this behavior conditional on the field names not
> matching, as well? I thought that's what you suggested when we discussed it on
> IRC.

I wasn't entirely pleased with that idea (even though it was mine :), because it was kind of hackish. And it would have been confusing to explain that field names don't matter, except for when they do. :/
Comment 34 :Gavin Sharp [email: gavin@gavinsharp.com] 2008-05-06 21:36:12 PDT
(In reply to comment #33)
> I wasn't entirely pleased with that idea (even though it was mine :), because
> it was kind of hackish. And it would have been confusing to explain that field
> names don't matter, except for when they do. :/

Well, the names currently matter in Firefox 2, right? We'd be making them matter slightly less, but I'm not really worried about confusion about the underlying implementation if we get all the cases that users will hit right, and I think using the field names this way is probably the closest we can get at this point.
Comment 35 Dotan Cohen 2008-05-07 00:50:14 PDT
The field names are important so the the password manager can work with some banks that require three fields to be filled in instead of two. Here is such an example:
https://login.bankhapoalim.co.il/cgi-bin/poalwwwc?dt=924&nls=EN
Comment 36 Justin Dolske [:Dolske] 2008-05-07 12:17:55 PDT
(In reply to comment #34)

> Well, the names currently matter in Firefox 2, right? We'd be making them
> matter slightly less, but I'm not really worried about confusion about the
> underlying implementation

I'd rather have a explicit mechanism for sites to use, instead of relying on details of the implementation. Using field names to identify what field to [use | not use] is underspecified and wishy-washy, but autocomplete=off isn't.

(In reply to comment #35)
> The field names are important so the the password manager can work with some
> banks that require three fields to be filled in instead of two.

Not really relevant, because a password manager login only consists of 1 username and 1 password. And there's no way to control what it remembered in the first place, other than field order.
Comment 37 Dotan Cohen 2008-05-07 16:08:00 PDT
(In reply to comment #36)
> (In reply to comment #35)
> > The field names are important so the the password manager can work with some
> > banks that require three fields to be filled in instead of two.
> 
> Not really relevant, because a password manager login only consists of 1
> username and 1 password. And there's no way to control what it remembered in
> the first place, other than field order.
> 

Then the password manager is inadequete for cases like this. I will file a new bug.
Comment 38 Dotan Cohen 2008-05-08 08:23:28 PDT
A commenter on this bug emailed me requesting that I send to him the bug number for the issue mentioned in Comment #35. I lost the email (1 year old daughter + keyboard). So here it is:
Bug# 432824 – Password Manager cannot remember three fields for login
Comment 39 Mike Connor [:mconnor] 2008-05-08 20:27:12 PDT
Comment on attachment 319482 [details] [diff] [review]
Patch v.2

Ok, dolske convinced me over IRC that this is a problem we should fix before ship.

Short version is that by making the pw filling more tolerant of field changes, and fixing cases where forms wouldn't fill previously, we now fill in where we didn't before, and there's no easy way to prevent this.  Think we've hit this on bugzilla and litmus in our own tools, and there's been a growing number of reports.  We don't want to make the pwmgr less resilient, so we need to provide an explicit way for sites to prevent this behaviour.
Comment 40 Justin Dolske [:Dolske] 2008-05-08 20:35:22 PDT
Checking in toolkit/components/passwordmgr/src/nsLoginManager.js;
  new revision: 1.31; previous revision: 1.30
Checking in toolkit/components/passwordmgr/test/test_basic_form_autocomplete.html;
  new revision: 1.7; previous revision: 1.6
Checking in toolkit/components/passwordmgr/test/test_bug_227640.html;
  new revision: 1.6; previous revision: 1.5
Checking in toolkit/components/satchel/src/nsFormFillController.cpp;
  new revision: 1.101; previous revision: 1.100
Comment 41 Frédéric Buclin 2008-05-13 03:49:59 PDT
Verified fixed with Mozilla/5.0 (X11; U; Linux i686; fr; rv:1.9) Gecko/2008051202 Firefox/3.0. Fx3 no longer fills the name and password fields when editing a user account from Bugzilla (it was previously filling these fields with my own credentials, overwriting the other user's own password). Yay!
Comment 42 al_9x 2008-05-16 13:49:19 PDT
(In reply to comment #39)

> so we need to provide
> an explicit way for sites to prevent this behaviour.
> 

1) This change broke password only forms where username and password are on different pages, this is used by several banks (Bug 434094)

<form><input type='password' autocomplete="off" name='password1'></form>

the autocomplete handler(for manually selecting saved credentials) is only attached to the username field, which is absent.

2) there needs to be an option to ignore aautocomplete="off" when saving credentials and now when filling them out

what is the objection to letting the user control his credentials?


Note You need to log in before you can comment on or make changes to this bug.