When a tab is closed, the unload event is fired correctly, but pagehide is not.
Created attachment 198383 [details] [diff] [review] patch This makes us destroy the frameloader before destroying the content viewer. Frameloader destruction tears down the docshell, which dispatches the pagehide/unload events. If we've already destroyed the content viewer, the document can't get a prescontext to use for the event and bails.
Won't HTML iframes (which can end up with no presentation at all well before the docshell is destroyed) have the same problem?
Probably... but I don't know what a good solution is in that case. We could use a null nsPresContext for the event, I suppose, but I'm wary because of the weird way that the prescontext is used to determine the event target.
Is this something for 1.9, or for 1.8? For 1.9 we should definitely just fix that aspect of event targeting. For 1.8, I would be tempted to add a |target| member to nsEvent that can be used as a last fallback if non-null by nsDOMEvent::GetTarget. And set it when we fire pagehide/pageshow events. And then use null prescontexts. Or maybe even not as a last fallback -- if I read this code right the target of a pagehide can be some random content node if it happens to be the ESM's currentTarget when we fire the PAGE_HIDE.
There's another option, which isn't great but works and already exists... force creation of the DOMEvent up-front and use nsIPrivateDOMEvent::SetTarget ().
That doesn't sound so bad -- we end up creating the event anyway as soon as we have an event listener for it, and I think we always do here.
Brian, you're making a new patch here, right?
Comment on attachment 198383 [details] [diff] [review] patch yep, i'll be posting a new patch.
Created attachment 199106 [details] [diff] [review] patch Don't rely on a PresContext.
Comment on attachment 199106 [details] [diff] [review] patch Most excellent.
Comment on attachment 199106 [details] [diff] [review] patch I'd like to land this fix on the branch. It's important for extensions that want to be notified that a page is no longer visible, and should be safe (doesn't affect other codepaths).
bryner, is there any chance this would fix the problem at bug 312027. If not, can you look into that and tell us what you think?
(In reply to comment #13) > bryner, is there any chance this would fix the problem at bug 312027. If not, > can you look into that and tell us what you think? It's unlikely that this patch will address that problem.
Although nsDOMEvent::DuplicatePrivateData is no-op atm, nsDocument::DispatchEventToWindow should call it if there are more than one references to domEvt after sgo->HandleDOMEvent. I noticed this while doing some fixes to event dispatching (in trunk). The problem is that DOMEvent doesn't own nsEvent (because nsEvent is in stack) when events are dispatched in this strange way. But this is probably not a problem in branch because DuplicatePrivateData is anyway no-op.