Autofill username/password only fills the first form

RESOLVED FIXED in Camino1.5

Status

Camino Graveyard
HTML Form Controls
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: robbie_usenet, Assigned: Stuart Morgan)

Tracking

({fixed1.8.1.1})

unspecified
Camino1.5
PowerPC
Mac OS X
fixed1.8.1.1

Details

(URL)

Attachments

(1 attachment)

fix
639 bytes, patch
Stuart Morgan
: review+
Mike Pinkerton (not reading bugmail)
: superreview+
Details | Diff | Splinter Review
(Reporter)

Description

11 years ago
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
(Assignee)

Comment 1

11 years ago
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.
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

11 years ago
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
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: Autofill of user/login doesn't work on Slashdot since the redesign → Autofill of username/password fills in hidden forms
Target Milestone: --- → Camino1.1

Comment 4

11 years ago
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
(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. 
Stuart, can you take this?
(Assignee)

Comment 7

11 years ago
(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.
Assignee: nobody → stuart.morgan
Summary: Autofill of username/password fills in hidden forms → Autofill username/password only fills the first form
(Assignee)

Comment 8

11 years ago
(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.
(Assignee)

Comment 9

11 years ago
Created attachment 247147 [details] [diff] [review]
fix

Pretty straightforward.
Attachment #247147 - Flags: review?
(Assignee)

Comment 10

11 years ago
Comment on attachment 247147 [details] [diff] [review]
fix

r=cl and kreeger per IRC
Attachment #247147 - Flags: superreview?
Attachment #247147 - Flags: review?
Attachment #247147 - Flags: review+
(Assignee)

Updated

11 years ago
Attachment #247147 - Flags: superreview? → superreview?(mikepinkerton)
Comment on attachment 247147 [details] [diff] [review]
fix

sr=pink
Attachment #247147 - Flags: superreview?(mikepinkerton) → superreview+
(Assignee)

Comment 12

11 years ago
Checked in on trunk and MOZILLA_1_8_BRANCH.
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Keywords: fixed1.8.1.1
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.