panel-list fires its "shown" event before the XUL panel it is parented to is open
Categories
(Toolkit :: UI Widgets, task)
Tracking
()
People
(Reporter: florian, Unassigned)
References
Details
Attachments
(1 external file)
When a <panel-list> is used in a chrome window it is parented to a XUL <panel>. show() opens that panel asynchronously, from inside a requestAnimationFrame + setTimeout (see the bug 1842070 comment at panel-list.mjs:129-135), while onShow() fires sendEvent("shown") from its own requestAnimationFrame (panel-list.mjs:656-666).
The interleaving is deterministic and openPopup() is called first, but "shown" fires one animation frame after that call, whereas popupshown needs popupshowing, widget creation, positioning and the "showing" -> "open" state transition, which is many frames later.
So "shown" routinely fires while the wrapping XUL panel is still not open. Anything inside a popup that is not open is invisible to the accessibility APIs and not focusable, so a consumer that awaits "shown" and then clicks an item can click nothing. This was called a foot-gun in review, in D318044.
Two production consumers are affected, both XUL-panel-parented: SearchModeSwitcher.mjs:98 and migration-wizard.mjs:493. Everything else uses the popover path and is unaffected.
Proposed fix: in onShow(), when parentIsXULPanel(), await the parent panel's popupshown before firing "shown", resolving also on popuphidden and bailing out if this.open became false. onShow() is already async, and the class already adds a popuphidden listener on this.parentElement in addHideListeners().
This also needs an if (!this.open) return; guard at the top of the deferred openPopup() callback. There is a pre-existing race where hiding between show() and that setTimeout leaves hidePopup() a no-op on a still-closed panel, and then opens a panel whose panel-list has open == false; without the guard the new await can hang.
Fixing this at source lets us delete the test workarounds it forced, all added in bug 2062572: waitForPanelListShown() and its four call sites in browser/components/migration/tests/browser/ (head.js, browser_disabled_migrator.js, browser_file_migration.js), and the xulPanel.localName == "panel" wait in browser/components/urlbar/tests/UrlbarTestUtils.sys.mjs.
A hide-during-open case is worth adding to toolkit/content/tests/widgets/test_panel_list_in_xul_panel.html, since it is not confirmed from the code that popuphidden always fires for a popup hidden before it reached "open"
|
Reporter | |
Comment 1•2 months ago
|
||
A panel-list parented to a XUL <panel> asks that panel to open
asynchronously, but used to fire its "shown" event one animation frame
after show(), ie. typically while the panel was still in the "showing"
state. The contents of a popup that is not open are not focusable and
are invisible to the accessibility APIs, so consumers that wait for
"shown" before clicking a panel-item could be clicking nothing.
onShow() now waits for the parent panel's popupshown event before moving
the keyboard focus, setting aria-expanded on the anchor and firing
"shown". Each onShow() call has an AbortController that hide() aborts,
so a show cycle that has been superseded fires nothing: the panel it
asked to open may never fire popupshown, and hidePopup() on a panel that
isn't open yet fires no popuphidden event either.
show() also no longer opens the XUL panel if the panel-list was hidden
before its deferred openPopup() call ran: the hidePopup() call in hide()
is a no-op on a panel that isn't open yet, so the panel would otherwise
end up shown for a panel-list that no longer considers itself open.
The tests that worked around this by also waiting for the XUL panel's
popupshown event no longer need to.
Description
•