344356 - Autofill username/password only fills the first form

Status: RESOLVED FIXED

(Camino Graveyard :: HTML Form Controls, defect)

Description

[robbie_usenet]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.1b1) Gecko/20060712 Camino/1.0+
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.1b1) Gecko/20060712 Camino/1.0+

Slashdot redesigned their site to use better HTML/CSS. Since then, Camino doesn't want to autofill the username and login for the main page (although it accesses the keychain)

Reproducible: Always

Steps to Reproduce:
1. Create a Slashdot username/password
2. Login with your credentials
3. Logout of slashdot again
4. Go back to http://slashdot.org/

Actual Results:  
The slashdot main page is presented with a blank Username and Password field

Expected Results:  
It should have the Username and Password filled in with the details from step 2

Comment 1

[Stuart Morgan]

If you open the disclosure triangle next to "Login" in the upper left, you'll find that it's filled in there.  Why they think they need two different login forms on the same page, I'm not sure.

I'm not sure there's much Camino can do here, except maybe change the password field detection to skip anything hidden.

Comment 2

[Mike Pinkerton (not reading bugmail)]

agreed, hidden fields could be a security hole for sites that want to grab your password w/out you being able to see it.

Comment 3

[Chris Lawson (gone)]

Confirming per IRC, since this is a potential security issue. We need to skip hidden form fields when looking for stuff to fill.

I'll hazard a guess that this won't be too difficult, and we should have it for 1.1.

cl

Comment 4

[Chris Lawson (gone)]

Change needs to be made here:

http://lxr.mozilla.org/mozilla/source/camino/src/browser/KeychainService.mm#1125

That shouldn't return hidden fields.

cl

Comment 5

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

(In reply to comment #2)
> agreed, hidden fields could be a security hole for sites that want to grab your
> password w/out you being able to see it.

Disagreed. If you intentionally give a site your username and password and save it, you want it to be filled in. We don't fill in username/passwords in iframes that don't match the domain, do we? Not as I understand it, at least. There's no security hole here.

The only bug I see is that we're not filling in both forms as we should be. 

Comment 6

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Stuart, can you take this?

Comment 7

[Stuart Morgan]

(In reply to comment #5)
> The only bug I see is that we're not filling in both forms as we should be. 

Yeah, we stop once we've filled a form, and I don't think there's any compelling reason not to fill all the forms that look like login forms.

I also don't really buy the argument that a site would want to steal its own password from users--and if it were a real problem, skipping things in hidden divs wouldn't help, since I can think of a handful of other ways to get the same effect with varying degrees of undetectability to the browser and user.

Comment 8

[Stuart Morgan]

(In reply to comment #7)
> I also don't really buy the argument that a site would want to steal its own
> password from users

I wish I'd waited a few more days before making that claim. Anyway, I've filed bug 361463 to address that issue; regardless of how we solve that and what forms we don't fill, we should probably fill any legit forms on a page, even if there are more than one.

Comment 9

[Stuart Morgan]

Attachment: fix

Pretty straightforward.

Comment 10

[Stuart Morgan]

Comment on attachment 247147 [details] [diff] [review]
fix

r=cl and kreeger per IRC

Comment 11

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 247147 [details] [diff] [review]
fix

sr=pink

Comment 12

[Stuart Morgan]

Checked in on trunk and MOZILLA_1_8_BRANCH.