Last Comment Bug 933933 - OS X titlebar buttons should not jump during customize mode transition
: OS X titlebar buttons should not jump during customize mode transition
Status: RESOLVED WORKSFORME
[Australis:P3]
:
Product: Firefox
Classification: Client Software
Component: Toolbars and Customization (show other bugs)
: Trunk
: All Mac OS X
-- normal (vote)
: ---
Assigned To: Mike Conley (:mconley)
:
: :Gijs
Mentors:
Depends on:
Blocks: australis-cust 873060 851652
  Show dependency treegraph
 
Reported: 2013-11-01 13:18 PDT by Mike Conley (:mconley)
Modified: 2014-02-05 18:10 PST (History)
9 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
window buttons shouldn't move down for customize mode, (1.62 KB, patch)
2013-11-01 16:43 PDT, :Gijs
no flags Details | Diff | Splinter Review
Patch v1 (1.25 KB, patch)
2013-11-07 10:51 PST, Mike Conley (:mconley)
no flags Details | Diff | Splinter Review
WIP Patch 1 (3.97 KB, patch)
2013-11-14 12:55 PST, Mike Conley (:mconley)
no flags Details | Diff | Splinter Review

Description User image Mike Conley (:mconley) 2013-11-01 13:18:23 PDT
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 User image :Gijs 2013-11-01 15:34:56 PDT
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 User image :Gijs 2013-11-01 16:43:59 PDT
Created attachment 826179 [details] [diff] [review]
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 User image Mike Conley (:mconley) 2013-11-04 08:43:30 PST
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 User image :Gijs 2013-11-04 08:45:22 PST
(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 User image Mike Conley (:mconley) 2013-11-04 08:46:58 PST
(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 User image :Gijs 2013-11-04 08:57:43 PST
(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 User image Mike Conley (:mconley) 2013-11-04 09:03:38 PST
That's what I'm seeing:

http://www.screencast.com/t/HTJ9HCkaMfs
Comment 8 User image :Gijs 2013-11-06 02:31:14 PST
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 User image :Gijs 2013-11-07 02:14:00 PST
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 User image :Gijs 2013-11-07 02:30:24 PST
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 User image Mike Conley (:mconley) 2013-11-07 07:29:20 PST
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 User image Mike Conley (:mconley) 2013-11-07 10:51:27 PST
Created attachment 828792 [details] [diff] [review]
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 User image Mike Conley (:mconley) 2013-11-07 11:49:58 PST
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 User image :Gijs 2013-11-13 05:25:58 PST
Punt! :-)
Comment 15 User image Mike Conley (:mconley) 2013-11-14 12:55:49 PST
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.
Comment 16 User image :Gijs 2013-12-09 03:54:32 PST
(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 User image Justin Dolske [:Dolske] 2013-12-12 00:46:04 PST
Adjusting summary/priority per comment 8.
Comment 18 User image :Gijs 2014-02-05 14:32:14 PST
Isn't this fixed now?
Comment 19 User image Mike Conley (:mconley) 2014-02-05 18:10:34 PST
Yes, this should no longer be an issue, now that bug 930094 has landed.

Note You need to log in before you can comment on or make changes to this bug.