933933 - OS X titlebar buttons should not jump during customize mode transition

Status: RESOLVED WORKSFORME

(Firefox :: Toolbars and Customization, defect)

Description

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

Since bug 851652 landed, the OS X titlebar buttons are getting moved during the customize mode transition, along with the tabs, nav-bar, et al. I don't think we want that.

Comment 1

[:Gijs (he/him)]

I wonder if we could/should just only take customize mode into account for Windows (and maybe Linux) titlebars. Alternatively, we'll need to build in logic in TabsInTitlebar._update to compensate the margins on the titlebar content (which are moved on OS X only) for the customize mode difference.

Comment 2

[:Gijs (he/him)]

Attachment: window buttons shouldn't move down for customize mode,

Note that using this patch, they do still move - closer to the top. But this looks correct to me. Can you try this and see how you feel about it?

Comment 3

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

Comment on attachment 826179 [details] [diff] [review]
window buttons shouldn't move down for customize mode,

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

Hm, yes, this is better, but it's still a bit bumpy. I see the buttons hit the top of the window and snap down during transition in, and the opposite on transition out.

Any way we can smooth that out?

Comment 4

[:Gijs (he/him)]

(In reply to Mike Conley (:mconley) from comment #3)
> Comment on attachment 826179 [details] [diff] [review]
> window buttons shouldn't move down for customize mode,
> 
> Review of attachment 826179 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Hm, yes, this is better, but it's still a bit bumpy. I see the buttons hit
> the top of the window and snap down during transition in, and the opposite
> on transition out.
> 
> Any way we can smooth that out?

Is this the right way around? It'd make more sense in the opposite direction, because we're adding padding when going in, and removing padding when going out, and the positions are updated after entering/exiting customize mode.

Comment 5

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

(In reply to :Gijs Kruitbosch from comment #4)
> Is this the right way around? It'd make more sense in the opposite
> direction, because we're adding padding when going in, and removing padding
> when going out, and the positions are updated after entering/exiting
> customize mode.

Sorry, to be clear - I mean "I see the buttons...etc" as in, this is what I'm observing with your patch - not what would be ideal.

What would be ideal is probably for the buttons to transition up to the top of the window during the customize transition, and transition down when exiting.

Comment 6

[:Gijs (he/him)]

(In reply to Mike Conley (:mconley) from comment #5)
> (In reply to :Gijs Kruitbosch from comment #4)
> > Is this the right way around? It'd make more sense in the opposite
> > direction, because we're adding padding when going in, and removing padding
> > when going out, and the positions are updated after entering/exiting
> > customize mode.
> 
> Sorry, to be clear - I mean "I see the buttons...etc" as in, this is what
> I'm observing with your patch - not what would be ideal.

Yeah, and I meant, "is that really what you're observing, or did you make a mistake writing it down?". It doesn't make any sense that they move up to the top when transitioning in.

Comment 7

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

That's what I'm seeing:

http://www.screencast.com/t/HTJ9HCkaMfs

Comment 8

[:Gijs (he/him)]

This is better now that bug 886444 landed, but there's some weird drawing going on still. In particular, the buttons seem to jump down when exiting customize mode, and get painted over briefly when entering it. Not sure why.

Comment 9

[:Gijs (he/him)]

Comment on attachment 826179 [details] [diff] [review]
window buttons shouldn't move down for customize mode,

I've also noticed that the tabs seem to jump to the top of the window when entering/exiting customize mode. I don't know why, but it's pretty jarring. :-\

Comment 10

[:Gijs (he/him)]

Mike, needinfo'ing you here because I suspect that we can possibly get rid of the aforementioned nasty 'jump' by not having the titlebar redrawn when we enter customize mode... I *think* that'll just work. If it doesn't, it might at least give us a clue where this jump is coming from. It doesn't make sense to me that stuff is jumping up.

Comment 11

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

I think the jump is related to this CSS rule:

https://mxr.mozilla.org/projects-central/source/ux/browser/themes/osx/browser.css#3998

Let me poke at it for a few minutes, and I'll get back to you.

Comment 12

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

Attachment: Patch v1

My investigations resulted in a patch.

So it looks like we *have* to collapse the titlebar by eliminating the padding, otherwise we get this grey bar along the top. This is because the deck with the grid background doesn't extend above it / through it. Strangely, I wasn't able to get the background to display on the main-window when I moved it to that element...

Anyhow, this patch causes the buttons to move back up while customizing, and back down when exiting customization mode. I know this bug asks for not moving the buttons at all, but I think this might be an acceptable compromise.

What do you think, Gijs?

Comment 13

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

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

Hm, actually, I *should* be able to keep the buttons down in their starting spot, since that's what we currently do. We just have to maintain that...

So, my patch shows where at least some of the bump is coming from - it's the sudden appearance of the margin-top once customize-entered hits, in conjunction with the lack of padding-top on #titlebar when the customizing attribute hits.

Comment 14

[:Gijs (he/him)]

Punt! :-)

Comment 15

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

Attachment: WIP Patch 1

Ok, so this does the job on OSX, but there's probably a better way of doing it that doesn't involve us calling TabsInTitlebar._update every time a transition starts and ends, and I'll be looking into that. Just posting this here for posterity.

Comment 16

[:Gijs (he/him)]

(In reply to Mike Conley (:mconley) from comment #15)
> Created attachment 832457 [details] [diff] [review]
> WIP Patch 1
> 
> Ok, so this does the job on OSX, but there's probably a better way of doing
> it that doesn't involve us calling TabsInTitlebar._update every time a
> transition starts and ends, and I'll be looking into that. Just posting this
> here for posterity.

So the thing is, I think we needed to call TabsInTitlebar._update especially on Windows when we were changing the top padding. Now that we no longer do this, I *think* we can just not call TabsInTitlebar._update at all. If we're reducing some of the titlebar's padding in customize mode within the CSS (IIRC the other patches showed that?) in order not to get the ugly grey bar, maybe we can 'just' increase margins/paddings elsewhere so the calculations otherwise come to the same? Not sure if that idea works, but that seems like the easiest way to deal with this.

Comment 17

[Justin Dolske [:Dolske]]

Adjusting summary/priority per comment 8.

Comment 18

[:Gijs (he/him)]

Isn't this fixed now?

Comment 19

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

Yes, this should no longer be an issue, now that bug 930094 has landed.