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)
Tracking
()
RESOLVED
FIXED
mozilla1.8.1beta2
People
(Reporter: elreydetodo, Assigned: mwu)
References
()
Details
(Keywords: fixed1.8.1, testcase)
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
mconnor
:
approval1.8.1+
|
Details | Diff | Splinter Review |
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 2•21 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.?
Comment 5•21 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•21 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•21 years ago
|
Component: Autocomplete → Password Manager
Updated•21 years ago
|
Flags: blocking1.0?
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.
Comment 9•20 years ago
|
||
*** Bug 272462 has been marked as a duplicate of this bug. ***
Comment 10•20 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?
Comment 11•20 years ago
|
||
*** Bug 278147 has been marked as a duplicate of this bug. ***
Comment 12•20 years ago
|
||
*** Bug 279430 has been marked as a duplicate of this bug. ***
Comment 13•20 years ago
|
||
Nominating as blocker, annoying bug esp. for users on dialup connections and
bloated pages.
Flags: blocking-aviary1.1?
Comment 14•20 years ago
|
||
Adding bug 58724 as depends to help tracking.
Comment 15•20 years ago
|
||
*** Bug 281124 has been marked as a duplicate of this bug. ***
Comment 16•20 years ago
|
||
*** Bug 281453 has been marked as a duplicate of this bug. ***
Comment 17•20 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.
Comment 18•19 years ago
|
||
not going to block on this, it'd be nice to get a fix, of course
Flags: blocking-aviary1.1? → blocking-aviary1.1-
Comment 19•19 years ago
|
||
*** Bug 303701 has been marked as a duplicate of this bug. ***
Comment 20•19 years ago
|
||
*** Bug 316292 has been marked as a duplicate of this bug. ***
Comment 21•19 years ago
|
||
*** Bug 317350 has been marked as a duplicate of this bug. ***
Comment 22•19 years ago
|
||
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.
Comment 24•19 years ago
|
||
One possible solution is to call pwd mgr on DOMContentLoaded
See bug 325280
Comment 25•19 years ago
|
||
*** Bug 326537 has been marked as a duplicate of this bug. ***
Comment 26•19 years ago
|
||
*** Bug 325280 has been marked as a duplicate of this bug. ***
Comment 27•19 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•19 years ago
|
||
Does anyone have a good testcase for this? Ideally, please also supply username/password I can test with.
Comment 29•19 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•19 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•19 years ago
|
||
www.jdate.com is good - they load this awful flash thingy on the home page.
Updated•19 years ago
|
Assignee: bryner → hwaara
Comment 32•19 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?
Comment 33•19 years ago
|
||
> (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•19 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•19 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 36•19 years ago
|
||
http://www.squarefree.com/bug221634/ is a simple, reliable testcase.
Keywords: testcase
Comment 37•19 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
Comment 38•19 years ago
|
||
@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.
Comment 39•19 years ago
|
||
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•18 years ago
|
||
Mass edit: Changing QA to default QA Contact
QA Contact: davidpjames → password.manager
Assignee | ||
Comment 41•18 years ago
|
||
*** This bug has been marked as a duplicate of 58724 ***
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → DUPLICATE
Updated•18 years ago
|
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Updated•18 years ago
|
Assignee: hwaara → michael.wu
Status: REOPENED → NEW
Comment 42•18 years ago
|
||
Fixed by patches attached to bug 58724
Status: NEW → RESOLVED
Closed: 18 years ago → 18 years ago
Resolution: --- → FIXED
Comment 43•18 years ago
|
||
Re-requesting blocking-firefox2, as bug 58724 was marked blocking1.8.1+.
Status: RESOLVED → REOPENED
Flags: blocking-firefox2- → blocking-firefox2?
Resolution: FIXED → ---
Updated•18 years ago
|
Severity: normal → enhancement
Status: REOPENED → ASSIGNED
Target Milestone: --- → Firefox 2 beta1
Version: Trunk → 2.0 Branch
Comment 44•18 years ago
|
||
Er, already on trunk, but 1.8 patch will go here, too. :)
Sorry about that.
Status: ASSIGNED → RESOLVED
Closed: 18 years ago → 18 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 45•18 years ago
|
||
Copied from bug 58724.
Attachment #227433 -
Flags: approval1.8.1?
Comment 46•18 years ago
|
||
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?
Comment 47•18 years ago
|
||
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•18 years ago
|
Attachment #227433 -
Flags: approval1.8.1?
Comment 48•18 years ago
|
||
Were bugs 343182 and 343264 regressions from this bug or from bug 257781?
Updated•18 years ago
|
Comment 49•18 years ago
|
||
Need an answer to comment 48 before we approve for branch...
Assignee | ||
Comment 50•18 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.
Comment 51•18 years ago
|
||
The patch also seems to have caused bug 343282, which is a too serious bug for 1.8.1, I think.
Assignee | ||
Updated•18 years ago
|
Attachment #227433 -
Flags: approval1.8.1?
Assignee | ||
Updated•18 years ago
|
Target Milestone: Firefox 2 beta1 → Firefox 2 beta2
Assignee | ||
Comment 52•18 years ago
|
||
*** Bug 286319 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 53•18 years ago
|
||
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 54•18 years ago
|
||
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+
Comment 55•18 years ago
|
||
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
Comment 56•18 years ago
|
||
*** Bug 352239 has been marked as a duplicate of this bug. ***
Comment 57•18 years ago
|
||
*** Bug 357054 has been marked as a duplicate of this bug. ***
Comment 59•17 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.
Comment 60•17 years ago
|
||
(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
Updated•16 years ago
|
Product: Firefox → Toolkit
You need to log in
before you can comment on or make changes to this bug.
Description
•