861702 - PanelUI sub-view container can end up showing multiple views

Status: RESOLVED FIXED

(Firefox :: Toolbars and Customization, defect)

Description

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Trying to open both the subviews we have at the moment ends up messing up the display, showing two subviews. Turns out the history subview doesn't include class="PanelUI-subView", which is needed to hide it.

That's the only time we use that class, and its not obvious what the consequences are if left out. We don't actually need it - all we're using it for is to match all children of PanelUI-subViews.

Comment 1

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Attachment: Patch v1

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 737316 [details] [diff] [review]
Patch v1

Review of attachment 737316 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/themes/shared/panelUIOverlay.inc.css
@@ +85,5 @@
>    transform: translateX(@menuPanelWidth@);
>    transition: transform 150ms;
>  }
>  
> +#PanelUI-subViews > *:not([current]) {

The seems like it would be pretty bad for performance since it would first retrieve all elements in the DOM and then check their parent. Could buildWidget just add the class?

Comment 3

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Ugh, yes. Must remember selectors are read RTL.

Comment 4

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Attachment: Patch v1.1

Comment 5

[Blair McBride [:Unfocused] (UNAVAILABLE)]

https://hg.mozilla.org/projects/jamun/rev/78acb3457fe4

Comment 6

[Mike Conley (:mconley) (:⚙️)]

https://hg.mozilla.org/projects/ux/rev/1b5f8fb0a275

Comment 7

[:Gijs (he/him)]

https://hg.mozilla.org/mozilla-central/rev/1b5f8fb0a275