On sites where there is a pop-up that asks for your username and password, there is the option to "save these values". If I select it, it saves them, but on the next time I see the pop-up, it is unchecked. This is somewhat confusing, especially since it works if left unselected.

That was done by design. The checkbox always comes up unchecked, regardless of whether you checked it or not on the last usage of the box. Reason: saving passwords might comprimise privacy and we want the user to always do an explicit opt-in on each one he wants to save. In fact, in the original implementation I had the checkbox always coming up checked. This required an opt-out and I immediately got criticized for it. It was decided then to make it either opt-in or to remember previous state. I chose opt-in for the reason given above (and also because then I wouldn't have to implement any means of remembering the last state).

Actually, I meant that if I go to the same site I was before on which I checked it, it comes up unchecked. This confuses a bit, as you had previously checked this on the same site, and the info is inserted.

If you go to the same site, and you resubmit the same username and password, you won't need to save it again (it's already saved). And if you are now using a different username at that site, it could very well be a temporary thing in which case you don't want to save it. At least you should be made to opt-in if you have changed the conditions (username in this case).

Actually I can see your argument. If you go to a site for which the form comes up prefilled (meaning it was previously saved), then it should also be pre-checked. In fact, that would be a reminder to the user that these values are already being saved. And the implementation would be very simple So I'll remove the invalid indication and leave it as a request for a future enhancement.

Updating the summary to reflect current thinking on this enhancement request.

changed to enhancement...

Changing fictional "M30" to reality

adding helpwanted...

--> me morse, do we currently save this info anywhere?

Of course we already have saved the URL/name/password that was prefilled for the form. So when the form is submitted, we could go through the same logic that we went through at prefill time to see if a value is being saved. That could serve as the trigger for determining whether or not to precheck the checkbox.

spam: mass-moving open password manager (single signon) and form manager (autofill) bugs to Terri for qa contact. unfortunately, i cannot cc myself with this form, so feel free and add me if you want to keep me in the loop with any (but, pls not all :) of these... will also go thru 'em meself, a bit later...

any chance of this for 0.9?

No time right now.

Let me just clarify what this bug report is saying. There are two different situations in which the password manager asks the user if he wants to save his login. Case 1: Site puts up a login form, user fills it out, and then presses submit. He gets a "do you want to save" dialog with OK/Cancel buttons but no checkbox. Case 2: Site requests authentication in which case the password manager puts up a login form together with a "do you want to save" checkbox. This bug report is addressing case 2. If the password manager is able to prefill its own login form, it should also precheck the save box reminding the user that a value is already saved. This bug does not address case 1 since there is no checkbox in that case. Attaching real simple patch to fix this problem.

Makes sense to me. r:pnunn

Reviewing surrounding code: how about checking whether those ToNewUnicode calls fail and returning NS_ERROR_OUT_OF_MEMORY if so? /be

There's no error code returned from ToNewUnicode, so the best that could be done would be to check for the result being of 0 length. Furtherm, ToNewUnicode is called throughout this module so such a change would have to be more globally. In fact it is probably done thoughout the codebase without any check for failure. In any case, this is tangential to the current bug. If you feel that such checks should be made, shouldn't that be in a separate bug report?

>There's no error code returned from ToNewUnicode, so the best that could be >done would be to check for the result being of 0 length. Huh? ToNewUnicode returns a pointer to PRUnichar, null on out-of-memory. >Furtherm, ToNewUnicode is called throughout this module so such a change would >have to be more globally. In fact it is probably done thoughout the codebase >without any check for failure. Could be, but can we fix what we know to be broken? Worse is better, whereas making it everyone's problem makes it no one's problem. >In any case, this is tangential to the current bug. If you feel that such >checks should be made, shouldn't that be in a separate bug report? I'm asked to review code in order to uphold quality, and I'm doing just that, including looking at context (see http://www.mozilla.org/hacking/reviewers.html rule/tip #7). A separate bug does sound like a fine way to track the unchecked ToNewUnicode problem. Are you going to file one? /be

Netscape Nav triage team: this is a Netscape beta stopper. Adding Vishy to cc list

sr=brendan@mozilla.org on http://bugzilla.mozilla.org/showattachment.cgi?attach_id=22641. /be

Fix checked in.