Open Bug 1438507 Opened 3 years ago Updated 2 years ago

Make PanelMultiView events more consistent

Categories

(Firefox :: Toolbars and Customization, enhancement, P3)

59 Branch
enhancement

Tracking

()

People

(Reporter: Paolo, Unassigned)

References

Details

This came up in bug 1434883, and is something we could consider for future clean up work, although it does not block the asynchronous description height workaround.

We could wrap the entire panel opening operation in PanelMultiViewShowing and PanelMultiViewHidden events that are always called in pairs, and can be used to control the anchor or other states. Multiple calls to openPopup would result in only one PanelMultiViewShowing event. So the process would look like:

 - PanelMultiViewShowing
 - ViewShowing (can delay or cancel the operation)
    - popupshowing
    - popupshown and ViewShown (in any order)
    - popuphiding and ViewHiding (in any order)
    - popuphidden
 - ViewHidden
 - PanelMultiViewHidden

Note that there is currently code relying on ViewHiding being fired even if ViewShowing cancels the operation. It makes sense to have an event that behaves like this, as there may be multiple ViewShowing event handlers and the cancellation may be triggered by a different handler. However, this event should probably be ViewHidden, while ViewHiding should be called only if ViewShown was called.

The order of popuphiding and ViewHiding can't be guaranteed because we would likely have to call ViewHiding from one of the popuphiding handlers. The same goes for popuphidden and ViewHidden when they are called as a result of the panel being hidden, rather than the operation being canceled.
Depends on: 1439358
From bug 1440358 comment 11:

Comment on attachment 8953856 [details]
Bug 1440358 - Part 2 - Add tests for cancellation while opening a view.

https://reviewboard.mozilla.org/r/223042/#review229014

::: browser/components/customizableui/test/browser_PanelMultiView.js:375
(Diff revision 1)
> +  stopRecordingEvents(gPanels[0]);
> +
> +  Assert.deepEqual(recordArray, [
> +    "panelview-0: ViewShowing",
> +    "panelview-0: ViewHiding",
> +    "panelmultiview-0: popuphidden",

Why doesn't the `popuphidden` event fire on the panel/popup, like it does elsewhere?

Also, why doesn't the `PanelMultiViewHidden` event fire? Could any consumers be confused by this?

::: browser/components/customizableui/test/browser_PanelMultiView.js:444
(Diff revision 1)
> +    "panelview-0: ViewShowing > panelmultiview-0: popuphidden",
> +    "panelview-0: ViewShowing > panelview-0: ViewHiding",

Why does the `popuphidden` event happen *before* the `ViewHiding` event, instead of vice versa, like elsewhere?
Depends on: 1440358
Depends on: 1484275
You need to log in before you can comment on or make changes to this bug.