Closed
Bug 362576
Opened 18 years ago
Closed 17 years ago
autocomplete="off" should prevent filling passwords in addition to remembering passwords
Categories
(Toolkit :: Password Manager, defect)
Toolkit
Password Manager
Tracking
()
VERIFIED
FIXED
mozilla1.9
People
(Reporter: BijuMailList, Assigned: Dolske)
References
()
Details
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
Gavin
:
review+
mconnor
:
ui-review+
mconnor
:
approval1.9+
|
Details | Diff | Splinter Review |
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•18 years ago
|
||
This would break the "remember password" bookmarklet.
Summary: autocomplete behavior is not the same way as IE → autocomplete="off" should prevent filling passwords in addition to remembering passwords
(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.
>
> 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•18 years ago
|
||
To make Mike happy :)
Biju, good work finding this bug, congrats.
Comment 5•18 years ago
|
||
No need for a Javascript hack. Look at this (disclaimer: my personal site):
http://dotancohen.com/howto/firefox_password_manager.php
Assignee | ||
Updated•17 years ago
|
Assignee: nobody → dolske
OS: Windows XP → All
Hardware: PC → All
Target Milestone: --- → Firefox 3 M8
Assignee | ||
Comment 6•17 years ago
|
||
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. :-)
Depends on: 389185
Target Milestone: Firefox 3 M8 → Firefox 3 M9
Comment 7•17 years ago
|
||
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•17 years ago
|
||
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.
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•17 years ago
|
||
(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•17 years ago
|
||
(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•17 years ago
|
||
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
Assignee | ||
Updated•17 years ago
|
Assignee | ||
Comment 13•17 years ago
|
||
Bumping to M10, I don't want to put this in M9.
Target Milestone: Firefox 3 M9 → Firefox 3 M10
Comment 14•17 years ago
|
||
(In reply to comment #13)
> Bumping to M10, I don't want to put this in M9.
>
What is the planned "fix" for this?
Assignee | ||
Updated•17 years ago
|
Assignee: dolske → nobody
Version: 2.0 Branch → Trunk
Comment 15•17 years ago
|
||
> 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.
Assignee | ||
Updated•17 years ago
|
Target Milestone: Firefox 3 beta2 → ---
Comment 18•17 years ago
|
||
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•17 years ago
|
||
@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•17 years ago
|
||
@Dotan: We are. FF3 (beta 5) doesn't seem to care, see bug 364970 and possibly others.
Comment 21•17 years ago
|
||
(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•17 years ago
|
||
(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•17 years ago
|
||
@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•17 years ago
|
||
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•17 years ago
|
||
(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•17 years ago
|
||
(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•17 years ago
|
||
@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•17 years ago
|
||
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.
Assignee | ||
Comment 29•17 years ago
|
||
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.
Assignee | ||
Updated•17 years ago
|
Target Milestone: --- → Firefox 3
Assignee | ||
Comment 30•17 years ago
|
||
Attachment #319447 -
Attachment is obsolete: true
Attachment #319482 -
Flags: ui-review?(beltzner)
Attachment #319482 -
Flags: review?(gavin.sharp)
Comment 31•17 years ago
|
||
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.
Attachment #319482 -
Flags: review?(gavin.sharp) → review+
Comment 32•17 years ago
|
||
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.
Assignee | ||
Comment 33•17 years ago
|
||
(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•17 years ago
|
||
(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•17 years ago
|
||
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
Assignee | ||
Comment 36•17 years ago
|
||
(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•17 years ago
|
||
(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•17 years ago
|
||
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•17 years ago
|
||
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.
Attachment #319482 -
Flags: ui-review?(beltzner)
Attachment #319482 -
Flags: ui-review+
Attachment #319482 -
Flags: approval1.9+
Assignee | ||
Comment 40•17 years ago
|
||
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
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Comment 41•17 years ago
|
||
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!
Status: RESOLVED → VERIFIED
Comment 42•17 years ago
|
||
(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?
Updated•16 years ago
|
Product: Firefox → Toolkit
You need to log in
before you can comment on or make changes to this bug.
Description
•