435531 - problem saving login when form or existing login is password-only

Status: VERIFIED FIXED

(Toolkit :: Password Manager, defect)

Description

[Adam Marton]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9) Gecko/2008051206 Firefox/3.0
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9) Gecko/2008051206 Firefox/3.0

Given a website which requires simple html based username/password authentication. Firefox saves the login information for this login page. The protected site has a frameset where some page requires another authentication with different password (without username). Firefox 3 autofills the password field with the login page's password and won't remember the password what i need this sub-protected page.

Reproducible: Always

Steps to Reproduce:
See above.
Actual Results:  
Firefox 3 remember the login page's password only.

Expected Results:  
Need to save the different passwords for the different fields as Firefox 2.x

Comment 1

[Justin Dolske [:Dolske]]

I can't get past the first page on the site without a valid account.

Can you enable debugging and attach logs here? See http://wiki.mozilla.org/Firefox:Password_Manager_Debugging

Comment 2

[Adam Marton]

(In reply to comment #1)
> I can't get past the first page on the site without a valid account.
> 
> Can you enable debugging and attach logs here? See
> http://wiki.mozilla.org/Firefox:Password_Manager_Debugging
> 

This is the log:

Login Manager: domEventListener: got event blur
Login Manager: fillPassword autocomplete username: [e-mail]
Login Manager: Searching for logins matching host: https://portal.adatpark.hu, formSubmitURL: https://portal.adatpark.hu, httpRealm: null
Login Manager: Found a matching login, filling in password.
Login Manager: onStateChange accepted: req = https://portal.adatpark.hu/, flags = 0x30004
Login Manager: domEventListener: got event DOMContentLoaded
Login Manager: Counting logins matching host: https://portal.adatpark.hu, formSubmitURL: , httpRealm: null
Login Manager: fillDocument processing 1 forms on https://portal.adatpark.hu/
Login Manager: Searching for logins matching host: https://portal.adatpark.hu, formSubmitURL: https://portal.adatpark.hu, httpRealm: null
Login Manager: form[0]: found 1 matching logins.
Login Manager: attaching autocomplete stuff
Login Manager: domEventListener: got event blur
Login Manager: fillPassword autocomplete username: [e-mail]
Login Manager: Searching for logins matching host: https://portal.adatpark.hu, formSubmitURL: https://portal.adatpark.hu, httpRealm: null
Login Manager: Found a matching login, filling in password.
Login Manager: domEventListener: got event blur
Login Manager: fillPassword autocomplete username: [e-mail]
Login Manager: Searching for logins matching host: https://portal.adatpark.hu, formSubmitURL: https://portal.adatpark.hu, httpRealm: null
Login Manager: Found a matching login, filling in password.
Login Manager: observer notified for form submission.
Login Manager: Checking if logins to https://portal.adatpark.hu can be saved.
Login Manager: Searching for logins matching host: https://portal.adatpark.hu, formSubmitURL: https://portal.adatpark.hu, httpRealm: null
Login Manager: Found an existing login matching this form submission
Login Manager: onStateChange accepted: req = https://portal.adatpark.hu/portal.php, flags = 0x30004
Login Manager: domEventListener: got event DOMContentLoaded
Login Manager: onStateChange accepted: req = https://portal.adatpark.hu/oldal.php, flags = 0x30004
Login Manager: domEventListener: got event DOMContentLoaded
Login Manager: onStateChange accepted: req = https://portal.adatpark.hu/info.php, flags = 0x30004
Login Manager: domEventListener: got event DOMContentLoaded
Login Manager: onStateChange accepted: req = https://portal.adatpark.hu/forgalom1.php, flags = 0x30004
Login Manager: domEventListener: got event DOMContentLoaded
Login Manager: Counting logins matching host: https://portal.adatpark.hu, formSubmitURL: , httpRealm: null
Login Manager: fillDocument processing 1 forms on https://portal.adatpark.hu/forgalom1.php
Login Manager: (form -- no username field found)
Login Manager: Searching for logins matching host: https://portal.adatpark.hu, formSubmitURL: https://portal.adatpark.hu, httpRealm: null
Login Manager: form[0]: found 1 matching logins.
Login Manager: observer notified for form submission.
Login Manager: Checking if logins to https://portal.adatpark.hu can be saved.
Login Manager: (form -- no username field found)
Login Manager: Searching for logins matching host: https://portal.adatpark.hu, formSubmitURL: https://portal.adatpark.hu, httpRealm: null
Login Manager: Caught error in onFormSubmit: [Exception... "Not enough arguments [nsILoginInfo.matches]"  nsresult: "0x80570001 (NS_ERROR_XPC_NOT_ENOUGH_ARGS)"  location: "JS frame :: file:///C:/Program%20Files/Mozilla%20Firefox%203%20Beta%203/components/nsLoginManager.js :: anonymous :: line 842"  data: no]
Login Manager: onStateChange accepted: req = https://portal.adatpark.hu/forgalom2.php, flags = 0x30004
Login Manager: domEventListener: got event DOMContentLoaded
Login Manager: onStateChange accepted: req = http://hostingmrtg.adatpark.hu/forgalom_uj.php?id=fbdca36ea057977c04e4e76370305ee7, flags = 0x30004
Login Manager: domEventListener: got event DOMContentLoaded

Comment 3

[Adam Marton]

I've created a simulation environment to test it yourself:
http://lab.fotofabrik.hu/435531/

Comment 4

[Justin Dolske [:Dolske]]

(In reply to comment #2)

> Login Manager: Caught error in onFormSubmit: [Exception... "Not enough
> arguments [nsILoginInfo.matches]"  nsresult: "0x80570001
> (NS_ERROR_XPC_NOT_ENOUGH_ARGS)"  location: "JS frame ::
> file:///C:/Program%20Files/Mozilla%20Firefox%203%20Beta%203/components/nsLoginManager.js
> :: anonymous :: line 842"  data: no]

Uh-oh, that's the problem. I think I goofed when implementing matches(). :-(

Comment 5

[Justin Dolske [:Dolske]]

Attachment: Patch v.1

A few notes:

* This fixes the immediate problem, where the improper arguments to .matches() causes saving the login to fail. Using IDL's "[optional]" argument tag might be a slicker solution, but this seemed simpler and more conservative.

* Instead of a test specific to just this one case, I'd like to whip up a more comprehensive set of tests for saving logins. That path isn't as well tested (compared to the exhaustive tests for *filling* logins), because simulating clicking yes/no/never seemed too hard at the time.

* Need to spin off an issue to a followup bug. Even when the testcase's password-only login is saved, it won't actually get used again. :( There's a special case in fillForm() to deal with having 1 user+pass login and 1 pass-only login.... but only for filling in a form that takes a username and password (it prefers the user+pass login). For password-only forms, the code just thinks there are multiple logins, and fills in nothing. I think it should probably prefer the password-only login in this situation.

Comment 8

[Justin Dolske [:Dolske]]

Attachment: Patch v.2 (added tests)

Comment 9

[Justin Dolske [:Dolske]]

Comment on attachment 329268 [details] [diff] [review]
Patch v.2 (added tests)

Requesting approval for 3.0.2...

This bug can cause saving logins to fail when either the new or an existing login only has a password. The fix is simple and obvious, and we now have glorious tests for it.

Comment 10

[Justin Dolske [:Dolske]]

Pushed v.2 in changeset 0f0b873b98ba.

Spun off the issue from comment 5 into bug 444968.

Comment 11

[Samuel Sidler (old account; do not CC)]

Comment on attachment 329268 [details] [diff] [review]
Patch v.2 (added tests)

Oh, how I love tests.

Approved for 1.9.0.2. Please land in CVS. a=ss

Comment 12

[Justin Dolske [:Dolske]]

Checked in on branch:

Checking in toolkit/components/passwordmgr/src/nsLoginManager.js;
  new revision: 1.32; previous revision: 1.31
Checking in toolkit/components/passwordmgr/test/Makefile.in;
  new revision: 1.17; previous revision: 1.16
Checking in toolkit/components/passwordmgr/test/subtst_notifications_6.html;
  initial revision: 1.1
Checking in toolkit/components/passwordmgr/test/test_notifications.html;
  new revision: 1.2; previous revision: 1.1

Comment 13

[Al Billings [:abillings - ex-MoCo]]

Verified for 1.9.0.2 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.2) Gecko/2008090212 Firefox/3.0.2.