Closed Bug 221634 Opened 21 years ago Closed 18 years ago

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

Categories

(Toolkit :: Password Manager, enhancement)

1.8 Branch
enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.8.1beta2

People

(Reporter: elreydetodo, Assigned: mwu)

References

()

Details

(Keywords: fixed1.8.1, testcase)

Attachments

(1 file, 1 obsolete file)

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.
See also bug 58724, same bug for Seamonkey.
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
-->Autocomplete
Component: General → Autocomplete
QA Contact: davidpjames
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
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
Component: Autocomplete → Password Manager
Flags: blocking1.0?
-ing will take patch
Flags: blocking1.0? → blocking1.0-
Blocks: 251959
No longer blocks: 251959
Blocks: 251959
No longer blocks: 251959
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. ***
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. ***
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. ***
*** Bug 281453 has been marked as a duplicate of this bug. ***
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-
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. ***
*** 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-
One possible solution is to call pwd mgr on DOMContentLoaded See bug 325280
*** Bug 326537 has been marked as a duplicate of this bug. ***
*** Bug 325280 has been marked as a duplicate of this bug. ***
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.
Does anyone have a good testcase for this? Ideally, please also supply username/password I can test with.
(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
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.
www.jdate.com is good - they load this awful flash thingy on the home page.
Assignee: bryner → hwaara
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.
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.
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.
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.
Mass edit: Changing QA to default QA Contact
QA Contact: davidpjames → password.manager
*** This bug has been marked as a duplicate of 58724 ***
Status: NEW → RESOLVED
Closed: 18 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
Closed: 18 years ago18 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
Closed: 18 years ago18 years ago
Resolution: --- → FIXED
Attached patch Fill in form on DOMContentLoaded event (obsolete) (deleted) — Splinter Review
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-
Attachment #227433 - Flags: approval1.8.1?
Were bugs 343182 and 343264 regressions from this bug or from bug 257781?
Depends on: 343169
Depends on: 343182, 343264
No longer depends on: 58724
Need an answer to comment 48 before we approve for branch...
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.
Attachment #227433 - Flags: approval1.8.1?
Target Milestone: Firefox 2 beta1 → Firefox 2 beta2
Blocks: 317894
No longer depends on: 343182
*** Bug 286319 has been marked as a duplicate of this bug. ***
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
Blocks: 345395
*** Bug 352239 has been marked as a duplicate of this bug. ***
Blocks: 355063
*** Bug 357054 has been marked as a duplicate of this bug. ***
Depends on: 376765
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
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: