User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7) Gecko/20040803 Firefox/0.9.3 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7) Gecko/20040803 Firefox/0.9.3 vBulletin is a popular message board system. Unfortunately, username and password used in such a message board are not saved by the password manager of Mozilla Firefox. Reproducible: Always Steps to Reproduce: 1. http://www.flightforum.ch/forum/ (a Swiss-German message board on aviation) 2. Login with username (Benutzername) and password (Kennwort) in the upper right corner Actual Results: Login is successful, however, Mozilla Firefox does NOT pop up the password manager window asking whether you want to save username and password Expected Results: Mozilla Firefox should pop up the standard password manager window asking whether you want to save username and password

WFM Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7) Gecko/20040803 Firefox/0.9.3

(In reply to comment #1) > WFM You're right. I've just noticed that username and password are saved (including the password manager popup window), however, they are not remembered. After having logged out, I tried to login again and I had to enter the password by myself instead of getting it from the password manager. Can you confirm this problem?

Mozilla is working as intended. vBulletin 3 uses clientside md5 prior attached to the onsubmit event. It hashes the value in the password field and stores it in a hidden variable and then clears the password field so the password manager never picks it up.

*** Bug 268716 has been marked as a duplicate of this bug. ***

I'd buy this as a wontfix|cantfix, i.e. "we can't change to pick up those without breaking this other thing" but it doesn't strike me as "it's working just the way it should" invalid. The only bad thing I can think of happening from having it remembered at the start of the onSubmit chain rather than at the end is that it would offer to remember things that would then be rejected by client-side validation, but I've only seen that in signups where it's not likely to be remembering anyway. Add in Live Journal, which uses the same login scheme, and there's several million more users, I think something like 7 million active.

*** Bug 271494 has been marked as a duplicate of this bug. ***

The point of the password manager is to emulate a user typing in the details. So it really shouldn't worry about onsubmit or any Javascript. It should just check what's in the password box when the button is clicked and save it. Then it can let the form continue with the submission. Perhaps the summary for this bug should be changed to something more general like 'password manager should save passwords before javascript events' Just my $.02

Submit this form and agree to save the password. If you look in your saved passwords, the username is there, but the password is empty. It was cleared by an onsubmit event attached to the form.

You can get FF to remember the password by typing something like: javascript:alert(document.forms[1].submit()); in the URL bar. Firefox will then ask you if you want to save the password. However, this only works the first time: the next time you log in, Firefox silently replaces the password with the empty password in the submit event. Therefore, to fix this I would propose: 1. Fixing the password manager code so that it does not overwrite saved passwords with empty passwords. Anyone know which files I have to look at? 2a. Changing the Firefox context menu on password fields from "Add a Keyword for this search" to "Remember this Password" OR 2b. Creating an extension that adds "Remember this Password" to the context menu for password fields.

*** Bug 291116 has been marked as a duplicate of this bug. ***

*** Bug 293211 has been marked as a duplicate of this bug. ***

I described in bug 293211 that a bookmarklet which neuters the onsubmit from the form is sufficient to have Firefox prompt you about saving your password. However, you must continue to run the bookmarklet EVERY time you log in, or Firefox will oh-so-helpfully update the password to null. (silently! as I ranted in the aforemented bug)

A workaround for this behavior is to disable javascript before entering the password. Seamonkey (and probably all mozilla-based browser) ask you if you want to save the password. After a click on yes you can enable javascript again and the password box will be prefilled like expected. Bruno

*** Bug 299497 has been marked as a duplicate of this bug. ***

This problem also occurs with Simple Machine Forums.

Simple Machines WFM: http://support.simplemachines.org/demo/index.php test/test, prompted to remember, remembers, and has no script hashing and removing the password before submitting (and if it doesn't do that, no matter whether or not it works it isn't this bug).

(In reply to comment #16) > Simple Machines WFM: http://support.simplemachines.org/demo/index.php test/test, > prompted to remember, remembers, and has no script hashing and removing the > password before submitting (and if it doesn't do that, no matter whether or not > it works it isn't this bug). That's a demo of SMF 1.0.5. This happens only in SMF 1.1 versions (which do onsubmit password hashing, using SHA-1.) I understand this is similar to the way vBulletin and LiveJournal do challenge authentication. Opera actually seems to handle it properly (saving the password before firing onsubmit events), but Internet Explorer does not. -[Unknown]

*** Bug 235765 has been marked as a duplicate of this bug. ***

Martin: Would you mind if the summary changed to "vBulletin javascript causes wrong password value to be remembered" It seems like the password manager does remember the username, but the password is blank because vB clears the password for the md5. Checked with Firefox 1.0.7 and Firefox 1.4 20050928 build (Mac). Perhaps the password manager should try to catch the values before onsubmit because otherwise forms that modify text/password input fields cause the password manager to store the new value and not what the user typed in. A couple possible problems that this currently causes: - Passwords blanked out before submitting (with a md5-pass hidden field) store a blank password - Saved modified password will be loaded upon next visit and onsubmit preprocessing will be triggered again resulting in a wrong password

*** Bug 315815 has been marked as a duplicate of this bug. ***

I read a lot of posts saying that Firefox shows the prompt to save, but it will not work (blank password). This is true with Firefox 1.0, but with Firefox 1.5 it doesn't even show the prompt!

(In reply to comment #19) I am having a problem in FF 1.5 with the same symptoms: login is remembered but password isn't (blank). We have heavy JS on the page and in the form + two password fields with different names. Once you remove the second password field or set it to type="text", FF starts remembering the password as well. The interesting thing is that it did work with both password fields at some point, but I can't reproduce it. Any thoughts?

In the latest nightlys it is still the case that there is no longer a prompt to remember username and password at all. It seems that the password manage is simply broken when it comes to vBulletin sites.

We should try to hook into the password before the submit event gets fired, yes, since in theory when we fill the password later, the same client-side stuff gets run.

(In reply to comment #24) > We should try to hook into the password before the submit event gets fired, > yes, since in theory when we fill the password later, the same client-side > stuff gets run. > That is assuming we're dealing with a submit originating from a button or enter. If there is a custom made button or some other way of submitting the form, it is possible that the script will butcher the password and then call submit(). Hooking in the password manager before the submit event won't do anything because the javascript on the page has already butchered the password before calling submit() itself.

This attempts to call the password manager earlier when widgets call OnSubmitClickBegin. Both input and button call OnSubmitClickBegin early enough to do the job. Modifying OnSubmitClickBegin also allows us to keep the early password manager notify changes in one file.

Comment on attachment 224503 [details] [diff] [review] Notify password manager about form earlier I'd really prefer, if possible, that someone more familiar than I with this code review this change. I guess jkeiser is not around, so I'd check the blame for who added the code and who reviewed? I'd guess jst and maybe bryner, but check the blame. That said, I'd maybe name "mEarlyNotify" something more like "mNotifiedObservers". I'm also concerned that this will miss whole classes of form submissions (eg submitting via hitting enter in a textfield on a form with no submit button or with just one textfield). Wouldn't it make more sense to notify right before firing the onsubmit event? Or are these sites doing weird stuff before that event even fires?

(In reply to comment #27) > (From update of attachment 224503 [details] [diff] [review] [edit]) > I'd really prefer, if possible, that someone more familiar than I with this > code review this change. I guess jkeiser is not around, so I'd check the blame > for who added the code and who reviewed? I'd guess jst and maybe bryner, but > check the blame. > I originally asked jst for a review, but it seems that he's gonna be out, so I picked someone else from the cvs logs. > That said, I'd maybe name "mEarlyNotify" something more like > "mNotifiedObservers". > Sure. > I'm also concerned that this will miss whole classes of form submissions (eg > submitting via hitting enter in a textfield on a form with no submit button or > with just one textfield). > Hitting enter will work - we get the event before the page does. However, there is one class of form submission that this will miss, which I commented on earlier. A page can make a custom submit button which butchers the fields and calls submit() itself. This patch will only work for the cases where we submit via hitting enter or hitting a (real) submit button. If a page really wants to screw with the password manager, it can. > Wouldn't it make more sense to notify right before firing the onsubmit event? > Or are these sites doing weird stuff before that event even fires? > No big reason behind putting the early notify code where it is now. It's just that I get to put in the early notify changes in one file instead of three this way. Also, this file has a helper for notifying the password manager so I don't need to copy that helper code to the other files.

> Hitting enter will work - we get the event before the page does. We do? Why is that case different from the button-click case? > A page can make a custom submit button Yeah, I don't think we can really do much about that case. ;) You want reviewers from the blame for the code you'r changing, not just from the CVS logs for the files, basically. I know things about parts of these files, but not other parts...

Comment on attachment 224503 [details] [diff] [review] Notify password manager about form earlier - In nsHTMLFormElement::OnSubmitClickBegin(): + PRBool aCancelSubmit = PR_FALSE; I know you're not the first one to do this in this file, but please don't name local variables _a_Something, that's the naming convention we use for function arguments, so name the above boolean cancelSubmit to avoid that confusion, and while you're at it, rename the same variable in nsHTMLFormElement::SubmitSubmission() as well. As for bz's concerns about whether this does the right thing if the user hits enter in a text field, that *will* trigger a synthesized left-click event, but that event is triggered after the enter key-up event is processed. So the synthetic click event is what triggers the submit, so that part should be fine as long as the page doesn't trigger any weird JS that laters the password etc from the enter key event(s). r=jst with the above naming change and bz's suggsted naming change. sicking should probably sr this one.

Name changes by bz and jst. Otherwise, basically identical to before.

Comment on attachment 226426 [details] [diff] [review] Notify password manager about form earlier, v2 >+ // >+ // Notify observers of submit >+ // >+ PRBool cancelSubmit = PR_FALSE; >+ rv = NotifySubmitObservers(actionURI, &cancelSubmit); >+ if (NS_SUCCEEDED(rv)) >+ mNotifiedObservers = PR_TRUE; >+ >+ mNotifiedObserversResult = cancelSubmit; Nit: Put last statement inside the |if| as well for clarity (since we'll ignore the value anyway).

Sicking's suggestion applied.

v3 checked in on trunk. Thanks for the reviews!

Verified FIXED using Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a1) Gecko/20060623 Minefield/3.0a1. The Save Password dialog now shows and when I choose "Remember Password" it gets saved.

This patch should be fairly safe to put in and gives form/password managers more correct behavior when it comes to saving forms. (forms are saved before the page gets to change the form submitted)

Checked in on the 1.8 branch. mozilla/content/html/content/src/nsHTMLFormElement.cpp 1.186.4.1

*** Bug 320661 has been marked as a duplicate of this bug. ***

Still got a problem with the password manager. If you go to www.dimeadozen.com and you give your username and password FF 2.0 says do you wanna keep this YES. The next time you wanna go to this page the username and password are not in the login scream. There has been a discussion on: http://www.mozbrowsec.php?t=8671&postdays=0&postorder=asc&start=0r.nl/forum/viewtopi

Frits, please file a separate bug on that issue with steps to reproduce, etc. Thanks.

I have identified the nasty bit of code responsible for this issue. It's in the vBulletin backwards compatibility engine, they had to leave in this automatic form reset for some reason (it just says "for backwards compatibility with old forum templates"). And although you are able to disable javascript and thereby login and save the password, I find that troublesome (going into Options and disabling it, then going back and enabling it). So instead, I decided to use Greasemonkey to inject a fix that bypasses the form clearing. Get it from here: http://userscripts.org/scripts/show/8021 It works seamlessly and transparently on all forums. The script has been setup to match sites containing the words *forum*, *board* and *bbs*, so in case your particular forum site doesn't have any of those in the name you can simply add it to the list of included sites. Unlike the fixed 2.0 beta, this script works on all versions of Firefox that support Greasemonkey (1.5 and higher).