[popup] Consider to dispatch show event as the first step of showPopup

[smaug----]

Currently there is

1. Move the pop-up to the top layer, and remove the UA display:none style.
2. Fire the show event, synchronously.

I'd expect show event to be cancelable (though such behavior could be added later too, assuming firing show is the first step).
Is there any reason for the current behavior?

@emilio

[mfreed7]

Thanks for the comment. In a practical sense, I don't think it would matter if we reversed the steps there, because only step 3 does any rendering. It would be an observable change, of course, but what I'm saying is that I'd be open to that change.

We've discussed the cancelability of the show/hide events in the past, e.g. here, and the conclusion we came to was that it ends up being a footgun. Practically, I think we were mostly concerned about the hide event though. Perhaps the show event is less of a footgun? What's the use case?

[smaug----]

It is partially an implementation detail why I'd prefer to fire event first and only after it change styling. There are cases where JS may spin event loop in Gecko, and I'd rather not change style right before that.

But also, based on the experience with popupshowing, I think making show cancelable is quite reasonable.
In XUL popupshowing event is cancelable. Some use cases seem to be:
tooltips, one may want to try to trigger a tooltip like popup as a reaction to something, but perhaps it is only the event listener itself which knows whether there is something to show. (I think something like that is done in 'Manage Bookmarks' code in Firefox)
And looks like similar thing for context menu type of case. Something triggers the popup, but only the listener code knows whether there is anything to show. (the code I'm looking at checks some preference value in the listener whether to show context menu)

[mfreed7]

Thanks for the cancelable use case examples. They seem reasonable. And changing show to be cancelable is easy enough, so I'm inclined to just do that. Agenda+ to discuss that.

@smaug---- would you agree it's ok to keep hide as non-cancelable, to avoid footgun-y interactions with the one-at-a-time and light-dismiss behaviors? One use case does seem to be making a "light dismissible dialog" which asks users for information, yet wants to be able to close in most cases if you click outside the dialog. But it also might want to confirm before closing in case you've changed something and want to save it or avoid data loss. That seems reasonable. My fear is that developers will be too tempted to try to manage the very things pop-up provides in the way of nested pop-ups, and interactions between popup=auto and popup=hint, for example. I'm torn.

[smaug----]

(XUL popuphiding is cancelable, but I couldn't find any case where the event is actually cancelled in FF code.)

I'm ok leaving hide as non-cancelable. I think it has fewer use cases and perhaps, if needed, it can be changed later.

[mfreed7]

Good point that it can be changed later. Ok, thanks.

For reference, I implemented (in Chromium) the change to "show", and didn't find any issues. I also implemented the change to "hide", but there is some trickiness to the ordering when you have a stack of pop-ups. So yes it'd be nice to tackle this later, if the use case arises.

[css-meeting-bot]

The Open UI Community Group just discussed [popup] Consider to dispatch show event as the first step of showPopup, and agreed to the following:

• RESOLVED: Make the "show" event cancelable and make it the first step of `showPopUp()`, but leave the "hide" event non-cancelable.

The full IRC log of that discussion

<masonf> Topic: [popup] Consider to dispatch show event as the first step of showPopup
<masonf> github: https://github.com//issues/579
<bkardell_> masonf: I think this is a short one.
<bkardell_> masonf: this was an issue raised around show/hide event and whether they are cancellable - the main request was first to move the show earlier - it didn't make any diff in the chromium implementation so I did it
<bkardell_> masonf: the second was about making it cancellable, I wanted to talk about it
<bkardell_> masonf: the other was about whether hide should be cancellable - we decided previously that this is a foot gun and should _not_ do that - it sort of undoes all the goodness here
<masonf> q?
<bkardell_> masonf: if you want the things that would enable, you can use popup = manual --- I think we could make show cancellable, but not hide
<bkardell_> masonf: thoughts?
<masonf> q?
<bkardell_> {awkward silence}
<masonf> Proposed resolution: Make the "show" event cancelable and make it the first step of `showPopUp()`, but leave the "hide" event non-cancelable.
<bkardell_> seems reasonable to me
<emilio> +1
<bkardell_> scotto_: I don't have strong opinions, seems gine
<bkardell_> s/gine/fine
<bkardell_> dbaron: are there cases where you want _most_ of the behaviour but somehow still want to cancel hide?
<bkardell_> masonf: it's a complex series of things, and it seems very difficult to unwind all of the sequence of behaviours - stacks of popups - and make it possible to do that
<bkardell_> masonf: if you have nested popups, and you cancel, what happens? it gets very very tricky even with 2
<bkardell_> dbaron: yeah, ok
<masonf> q?
<bkardell_> masonf: any objections?
<bkardell_> masonf: resolved
<masonf> RESOLVED: Make the "show" event cancelable and make it the first step of `showPopUp()`, but leave the "hide" event non-cancelable.