1921819 - Revamped sidebar doesn't play well with macOS toolbar slide-down animation during full-screen

Status: RESOLVED MOVED

(Firefox :: Toolbars and Customization, defect, P2)

Description

[Emilio Cobos Álvarez (:emilio)]

Attachment: Screenshot

Right now it's even more broken due to bug 1921811, but even before that regression it looks really odd, because we shift the toolbox down but not the sidebar, see screenshot.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

STR is:

• sidebar.revamp = true
• Go into macOS full-screen (click green button in the titlebar)
• Move the mouse to the top of the screen.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Apparently on latest macOS the expectation is that the buttons remain taking space, and there is extra shadow around the header while the menubar is shown... Should we do something like that? It'd certainly make stuff simpler... Maybe

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Apparently both chrome and safari do that kind of "titlebar buttons show up without shifting everything".

Comment 4

[Emilio Cobos Álvarez (:emilio)]

(Even on sonoma)

Comment 5

[Sarah Clements [:sclements]]

Sam might also have some thoughts on this, since he worked on bug 1899598 which hasn't landed yet.

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Yeah I ni?d sam in bug 1921811 which is fairly related.

Comment 7

[Markus Stange [:mstange]]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #3)

Apparently both chrome and safari do that kind of "titlebar buttons show up without shifting everything".

Oh, it's interesting that Chrome doesn't have the extra titlebar here. I was under the assumption that the only way to get rid of the extra bar would be to have an NSToolbar in the window, and to be able to slide the toolbar contents over to the right to make space for the window buttons. It seems Chrome found a way to make things look better without having an NSToolbar. But they reserve space for the buttons and don't slide anything to the right, unlike Safari.
Oh, and native apps with sidebars always have a full-height sidebar, and then it's that sidebar space that is the reserved space for the toolbar buttons.

I don't have a strong opinion on what to do in Firefox. But if we want to get rid of the slide-down-empty-titlebar, that'll require some widget work.

Comment 8

[Markus Stange [:mstange]]

It looks like Chrome is still tweaking what they're doing. I found https://chromium-review.googlesource.com/c/chromium/src/+/5837702 from yesterday.

Comment 9

[Stuart Colville [:muffinresearch]]

Ni'ing Gijs for visibility and input.

Comment 10

[Sam Foster [:sfoster] (he/him)]

I'm not sure I have much to add here. The proposed plan in bug 1921811 to re-cast the url bar results container as a popover seemed reasonable to me, and IIUC that would open up new options for this bug?

As far as bug 1899598 goes, the important thing that changes is that the first visible toolbar can function as the titlebar, and when you hide the tabstrip toolbar, that becomes the #nav-bar. The z-index thing came up there as when the #nav-bar is a titlebar, it gets a opacity property when the window its in becomes inactive. That means we get a new stacking context, which means the URL bar results element within would get clipped. We didn't have this problem before because the only popup/z-indexed elements you could create from the tabstrip toolbar were menus and tooltips.

Comment 11

[:Gijs (out for now; he/him)]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #2)

Apparently on latest macOS

Please can we clarify which version this relates to? (Is it Sequoia?) And does this problem only occur there, or also on earlier versions?

the expectation is that the buttons remain taking space, and there is extra shadow around the header while the menubar is shown... Should we do something like that? It'd certainly make stuff simpler... Maybe

I'm sorry but I can't follow. Which buttons/shadow/header is this talking about? :-)

A screencast would probably help.

Based on the conversation here it's not entirely clear to me if this needs a widget/cocoa fix or if there is something for frontend to do here - clearing that up would also be helpful. Apologies if I've missed other conversations about this outside of this bug with Nikki who is now assigned to this?

Comment 12

[Emilio Cobos Álvarez (:emilio)]

(In reply to :Gijs (he/him) from comment #11)

(In reply to Emilio Cobos Álvarez (:emilio) from comment #2)

Apparently on latest macOS

Please can we clarify which version this relates to? (Is it Sequoia?) And does this problem only occur there, or also on earlier versions?

This problem happens on any macOS version, because we do the same "shift the toolbar down" animation everywhere.

On older macOS version that was the usual thing to do (IIUC). It seems at least on Sequoia (but Chrome at least also does it on older macOS versions) the expectation is that the titlebar keeps taking space, and the buttons either fade in or shift in. Which would solve this.

I'm sorry but I can't follow. Which buttons/shadow/header is this talking about? :-)

This is the window buttons (as in, the semaphore buttons for close / minimize / etc).

A screencast would probably help.

Will attach some screencasts (of Sequoia only because that's the only thing I have available, but someone checked that Chrome at least does the same on Sonoma).

Based on the conversation here it's not entirely clear to me if this needs a widget/cocoa fix or if there is something for frontend to do here - clearing that up would also be helpful. Apologies if I've missed other conversations about this outside of this bug with Nikki who is now assigned to this?

This would need some widget/cocoa work at least (to prevent the empty titlebar from showing in), see comment 7. Not sure how trivial to do that would be, but it seems we can probably save some time by looking at what Chromium is doing.

Comment 13

[Emilio Cobos Álvarez (:emilio)]

Attachment: Current Firefox animation.

Comment 14

[Emilio Cobos Álvarez (:emilio)]

Attachment: Current Chrome animation.

Comment 15

[Emilio Cobos Álvarez (:emilio)]

Attachment: Current Safari animation.

Comment 16

[Emilio Cobos Álvarez (:emilio)]

Attachment: Nightly animation with revamped sidebar.

Comment 17

[Emilio Cobos Álvarez (:emilio)]

Do the screencasts above help?

Comment 18

[:Gijs (out for now; he/him)]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #17)

Do the screencasts above help?

Yes. But I confess I'm still a bit lost.

In particular, ISTM that there is a "simple" fix here which is to also adjust the top coordinate of the sidebar (ie make that shift down when the animation happens), which doesn't need any platform support and would keep parity with the current animation, and would (IMO) look non-terrible, even if it's not quite the "perfect" solution from a macOS integration PoV for Sequoia.

Then there is updating all of the behaviour for Sequoia, which would also solve the sidebar issue (because the navbar would not move) but needs platform support. Given the need for platform support I'd expect that we may not be able to apply this fix on earlier versions of macOS (depending on when the relevant APIs are available).

Is that right? Am I missing something (e.g. the first fix being much harder than I'm making out and/or the second one being easier + more generally applicable)? If not, would it make sense to split these two efforts off so they are 2 bugs instead of 1? :-)

Comment 19

[Nikki Sharpley (:nikkis) (she/her)]

I think Emilio covered most of it! But I do think :Gijs 's suggestion to just translate the sidebar down warrants some more investigation. Before we look into changing the navbar to behave more like chrome or safari on fullscreen macs, I'll see if I can prototype something here. Stay tuned.

Comment 20

[Emilio Cobos Álvarez (:emilio)]

(In reply to :Gijs (he/him) from comment #18)

Is that right? Am I missing something (e.g. the first fix being much harder than I'm making out and/or the second one being easier + more generally applicable)? If not, would it make sense to split these two efforts off so they are 2 bugs instead of 1? :-)

Yes, the first fix happens to be very tricky because with sidebar.revamp=true, since on that configuration the content separator (and shadow) is drawn from the tabbox. Which means that the tabbox should paint on top of the toolbox and sidebar.

So keeping doing what we're doing happens to be somewhat tricky because it either breaks the content separator (which is currently broken in nightly already), or it makes content overlap on top of the toolbox (which would be undesirable).

Another alternative is reconsidering the shadow separator to make it part of the sidebar and toolbox instead, but I don't see how to make that work easily either.

Comment 21

[Emilio Cobos Álvarez (:emilio)]

I might be missing something, please correct me if that's the case :)

Comment 22

[Nikki Sharpley (:nikkis) (she/her)]

Attachment: WIP: Bug 1921819 - Adjust sidebar to respond to fullscreen macos slide down animation

This is a WIP. It unclear if it would affect performance using this method.

Comment 23

[Nikki Sharpley (:nikkis) (she/her)]

:emilio, I'm not sure if the WIP patch in comment 22 would affect performance, but this is what it would look like if we only shift the sidebar down. It does look a little funky because we lose the top shadow for the tabbrowser-tabbox, but perhaps that's solveable with adding a bottom shadow to the navigator-toolbox upon translate, similar to Chrome and Safari? I'll attach a screenshare.

Comment 24

[:Gijs (out for now; he/him)]

(In reply to Nikki Sharpley (:nikkis) (she/her) from comment #19)

I think Emilio covered most of it! But I do think :Gijs 's suggestion to just translate the sidebar down warrants some more investigation. Before we look into changing the navbar to behave more like chrome or safari on fullscreen macs, I'll see if I can prototype something here. Stay tuned.

Emilio pointed out to me (which I hadn't realized, sorry) that the border/overlap problem also applies to the main web content, not just the sidebar - right now the border between web content and the toolbar is generated from https://searchfox.org/mozilla-central/rev/7a85a111b5f42cdc07f438e36f9597c4c6dc1d48/browser/themes/shared/tabbrowser/content-area.css#51-55 . It's not visible on Nightly atm but it becomes more obvious if you apply Nikki's patch in https://phabricator.services.mozilla.com/D224201#change-OayWqRyT5dTQ .

So if we move the toolbox down, it ends up "on top of" that and we lose the border. Moving the sidebar down solves the sidebar but not the web content side of things.

We can generate a border on the entirety of the moving nav toolbox but then that border overlaps the sidebar which looks odd if the sidebar also moves.

Looking at other macOS apps on Sonoma, oddly Calendar.app already does the same thing as Chrome on Sequoia: it fades/moves the titlebar buttons inside the main toolbar.

On the other hand, Music.app overlaps its own sidebar with the toolbar content and has a shadow along the entire edge (also on top of the sidebar).

So I see at least the following 3 options:

1. move everything. Nobody else does this and it's kind of my lazy idea but hear me out: this is a 1-line patch on top of https://phabricator.services.mozilla.com/D224201 (just put the translate on gNavToolbox.parentElement instead of gNavToolbox). Then web content, the sidebar, the toolbox - everything moves to make space for the titlebar. And sure, it's kinda weird - but the whole point of this interaction is that the user wants the menubar / titlebar. AFAIK there's no way to "persist" with this when not hovering the top of the window, so presumably it's not actually bad that some number of pixels get cut off at the bottom of the screen.
2. move the toolbox as we do now and swap over the bottom border to be a shadow along the toolbox, which overlaps web content and the sidebar (a la Music.app). This is more work but should be doable: we'd put shadow CSS back on the gNavToolbox for this situation based on some selector and then that would show the overlap on top of the sidebar and web content. Not ideal but matches Music.app so not awful, other people did this first. This would also match the non-sidebar-revamp state, where the toolbox already overlaps the sidebar.
3. do the Sequoia / Calendar.app thing and fade the titlebar buttons in/out instead of any vertical translation. This would require some work on the widget side in order to apply opacity to the titlebar buttons, and to not have the animating titlebar on top of us, but other than that we could potentially do this in frontend. Another wrinkle would be toolbar overflow in this situation, and making the animation smooth. So this definitely seems the most complex.

I would suggest settling on (1) or (2) for 133 and have (3) as some kind of stretch goal; it's not clear to me how nice it'd be in the toolbar overflow situation. Calendar.app doesn't have this problem and we do...

Comment 25

[:Gijs (out for now; he/him)]

(argh, sorry - I hadn't seen the earlier patch and comment from Nikki before posting my essay...)

Comment 26

[Nikki Sharpley (:nikkis) (she/her)]

:Gijs, you put it more eloquently! :)

My patch is sort of a spin on option 1. I like the idea of making everything shift down, rather than just the sidebar as I've done, but I think we'd need to change the height of the browser element too, no? Option 2 was my other thought as well! I don't think it would be a considerable amount of effort to add the box-shadow on the gNavToolbox (famous last words!)

Agreed, 3 involves far more work. We can come back to it, but either 1 or 2 is best for now.

I guess the question is, who makes the call? Do we need to loop someone from UX in?

Comment 27

[Emilio Cobos Álvarez (:emilio)]

We shouldn't change heights during the animation, that'd make it jank quite a bit.

Comment 28

[Emilio Cobos Álvarez (:emilio)]

(In reply to :Gijs (he/him) from comment #24)

Another wrinkle would be toolbar overflow in this situation, and making the animation smooth. So this definitely seems the most complex.

I'm not sure I get this, what's the issue with toolbar overflow / making the animation smooth? If we fade-in the titlebar buttons from widget code, I think this is literally zero work for the front-end? Or am I missing something? I think it's worth at least trying this out, fwiw, I don't think it'll end up being that much widget work.

Were you talking about a potential slide-in animation or so? If so I'd agree that seems like more work. But I don't think that's required to avoid the shift.

Comment 29

[:Gijs (out for now; he/him)]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #28)

(In reply to :Gijs (he/him) from comment #24)

Another wrinkle would be toolbar overflow in this situation, and making the animation smooth. So this definitely seems the most complex.

I'm not sure I get this, what's the issue with toolbar overflow / making the animation smooth? If we fade-in the titlebar buttons from widget code, I think this is literally zero work for the front-end?

Well, if we always have the buttons take up space, we'd need to always include the titlebar buttons in the navbar in fullscreen, and apply some opacity and visibility CSS to fade in/out depending on menubar/titlebar hover state etc., so not sure I agree with "literally zero", but it's certainly not a lot of work, comparatively speaking, if we can update the widget code to honour opacity on these buttons. (I think we need visibility as well so that the buttons don't "work" while hidden via opacity, assuming that works for the native buttons...)

But also, in this proposal AIUI the buttons always take up space so that kind of begs the question of why we wouldn't always show them rather than showing an empty space until you hover the menubar? What's the point of the empty space?

Were you talking about a potential slide-in animation or so? If so I'd agree that seems like more work. But I don't think that's required to avoid the shift.

I was talking about having the buttons not take up space when not hovering the toolbox/titlebar, and then take up space once the menubar is hovered until the mouse leaves the navbar+menubar+titlebar again. And yes, Calendar.app also has a slide-in/out animation between those two states. That (with or without a slide-in animation) would change the amount of space available for everything else in the navbar and thus trigger toolbar overflow/underflow, potentially moving widgets into the overflow panel etc. Which IMO would look messy - the opacity fade I'm sure could be smooth but moving the widgets isn't...

Does that help clarify? :-)

Comment 30

[Emilio Cobos Álvarez (:emilio)]

But also, in this proposal AIUI the buttons always take up space so that kind of begs the question of why we wouldn't always show them rather than showing an empty space until you hover the menubar? What's the point of the empty space?

Mostly consistency with the other apps. But sure we could also just do that (suppress the current effect, always show the buttons), then figure out if we want something fancier (either an opacity animation, or an slide-in animation) in a follow-up.

I was talking about having the buttons not take up space when not hovering the toolbox/titlebar, and then take up space once the menubar is hovered until the mouse leaves the navbar+menubar+titlebar again.

Yeah, that seems like a bit of a pain to do smoothly.

Does that help clarify? :-)

Yes, thanks!

I guess the main question is which path forward to go:

1. Keep the current effect, slide in the whole <body> (comment 24): Somewhat trivial, but not what any other apps do.
2. Keep the current effect, rejigger stuff to add border / shadow to the toolbox while animating only (keep overlapping the sidebar).
3. Remove the current shift altogether (unfix bug 1699506): Also trivial of course. Maybe worth considering? This gets us the Music.app behavior on my machine.
4. Suppress the current effect, always show the titlebar buttons in fullscreen (I think this is what chrome plans to do in the link for comment 8).
5. Suppress the current effect, do an opacity animation to fade them in (comment 14).
6. Suppress the current effect, do a more complex animation to slide them in (comment 15,): As noted above, quite a few technical challenges.

I think we should discard (6) altogether for now at least for the reasons you mentioned.

Should we ask UX? My gut feeling is that in terms of complexity (simplest to hardest) we have 3, 1, 4, 5, 2 (where 4 and 5 need some widget work).

In terms of personal preference (in terms of the effort to get it working vs. maintainability) I think I'd go: 4/5, 1, 2, 3. But I don't feel too strongly.

Comment 31

[Sam Foster [:sfoster] (he/him)]

I'm marking as P2. I think this still needs improvement but is workable for now.

Comment 32

[Nikki Sharpley (:nikkis) (she/her)]

Attachment: Bug 1921819 - Prevent navigator toolbox from translating on MacOs fullscreen r=#sidebar-reviewers

Comment 33

[Nikki Sharpley (:nikkis) (she/her)]

Update as per https://mozilla.slack.com/archives/C07H21R792A/p1728502631161479

• For 133, we landed on a temporary fix in which we remove the translate from the navigator toolbox so that only the buttons translate in.
• Post-133, we have a few ideas hightlighted above and in the Slack thread that involve more widget work, such as sliding the OS buttons into the nav bar

Comment 34

[Sarah Clements [:sclements]]

(In reply to Nikki Sharpley (:nikkis) (she/her) from comment #33)

Update as per https://mozilla.slack.com/archives/C07H21R792A/p1728502631161479

• For 133, we landed on a temporary fix in which we remove the translate from the navigator toolbox so that only the buttons translate in.
• Post-133, we have a few ideas hightlighted above and in the Slack thread that involve more widget work, such as sliding the OS buttons into the nav bar

Is there a follow-up bug on file or do we need one?

Comment 35

[:Gijs (out for now; he/him)]

(In reply to Sarah Clements [:sclements] from comment #34)

(In reply to Nikki Sharpley (:nikkis) (she/her) from comment #33)

Update as per https://mozilla.slack.com/archives/C07H21R792A/p1728502631161479

• For 133, we landed on a temporary fix in which we remove the translate from the navigator toolbox so that only the buttons translate in.
• Post-133, we have a few ideas hightlighted above and in the Slack thread that involve more widget work, such as sliding the OS buttons into the nav bar

Is there a follow-up bug on file or do we need one?

Filed bug 1925257.

Comment 36

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1115fdcb7cef
Prevent navigator toolbox from translating on MacOs fullscreen r=sidebar-reviewers,desktop-theme-reviewers,Gijs,dao

Comment 37

[agoloman]

https://hg.mozilla.org/mozilla-central/rev/1115fdcb7cef

Comment 38

[Markus Stange [:mstange]]

(In reply to Nikki Sharpley (:nikkis) (she/her) from comment #33)

• For 133, we landed on a temporary fix in which we remove the translate from the navigator toolbox so that only the buttons translate in.

I don't really understand - it's not just the buttons that translate in. The buttons come in a gray bar that covers up the top part of the window. And this patch doesn't do anything about the gray bar. So now the bar covers the tabs or buttons at the top of the window. I've filed bug 1926120.

Comment 39

[Monica Chiorean]

Shouldn't the status be open as the fix was reverted based on bug#1926120? and the issue still reproduces on Mac 12.6 using beta 133.0b9 and nightly 134.0a1. Thank you.

Comment 40

[Nikki Sharpley (:nikkis) (she/her)]

Thanks Monica. We can keep this as resolved as we filed follow up bug 1925257 and I'll update the status to moved.