Closed
Bug 376563
Opened 17 years ago
Closed 17 years ago
nsFormFillController::Unload doesn't ever get called
Categories
(Toolkit :: Form Manager, defect)
Toolkit
Form Manager
Tracking
()
RESOLVED
INVALID
People
(Reporter: martijn.martijn, Unassigned)
References
Details
This is a follow-up from bug 374881. When attempting to fix that bug, I noticed that nsFormFillController::Unload doesn't seem to get called at all. Normally you would expect that to get called, for instance when you've focused Google's search text input and then visit another page. Because it doesn't get called, formfillcontroller keeps references of stuff of the old page, which could cause weird bugs (bug 374881 was an example of that). Also, if this gets fixed, then I think the pagehide event should be used. See bug 343282, comment 5, I think that comment might also (partially) apply here.
Flags: blocking1.9?
Comment 1•17 years ago
|
||
I think we need to block on this to fix the "hold refs to the old document" issue at least.
Assignee: nobody → martijn.martijn
Flags: blocking1.9? → blocking1.9+
Reporter | ||
Comment 2•17 years ago
|
||
Michael, you happen to remember what this is about? Wasn't this basically fixed with your last patch in bug 374881?
Updated•17 years ago
|
Target Milestone: --- → mozilla1.9 M10
Reporter | ||
Comment 3•17 years ago
|
||
Well, I guess it wasn't working when I last tested this, but this is worksforme now, with current trunk debug build.
Assignee: martijn.martijn → nobody
Reporter | ||
Updated•17 years ago
|
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → WORKSFORME
Comment 4•17 years ago
|
||
Reopening, as this doesn't workforme (see bug 397753). Also, I originally ran into this when working on bug 384230, where the lack of Unload firing is contributing to a memory leak. Seems like this even might be fired for JS code, but not this C++ code.
Blocks: 384230
Status: RESOLVED → REOPENED
Component: Satchel → DOM: Events
OS: Windows XP → All
Product: Toolkit → Core
Hardware: PC → All
Resolution: WORKSFORME → ---
Target Milestone: mozilla1.9 M10 → ---
Version: unspecified → Trunk
Comment 5•17 years ago
|
||
Hmm. In nsPIDOMWindow.h I see the comment: 388 // These two variables are special in that they're set to the same 389 // value on both the outer window and the current inner window. Make 390 // sure you keep them in sync! 391 nsCOMPtr<nsPIDOMEventTarget> mChromeEventHandler; // strong 392 nsCOMPtr<nsIDOMDocument> mDocument; // strong The interesting thing about this bug is that JS code (in content) is getting the Unload event which the C++ code is not. [All the other events we registered for on the chrome EL are working.] Might this imply that when a document is being unloaded, there's a period where the inner/outer chrome handlers are out of sync, and so the unload event isn't reaching us? * - I'm not familiar with this code, so I'm kind of stabbing in the dark here...
Comment 6•17 years ago
|
||
I'll try to come up with some solution.
Assignee: nobody → Olli.Pettay
Status: REOPENED → NEW
Comment 7•17 years ago
|
||
The only case when I don't see unload event is when a tab/window is closed, and the reason for that is that nsFormFillController::RemoveWindowListeners is called before unload. Perhaps ::RemoveWindowListeners should do the same thing as ::Unload() So not a DOM Event bug. Justin or Martijn, want to take this bug? ;)
Assignee: Olli.Pettay → nobody
Component: DOM: Events → Form Manager
Flags: blocking1.9+
Product: Core → Firefox
QA Contact: satchel → form.manager
Comment 8•17 years ago
|
||
Um, this had blocking1.9+, but changing product/component removed that. So asking blocking‑firefox3?.
Flags: blocking-firefox3?
Comment 9•17 years ago
|
||
Ugh! Well, seems the right thing to do here anyway is to remove the nsIDOMLoadListener interfaces, as Martijn started to do over in bug 397753. No point in having ::Unload around if it's not going to get called. Since the failure to call ::Unload is expected behavior, I'm closing this as INVALID.
Status: NEW → RESOLVED
Closed: 17 years ago → 17 years ago
Resolution: --- → INVALID
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
•