813808 - Show menubar in the titlebar when it's visible

Status: RESOLVED DUPLICATE of bug 813802

(Firefox :: Theme, enhancement)

Description

[Matthew N. [:MattN]]

There are some cases where we show the menubar on its own row:  
* restored windows
* when auto-hiding the menubar (after pressing alt when the menubar is hidden by default)

To simplify the designs, we should always show the menubar in the titlebar. As a side benefit, we will take less height away from content when the menubar is shown.

Comment 1

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

I can take this one.

Comment 2

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

Attachment: Mockup

Hey Matt - tell me if I'm understanding this correctly;

There's really only one case where we're changing behaviour - when the menubar is set to autohide="true", and the window is in the restored state. In that case, we want to draw the menubar in the titlebar, as in the mock-up.

For the case where the menubar is not autohidden, am I correct in assuming that we'll want to show the menu in its own row?

Comment 3

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

Hm - so, looking a bit closer at the Australis specs, when a window is in the restored state, we only get about 16px's of clearance for menuitems:

http://people.mozilla.com/~shorlander/files/australis-designSpecs/images-win7/padding-mainWindow.png

Current <menu> nodes for Windows have a height of 20px, before margins.

So we're going to be a bit squished. Is the plan to insert the menu into the titlebar on restored windows still applicable once the menu moves from the top left to the toolbar, and we're left with 16px of headroom?

Comment 4

[u428464]

Mike I think this mockup could be useful to you : http://people.mozilla.com/~shorlander/files/australis-design-specs/images/Australis-i01-DesignSpec-MainWindow-%28MenuBar%29.jpg

Comment 5

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

(In reply to Guillaume C. [:ge3k0s] from comment #4)
> Mike I think this mockup could be useful to you :
> http://people.mozilla.com/~shorlander/files/australis-design-specs/images/
> Australis-i01-DesignSpec-MainWindow-%28MenuBar%29.jpg

Ah, very much so. Thank you.

Comment 6

[Matthew N. [:MattN]]

(In reply to Mike Conley (:mconley) from comment #2)
> For the case where the menubar is not autohidden, am I correct in assuming
> that we'll want to show the menu in its own row?

I would prefer to simplify the CSS and have it one the same row when it's not auto-hidden.  This also makes the design simpler because you don't have to worry about an extra row (of 20px?) showing up and affecting things like the fog.  Do you prefer having the menu on its own row?

In case you haven't already found them, you will probably need to use the spacers/placeholders with class .titlebar-placeholder.

(In reply to Mike Conley (:mconley) from comment #3)
> Hm - so, looking a bit closer at the Australis specs, when a window is in
> the restored state, we only get about 16px's of clearance for menuitems:
> 
> http://people.mozilla.com/~shorlander/files/australis-designSpecs/images-
> win7/padding-mainWindow.png
> 
> Current <menu> nodes for Windows have a height of 20px, before margins.
> 
> So we're going to be a bit squished. Is the plan to insert the menu into the
> titlebar on restored windows still applicable once the menu moves from the
> top left to the toolbar, and we're left with 16px of headroom?

I've been meaning to ask shorlander about the 16px as it would also mean that tabs would collide with the button controls. Otherwise, we'd have dead space below the window controls.  I personally would like to have the tabs fit below the window controls which would give enough room for the menu bar as well IIRC.

Comment 7

[Stephen Horlander [:shorlander]]

(In reply to Matthew N. [:MattN] from comment #6)
> (In reply to Mike Conley (:mconley) from comment #3)
> > Hm - so, looking a bit closer at the Australis specs, when a window is in
> > the restored state, we only get about 16px's of clearance for menuitems:
> > 
> > http://people.mozilla.com/~shorlander/files/australis-designSpecs/images-
> > win7/padding-mainWindow.png
> > 
> > Current <menu> nodes for Windows have a height of 20px, before margins.
> > 
> > So we're going to be a bit squished. Is the plan to insert the menu into the
> > titlebar on restored windows still applicable once the menu moves from the
> > top left to the toolbar, and we're left with 16px of headroom?
> 
> I've been meaning to ask shorlander about the 16px as it would also mean
> that tabs would collide with the button controls. Otherwise, we'd have dead
> space below the window controls.  I personally would like to have the tabs
> fit below the window controls which would give enough room for the menu bar
> as well IIRC.

In the current configuration with the button in the titlebar there isn't a problem.

Once the button is on the toolbar can we slide the rest of the UI down to make room? Mockup: http://cl.ly/image/423u0z1O0c3x

Comment 8

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

(In reply to Matthew N. [:MattN] from comment #6)
> Do you prefer having the menu on its own row?
> 

Nope, I like simplifying. Let's keep with the current design; when the menubar is not autohidden, it just "stays" in the window titlebar, as opposed to getting its own super-special row.

> In case you haven't already found them, you will probably need to use the
> spacers/placeholders with class .titlebar-placeholder.
> 

Yep.

Can I assume all of this behaviour will be the same for XP as well?

Comment 9

[Matthew N. [:MattN]]

(In reply to Mike Conley (:mconley) from comment #8)
> Can I assume all of this behaviour will be the same for XP as well?

I haven't heard otherwise and I don't see why not.

Comment 10

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

Attachment: WIP Patch 1

Here's how I'm approaching this problem:

1) Menubar toolbars with autohide set to true completely collapse when inactive. This is something that xul.css sets[1], and I didn't want to monkey around with it. So, instead, I do a little bit of sleight-of-hand, and change the margin-bottom of the titlebar when the menubar is shown.

2) I detect when the menubar is active by using a Mutation Observer. I saw that "DOMMenuBarActive" and "DOMMenuBarInactive" events existed, but experimentation showed that they were a little buggy when switching from menu to menu. Maybe that's because those events are also being fired by child elements of the menubar - I'm not sure. There's not a whole lot of documentation on DOMMenuBarActive nor DOMMenuBarInactive, so I went with the more familiar Mutation Observer. Let me know if that's cool.

2) I changed some CSS logic in browser/base/content/downloads.css so that we never hide .titlebar-placeholders for the menubar.

3) I haven't changed any theme stuff for the menubar yet.

4) This patch is pretty fugly on XP - beware. 

Does this look like a reasonable approach?

[1] https://mxr.mozilla.org/mozilla-central/source/toolkit/content/xul.css#290

Comment 11

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

Comment on attachment 687216 [details] [diff] [review]
WIP Patch 1

Dao - do you have any thoughts on this? Does the MutationObserver look like the way to go here?

Comment 12

[Matthew N. [:MattN]]

Comment on attachment 687216 [details] [diff] [review]
WIP Patch 1

(In reply to Mike Conley (:mconley) from comment #10)
> Created attachment 687216 [details] [diff] [review]
>
> 1) Menubar toolbars with autohide set to true completely collapse when
> inactive. This is something that xul.css sets[1], and I didn't want to
> monkey around with it. So, instead, I do a little bit of sleight-of-hand,
> and change the margin-bottom of the titlebar when the menubar is shown.

This is unfortunate for restored windows where using visibility:hidden should avoid the need for a mutation observer.  After looking into this and bug 813802, I think this can build on top of that and so this bug should wait for that.

> [1]
> https://mxr.mozilla.org/mozilla-central/source/toolkit/content/xul.css#290

Comment 13

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

(In reply to Matthew N. [:MattN] from comment #12)
> This is unfortunate for restored windows where using visibility:hidden
> should avoid the need for a mutation observer.  After looking into this and
> bug 813802, I think this can build on top of that and so this bug should
> wait for that.

Okie doke.

Comment 14

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

Comment on attachment 687216 [details] [diff] [review]
WIP Patch 1

I'm pretty sure the approach in here has been completely obliterated by the patch in bug 813802 (probably for the best).