1398625 - Closing browserAction popup does not fire beforeunload event

Status: NEW

(WebExtensions :: General, enhancement, P3)

Description

[u462496]

The lack of `beforeunload` event when browserAction popup closes makes persistence of data between displays inconvenient and costly.

Note that for sidebarAction, beforeunload does get fired.

Comment 1

[u462496]

Any chance of there being some activity on this?

Comment 2

[Shane Caraveo (:mixedpuppy)]

Kevin,
What is the use case?  Couldn't you just use the visibility api?

https://developer.mozilla.org/en-US/docs/Web/API/Page_Visibility_API

Comment 3

[Shane Caraveo (:mixedpuppy)]

The problem with beforeunload is that its primary use case is to prevent unloading, with user interaction.  For doorhanger panels that isn't going to be allowed.  If the use case is to get a notification that the panel is being closed, I'm pretty sure the visibility api should cover that.  If it is not firing (i kind of recall there are situations where it does not) we can fix that.

Comment 4

[u462496]

(In reply to Shane Caraveo (:mixedpuppy) from comment #3)
> The problem with beforeunload is that its primary use case is to prevent
> unloading, with user interaction.  For doorhanger panels that isn't going to
> be allowed.  If the use case is to get a notification that the panel is
> being closed, I'm pretty sure the visibility api should cover that.  If it
> is not firing (i kind of recall there are situations where it does not) we
> can fix that.

I need more than just knowing the browserAction popup has closed, I need to get data before the objects disappear.

For example, I am needing to grab the last scroll position of a menu before the popup closes.  Currently I have to store the scroll position every time the user scrolls the menu since I never know when the popup will close.  There are other UI states also that I want to store on closing, which I am now having to store every time the user makes an action.

If the visibility API can reliably notify my that the popup is closing before the objects disappear, then that would suffice.  I guess I will have to experiment to see if that will work.  Though even if I see apparent success in using this approach, would I be able to guarantee consistency, since all this stuff works async?

Comment 5

[u462496]

So there is an interesting effect using visibilitychange event on browserAction popup.

When I try to grab the scroll position menu of a menu I have in the popup, it always returns 0, even though the menu is at a scroll position other than 0 when closing the menu.

But oddly, I observe that using visibilitychange event in the sidebar gives the correct scroll position.

I seem to be able to collect some other states correctly for my needs using visibilitychange event so that is a step forward.  But it appears I will still need to use a "scroll" event listener to track the last scroll position in a variable.

Comment 6

[Andy McKay]

Doing this is still questionable at this point. Does Chrome fire beforeunload and does it allow you to block? 

It seems like the particular use case here is to able to persist a state in the UI and I think that browser action might not be a good place for that, however it might be that there aren't many choices for doing that.

Comment 7

[u462496]

(In reply to Andy McKay [:andym] from comment #6)
> Doing this is still questionable at this point. Does Chrome fire
> beforeunload and does it allow you to block? 
> 
> It seems like the particular use case here is to able to persist a state in
> the UI and I think that browser action might not be a good place for that,
> however it might be that there aren't many choices for doing that.

Why do you feel browserAction is not a good place for that?  It is useful for an "open-when-you-need-it-then-hide-it-away" UI, but since the window is created afresh every time it opens, certain data needs to be persisted in order for the user to pick up where they left off.

(I have actually considered filing a bug to explore the feasibility of simply hiding the popup rather than recreating it each time, much like XUL popups.  In certain applications this would be more efficient and would take care of the state persistence problem.)

Comment 8

[Shane Caraveo (:mixedpuppy)]

It seems that tabbrowser persists scroll position across restart (I just tried it).  I wonder if we can use the same mechanism for a panel browser.  I'd rather investigate that than support onbeforeunload.  There are code paths that check if a document has onbeforeunload, chasing those down to try to do something different may not be trivial (though I haven't looked).

Comment 9

[u462496]

(In reply to Shane Caraveo (:mixedpuppy) from comment #8)
> It seems that tabbrowser persists scroll position across restart (I just
> tried it).  I wonder if we can use the same mechanism for a panel browser. 
> I'd rather investigate that than support onbeforeunload.  There are code
> paths that check if a document has onbeforeunload, chasing those down to try
> to do something different may not be trivial (though I haven't looked).

What do you think about my thought in comment 7, of not tearing down the popup, but simply hiding it?  Perhaps could be opted in.

Comment 10

[Shane Caraveo (:mixedpuppy)]

(In reply to Kevin Jones from comment #9)
> (In reply to Shane Caraveo (:mixedpuppy) from comment #8)
> > It seems that tabbrowser persists scroll position across restart (I just
> > tried it).  I wonder if we can use the same mechanism for a panel browser. 
> > I'd rather investigate that than support onbeforeunload.  There are code
> > paths that check if a document has onbeforeunload, chasing those down to try
> > to do something different may not be trivial (though I haven't looked).
> 
> What do you think about my thought in comment 7, of not tearing down the
> popup, but simply hiding it?  Perhaps could be opted in.

I'm not convinced that people spend enough time in the panels to warrant keeping them around.  I might be slightly inclined towards a long delay before unloading (like maybe a minute).

IMO The panels should be for short/quick operations, not for long lived operations, but I'm sure someone somewhere will shoehorn an imap client into them.

Comment 11

[Firefox Bug Husbandry Bot]

Bulk move of bugs per https://bugzilla.mozilla.org/show_bug.cgi?id=1483958