1686324 - Add a new mostly-empty panelmultiview to act as the stand-in for the new Proton AppMenu

Status: RESOLVED FIXED

(Firefox :: Menus, task, P1)

Description

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

While the designs solidify for the new AppMenu, we can get the foundations down for it by having a switch inside of panelUI.js, keyed off of the Proton pref, that will open up a separate menupopup.

We can probably put "New Window" and "New Private Window" menuitems in there, since we know those will almost certainly be in there. We should probably hold off on adding more menuitems until we have a clearer sense of what's in there.

(I'm assuming menupopup here, but this thing might end up being a PanelMultiView, pending design - so we might end up swapping it out eventually).

So here's where to get started! The AppMenu/hamburger button is defined here, and the code that manages it is here in panelUI.js. Note that there's one instance of panelUI.js running per window.

We're going to take advantage of the fact that XUL toolbarbuttons of with type="menu" set on them will open up the first menupopup that's defined as a child of them.

Here's how I recommend starting:

1. Add a new <menupopup> under the hamburger button toolbarbutton that contains a few <menuitem>'s, basically like this.
2. Inside of the panelUI.js init() function, add a lazy preference getter for browser.proton.enabled, like this one, but call it something like protonEnabled. The documentation for defineLazyPreferenceGetter is here. We're going to call into that lazy getter pretty much immediately, but we'll want it later on too, so I guess it's good to have.
3. Inside of init, if protonEnabled is true, set type="menu" on the menuButton element, which is the toolbarbutton for the AppMenu.
4. Inside of show(), if protonEnabled is true, just return immediately rather than opening the Photon panel.

If you've got that working, then we'll want to do a small optimization, which is to lazily populate the menupopup so that we don't have those menuitem nodes in the DOM until necessary. We can do that by moving each of those that <menuitem> nodes into a template like this, and then injecting those nodes in a popupshowing event handler on the <menupopup>. So to do that, I suggest:

0. Move the <menuitem>'s into a <html:template> like the example above, and give the <html:template> an ID.
1. Make sure there's an ID on the <menupopup>, and then get it via getElementById and add a popupshowing event handler somewhere in here.
2. In the handleEvent method for popupshowing here, check if this.protonEnabled is true. If so, get the <html:template> by ID, and for each item in the template, move them into the menupopup (via menupopup.appendChild). Then bail out immediately.

I think that'll give us the startings of something. This is mostly off of the top of my head, but if we can get that menupopup showing some items if browser.proton.enabled is true, and showing the old panel if browser.proton.enabled is false, then I think we'll be in good shape.

Comment 1

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

I think you won't need to add a lazy preference getter, and instead can use the already-defined gProton https://searchfox.org/mozilla-central/rev/07342ce09126c513540c1c343476e026cfa907bf/browser/base/content/browser.js#559

Also, instead of appending each child of the template, you should be able to do element.appendChild(template.content.cloneNode(true)); which will append all of them with one call.

Comment 2

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

Record scratch - UX confirmation has come in saying we are not using fold-out menus. We're going to stick with slide-y panels.

So I'll update my suggested approach in an upcoming comment - but we should halt the menupopup effort.

Comment 3

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

Okay, so the good news here is that we don't need to shoehorn a different menu technology (<toolbarbutton type="menu"> with a <menupopup>) into another one (<panelview>), so this might be a lot easier.

jaws is right about the gProton pref - I forgot we had that one. Let's use that instead.

So what we want to do is add a new entry to the appmenu-viewCache template here. In particular, we want to add a (very) simple <panelview>. You can see the current "main view" (the default contents of the AppMenu) defined here.

Create a new <panelview> inside of the appmenu-viewCache, and put a <vbox class="panel-subview-body"> like this one into it, and then put some toolbarbuttons like these into it.

Then, over in panelUI.js, switch on gProton here in the mainView getter, and have it return the ID for your new template entry. Maybe give your new <panelview> an ID like appMenu-protonMainView or something for now, and have the mainView getter refer to that.

Comment 4

[Molly Howell (she/her) (mostly inactive)]

Attachment: Bug 1686324 - Introduce a temporary Proton AppMenu skeleton. r=mconley

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b1cac3b3ffd0
Introduce a temporary Proton AppMenu skeleton. r=mconley

Comment 6

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/b1cac3b3ffd0