Closed
Bug 317894
Opened 19 years ago
Closed 18 years ago
password manager leaks element (and thus window object) after filling password on verizonwireless.com
Categories
(Toolkit :: Password Manager, defect)
Tracking
()
VERIFIED
FIXED
People
(Reporter: dbaron, Unassigned)
References
()
Details
(Keywords: memory-leak, verified1.8.1)
Attachments
(2 files)
(deleted),
text/plain; charset=utf-8
|
Details | |
(deleted),
patch
|
Details | Diff | Splinter Review |
Password manager leaks an element (and because of its event listener, the global window) when filling in the password on http://www.verizonwireless.com/ .
nsPasswordManager::Unload is never called. The code seems to rely on it being called to prevent leaks. (That the password manager itself is leaked is the only reason I can see that the leak still showed up after shutdown; that may be related to the window object being leaked; it may not.)
Steps to reproduce:
1. do whatever's needed to get password manager to fill in the password on http://www.verizonwireless.com/
2. start Firefox
3. load http://www.verizonwireless.com/ (and wait for the page to load and the username/password to be filled in)
4. exit Firefox
Reporter | ||
Comment 1•19 years ago
|
||
Reporter | ||
Comment 2•19 years ago
|
||
The DOM spec says unload is fired for BODY and FRAMESET. Do we fire it for documents as well, or is that the problem? Or is the problem that it's not firing in this specific case for some reason?
Assignee: dbaron → nobody
Comment 3•19 years ago
|
||
(In reply to comment #2)
> The DOM spec says unload is fired for BODY and FRAMESET. Do we fire it for
> documents as well, or is that the problem? Or is the problem that it's not
> firing in this specific case for some reason?
>
IIRC, we fire unload only for window objects. (Bug 99820)
Reporter | ||
Comment 4•19 years ago
|
||
I see this loading an expedia itinerary as well.
Reporter | ||
Comment 5•18 years ago
|
||
This may have been fixed by the patch to bug 221634. See bug 343282 comment 5.
Comment 6•18 years ago
|
||
Yeah, probably fixed. I'll try to look and see if it is fixed, soon.
Depends on: 221634
Comment 7•18 years ago
|
||
Somehow, I can't reproduce this, I tried to see leaks with the leak-gauge tool as well as with the latest leak extension.
Comment 8•18 years ago
|
||
(In reply to comment #7)
> Somehow, I can't reproduce this, I tried to see leaks with the leak-gauge tool
> as well as with the latest leak extension.
>
Bloat log should show nsPasswordManager being leaked, I think..
Updated•18 years ago
|
Flags: blocking1.9a1?
Flags: blocking-firefox2?
Comment 9•18 years ago
|
||
Since bug 221634 has now landed on branch, can we see if this leak still exists?
Keywords: qawanted
Comment 10•18 years ago
|
||
not going to block on this since we can't reproduce at present.
Flags: blocking-firefox2? → blocking-firefox2-
Reporter | ||
Comment 11•18 years ago
|
||
I think this assertion would fire if the bug still existed (and the password manager itself were not leaked).
Comment 12•18 years ago
|
||
Ok, I've added the assertion in my debug build and followed the steps to reproduce, after that I don't get an assertion, so that means this bug is fixed/worksforme now?
Comment 13•18 years ago
|
||
I verified (with leak gauge) that this was fixed sometime between 2006-07-19-04 and 2006-07-20-04 on branch. So mwu's patch for bug 221634/bug 343182 did in fact fix the leak.
Don't know if you still think the assertion should be added, David. Maybe file a follow-up bug?
Status: NEW → RESOLVED
Closed: 18 years ago
Flags: blocking1.9a1?
Keywords: qawanted
Resolution: --- → FIXED
Comment 14•18 years ago
|
||
*** Bug 323406 has been marked as a duplicate of this bug. ***
Updated•18 years ago
|
Keywords: verified1.8.1
Updated•18 years ago
|
Status: RESOLVED → VERIFIED
Assignee | ||
Updated•16 years ago
|
Product: Firefox → Toolkit
You need to log in
before you can comment on or make changes to this bug.
Description
•