[pwd-mngr] password manager needs to fill in forms before the page finishes loading

RESOLVED FIXED in mozilla1.8.1beta2

Status

()

Toolkit
Password Manager
--
enhancement
RESOLVED FIXED
15 years ago
7 years ago

People

(Reporter: Martin Meyer, Assigned: mwu)

Tracking

({fixed1.8.1, testcase})

1.8 Branch
mozilla1.8.1beta2
fixed1.8.1, testcase
Points:
---
Dependency tree / graph
Bug Flags:
blocking-aviary1.0 -
blocking-aviary1.5 -
blocking-firefox2 -

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

15 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6a) Gecko/20031002 Firebird/0.7+
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6a) Gecko/20031002 Firebird/0.7+

On pages that don't always completely load or that load slowely, saved passwords
are not filled in until the page finished loading (or somewhere near the end of
this process).  The result is that I visit a page, and instead of the login info
filling in immediatly I have to wait for the entire page to finish rendering
before I can click the login button.

Reproducible: Always

Steps to Reproduce:
1. Visit a slow-loading website with a saved login
2. Wait until the login box fills in

Actual Results:  
The login information is not filled in until the page is done.  Another,
possibly unrelated, bug is that sometimes either the password or both fields do
not get filled in at all.

Expected Results:  
I should be able to click the login button long before the page finishes loading.

Comment 1

15 years ago
See also bug 58724, same bug for Seamonkey.

Comment 2

15 years ago
how about changing the summary from:
"password manager needs to fill in forms earlier"
to something like:
"password manager needs to fill in forms before the page finishes loading"
so it's easier to search for this bug.?
-> bryner
Assignee: blake → bryner

Comment 4

15 years ago
-->Autocomplete
Component: General → Autocomplete
QA Contact: davidpjames

Comment 5

15 years ago
Adding [pwd-mngr] to summary of Password Manager bugs to facilitate querying.
Sorry for the bugspam.
Summary: password manager needs to fill in forms earlier → pwd-mngr] password manager needs to fill in forms earlier

Comment 6

15 years ago
Tweaking summary as per suggestion in comment #2 (plus fixing an oops of mine).
Again, sorry for the bugspam. Really.
Summary: pwd-mngr] password manager needs to fill in forms earlier → [pwd-mngr] password manager needs to fill in forms before the page finishes loading

Updated

15 years ago
Component: Autocomplete → Password Manager

Updated

14 years ago
Flags: blocking1.0?
-ing will take patch
Flags: blocking1.0? → blocking1.0-

Updated

14 years ago
Blocks: 251959

Updated

14 years ago
No longer blocks: 251959

Updated

14 years ago
Blocks: 251959

Updated

14 years ago
No longer blocks: 251959

Comment 8

14 years ago
I just came to report this same problem, an additional scenario:

When you visit a website (doesn't even need to be a slow one as in the first
example) the login/pass box can sit there empty and will not auto-fill because
Firefox is still waiting for a banner ad from a 3rd party site to finish
downloading, even though the page requested has finished loading completely, its
waiting for an image/banner from a third party domain to laod before it will
auto-fill the form.

I run into this constantly at:
http://movies.hsx.com/servlet/Portfolio

The banner at the top takes forever to load (depending on which of their
rotating ad servers is trying to show a banner, a couple are quicker), so im
being forced to type in my info every time, or wait forerver to see an ad that I
don't want to view anyways.
*** Bug 272462 has been marked as a duplicate of this bug. ***

Comment 10

14 years ago
I believe this is a very common bug that needs to be fixed. It is very annoying.
I am trying to find out the part of the source code which is responsible of
this. In the file content/html/content/src/nsHTMLFormElement.cpp I found some
lines of code that probably are the ones calling the password manager when a
form is found, however I still cannot find out from where that function is being
called during the parsing of the html page. I'm not very experienced in c++ yet.
So... any help anyone?
*** Bug 278147 has been marked as a duplicate of this bug. ***
*** Bug 279430 has been marked as a duplicate of this bug. ***

Comment 13

14 years ago
Nominating as blocker, annoying bug esp. for users on dialup connections and
bloated pages.  
Flags: blocking-aviary1.1?
Adding bug 58724 as depends to help tracking.
Depends on: 58724
OS: Windows XP → All
Hardware: PC → All
Version: unspecified → Trunk
*** Bug 281124 has been marked as a duplicate of this bug. ***

Comment 16

14 years ago
*** Bug 281453 has been marked as a duplicate of this bug. ***

Comment 17

14 years ago
This bug actually causes dataloss (loss of the saved password) in certain
circumstances, and this is easily reproducable, therefore, I would assume quite
commonplace. 

Let's say, for instance, that the particular login page is taking a while to
load, but instinctively, you click a button to "log in" before the password
field is populated. This submits the form with no password, which of course will
return a failure page. However, this will also save a _blank_ password to your
manager, overwriting your previous entry. While that should technically be a bug
in it's own right, populating the field on creation would no doubt suffice.

Steps to reproduce:
1) Go to a site that you've chosen to store the username/password for.
2) Click "login" or "ok" before page finishes loading (and thus, before the
password field is populated).
3) Password is now blanked out.

Note:
This is reliant on the username being filled in first, but chances are that
cookies will handle that for some sites. I don't know if the password manager
plays a part in this or not.
not going to block on this, it'd be nice to get a fix, of course
Flags: blocking-aviary1.1? → blocking-aviary1.1-

Updated

13 years ago
Flags: blocking-aviary2.0?
*** Bug 303701 has been marked as a duplicate of this bug. ***
*** Bug 316292 has been marked as a duplicate of this bug. ***

Comment 21

13 years ago
*** Bug 317350 has been marked as a duplicate of this bug. ***
Comment 17 is a separate bug and should be split off from this.  If we're erasing the password, that probably should either be ignored or be a prompt.  I can't think of any sites offhand that let registered users not have a password, but that's not empirical.

Leaving the decision on this up to bryner, since he's better-able to assess the performance hit we'll take if we're doing this during pageload.
nice-to-have, not a blocker.
Flags: blocking-aviary2? → blocking-aviary2-

Comment 24

13 years ago
One possible solution is to call pwd mgr on DOMContentLoaded 

See bug 325280

Comment 25

13 years ago
*** Bug 326537 has been marked as a duplicate of this bug. ***

Comment 26

13 years ago
*** Bug 325280 has been marked as a duplicate of this bug. ***

Comment 27

13 years ago
copying my comments from bug 325280...

Call Password manager on DOMContentLoaded, not onload.

Current U/X:
Page loads, images load, then password manager is called.

Should be:
Page loads, password manager is called while/before images are loaded.

The improvement to the U/X would be that Mozilla would seem faster. The U/X
would be sped up because the user wouldn't have to wait for images, which are
often doubleclick.net ad banners.

Using paypal, my ebay, and any other site would be faster for the user.

Comment 28

13 years ago
Does anyone have a good testcase for this? Ideally, please also supply username/password I can test with.

Comment 29

13 years ago
(In reply to comment #28)
> Does anyone have a good testcase for this?

huh?  pretty much any login page will do.  are you unable to notice that passwords and usernames aren't automagically filled in until after all content has finished loading?

marc

Comment 30

13 years ago
I'm working on a patch for this, and would like more sites for testing purposes (currently I have only about 3-4), especially if there is any particularly good testcase.

Comment 31

13 years ago
www.jdate.com is good - they load this awful flash thingy on the home page.

Updated

13 years ago
Assignee: bryner → hwaara

Comment 32

13 years ago
So, the main problem is that we're right now listening to nsIWebProgressListener's STATE_IS_DOCUMENT/STATE_STOP events, i.e., when a whole document request has completed, we start parsing the document and then autofilling.

Unfortunately we can't just change that to STATE_START, because then the content hasn't been parsed yet.

However, we can listen to DOMContentLoaded (fired when content has parsed the document), and then start having fun with the document.

The problem is that the passwordmgr is first initialized very late; when nsHTMLFormElement detects its first 'password' element, so the very first time passwordmgr is called we actually *miss* most of the request states (STATE_IS_DOCUMENT/STATE_START for example, where we could otherwise register as listeners to DOMContentLoaded).

Now, ALL embeddors (Firefox, Seamonkey and Camino etc.) have this same bug because it's such a mess knowing when you can start parsing the form-elements.

So here are some proposed ways to go here:

Add an interface nsIFormObserver (or even rename and generalize the current nsIFormSubmitObserver?) where we could add a notification for when the form elements are parsed and ready to look at.   This would make it very easy for all embeddors to know when to invoke the password-autofilling feature, and they don't have to listen to random network state requests anymore just for this (perf win?).

2) Register passwordmgr at startup, and make it always listen to DOMContentLoaded to every HTML document it encounters.

#1 is nicest IMHO, and is the way I propose to go.
#2 may not be good for performance, and we will still have to listen to request states just to know when to attach ourselves as DOMContentLoaded-listeners.

So I'd like to do #1.
Comments?
> (or even rename and generalize the current nsIFormSubmitObserver?)

This has nothing to do with form submission.

> where we could add a notification for when the form elements are parsed and
> ready to look at.

Meaning what?

> #1 is nicest IMHO, and is the way I propose to go.

Why?

> #2 may not be good for performance

Why?

> and we will still have to listen to request states just to know when to attach
> ourselves as DOMContentLoaded-listeners.

But that's why nsIWebProgress is there -- it's the generalized thing for being able to watch all documents.  As in, using it here is exactly correct as far as I can tell.

I also can't tell exactly what behaviour you're trying to achieve.  Which means I really have no idea what a reasonable way to accomplish it would be.

Comment 34

13 years ago
it looks like with option #1 we get each form as it is loaded, whereas with #2 we get all forms at once. 

#2 would still be a huge improvement. 
#2 would probably be easier to implement.


#1 would be a problem when you have more than one form per pg. A prompt to select the first form's pw would delay pg rendering of the rest of the pg. This could be annoying.

#1 could be a faster u/x only when given a large doc with large auto-layout tables that take time to load. In this case, all forms would be delayed until the page loaded.  still, though, it would slow down page load to have to deal with multiple pw select-prompts WHILE the page loads because it seems like the page load would have to wait for the user to select which username. Am i wrong? slow loading pages = bad u/x.

Comment 35

13 years ago
It should be noted that ever since IE began remembering passwords, they have always been filled-in way before the page is loaded. I don't think this is a nice-to-have, it's an extremely annoying performance issue that results in wasted time.

Comment 37

12 years ago
For the love of god can we get movement on this?  Every site I visit that I have saved a password at I have to wait for the page to complete loading before the password filled is auto-filled for me!  This is very annoying especially at bill paying sites!

~B
@gavin, @all:
are you pretty sure this is a "depend" relationship with bug 58724?

"Mozilla fills the textboxes of login and password
only when the page has been completely loaded.
Mozilla should fill this textboxes as soon as they
appears
(there is no need to wait the end of the download
of all the images)"

That bug has also a testcase and has more votes than this one.


1 - Everyone can quote comments from this bug there, as they are interesting and are one of the reasons I'm not closing this right now.
2 - Another reason is to ask every voter of this issue to vote on the other.


As bug 58724 has more votes, is older, have a good testcase, I think this one should be closed as duplicate (and changing a bit the former summary).
It's going to happen except assignee and QA contact from that bug agreed with closing that one in flavour of this.


2 or 3 days until there shouldn't bother.

No longer depends on: 325280
Bug 58724 is a SeaMonkey bug (it's in Core because the Mozilla Application Suite product doesn't have a good component for it), so no, this is not a duplicate. The reason this bug "depends" on that one is because it is possible that a fix to that bug could be ported over to the equivalent Firefox code, or vice versa.

Comment 40

12 years ago
Mass edit: Changing QA to default QA Contact
QA Contact: davidpjames → password.manager
(Assignee)

Comment 41

12 years ago

*** This bug has been marked as a duplicate of 58724 ***
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → DUPLICATE
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Assignee: hwaara → michael.wu
Status: REOPENED → NEW
Fixed by patches attached to bug 58724
Status: NEW → RESOLVED
Last Resolved: 12 years ago12 years ago
Resolution: --- → FIXED
Re-requesting blocking-firefox2, as bug 58724 was marked blocking1.8.1+.
Status: RESOLVED → REOPENED
Flags: blocking-firefox2- → blocking-firefox2?
Resolution: FIXED → ---
Severity: normal → enhancement
Status: REOPENED → ASSIGNED
Target Milestone: --- → Firefox 2 beta1
Version: Trunk → 2.0 Branch
Er, already on trunk, but 1.8 patch will go here, too. :)
Sorry about that.
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago12 years ago
Resolution: --- → FIXED
(Assignee)

Comment 45

12 years ago
Created attachment 227433 [details] [diff] [review]
Fill in form on DOMContentLoaded event

Copied from bug 58724.
Attachment #227433 - Flags: approval1.8.1?
Comment on attachment 227433 [details] [diff] [review]
Fill in form on DOMContentLoaded event

Please request approval on Friday once this has baked on trunk for a couple of days
Attachment #227433 - Flags: approval1.8.1?
Not blocking, but will take the patch if no regressions are on trunk. Michael is to ask for approval1.8.1 on Friday.
Flags: blocking-firefox2? → blocking-firefox2-
(Assignee)

Updated

12 years ago
Attachment #227433 - Flags: approval1.8.1?
Were bugs 343182 and 343264 regressions from this bug or from bug 257781?
(Assignee)

Updated

12 years ago
Depends on: 343169
No longer depends on: 58724

Comment 49

12 years ago
Need an answer to comment 48 before we approve for branch...
(Assignee)

Comment 50

12 years ago
This patch causes bug 343169, bug 343264, and bug 343022. It doesn't cause, though it does make worse, bug 343182. Bug 343264 and bug 343022 are both fixed by the same patch, and bug 343169 is somewhat minor IMHO. The problem right now is that the patch for bug 343264 is waiting for review. It's probably not a good idea to put this patch on branch until the regression fix in bug 343264 also lands.
Depends on: 343282
The patch also seems to have caused bug 343282, which is a too serious bug for 1.8.1, I think.
(Assignee)

Updated

12 years ago
Attachment #227433 - Flags: approval1.8.1?
(Assignee)

Updated

12 years ago
Target Milestone: Firefox 2 beta1 → Firefox 2 beta2
Blocks: 317894
(Assignee)

Updated

12 years ago
No longer depends on: 343182
(Assignee)

Comment 52

12 years ago
*** Bug 286319 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 53

12 years ago
Created attachment 229625 [details] [diff] [review]
Fill in form on DOMContentLoaded event, v2

This is a backport of the trunk early-form-fill patch with all regression fixes rolled in.
Attachment #227433 - Attachment is obsolete: true
Attachment #229625 - Flags: approval1.8.1?
Comment on attachment 229625 [details] [diff] [review]
Fill in form on DOMContentLoaded event, v2

a=mconnor on behalf of drivers, please land ASAP
Attachment #229625 - Flags: approval1.8.1? → approval1.8.1+
mozilla/toolkit/components/passwordmgr/base/nsPasswordManager.cpp 	1.65.2.7
mozilla/toolkit/components/passwordmgr/base/nsPasswordManager.h 	1.15.4.1
Keywords: fixed1.8.1
(Assignee)

Updated

12 years ago
Blocks: 345395

Comment 56

12 years ago
*** Bug 352239 has been marked as a duplicate of this bug. ***
(Assignee)

Updated

12 years ago
Blocks: 355063

Comment 57

12 years ago
*** Bug 357054 has been marked as a duplicate of this bug. ***
Duplicate of this bug: 369628

Updated

11 years ago
Depends on: 376765

Comment 59

11 years ago
This seems still to affect Camino 1.6a1 and prior, it seems to be fixed in a Gecko version that is not used in Camino yet, and I can find no information on roadmap for this inclusion.

How should I pursue this for Camino - a new bug, or some continuation of this one?

FWIW, I used the test URL associated with this issue, "logged in" once, saved the password in keychain via Camino, closed the tab, and loaded the URL again - it just sits and spins while trying to load the never-loading image, which I assume is a clear incarnation of the bug, in addition to the fact that it just doesn't seem fixed for me on a daily basis.
(In reply to comment #59)
> How should I pursue this for Camino - a new bug, or some continuation of this
> one?

This is a Firefox bug, feel free to file a bug against the Camino browser
Product: Firefox → Toolkit
No longer blocks: 355063
You need to log in before you can comment on or make changes to this bug.