714186 - Add padding to top of windows when fullscreen on Lion

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Attachment: what crampy mcCramperson looks like

I put this in toolkit > themes because if we make any other windows fullscreen capable, then this applies. Feel free to disagree and move to firefox...

We should probably have more padding at the top of the window when in Lion full screen. It feels cramped up there.

See attachment for how it's looking with (with WIP bug 639705).

Comment 1

[Dave Camp (:dcamp)]

On top of looking cramped, I'm constantly triggering the menubar dropdown trying to click tabs.

Comment 2

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

After talking with shorlander, I'm going to go with an additional 10-11px.

Changing the height of the ::before pseudoelement here (https://mxr.mozilla.org/mozilla-central/source/browser/themes/pinstripe/browser.css#1916) does the trick, but applying changes via CSS with a sizemode="fullscreen" rule is rather slow... there's a noticeable delay after the transition is complete before the rules apply (I think sizemode isn't updated synchronously).

So what I'm thinking is inserting a new toolbar from browser.js which we can style directly (or use a ::before on that if needed). That way it happens immediately. The non-Lion FS code does something similar (adds a toolbar to catch mouse events for showing the main toolbar)

Another option would be to add/remove a new attribute to the window following the fullscreen event which we could use for styling.

AFAIK there's no way to modify pseudoelements from JS, so changing the style of the existing one doesn't seem to be an option.

Comment 3

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

(In reply to Paul O'Shannessy [:zpao] from comment #2)
> Another option would be to add/remove a new attribute to the window
> following the fullscreen event which we could use for styling.

And actually "inFullscreen" already exists https://mxr.mozilla.org/mozilla-central/search?string=inFullscreen&find=css&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central - I just need actually set it on Lion.

It's still not smooth though - since it's happening after the fullscreen event, the transition is already complete, then the extra height is added (at least when entering, things seem ok when exiting... weird).

A possible solution to this (and other related FS choppyness) is if we add a couple more events, similar to how the Cocoa APIs work. It has a "before" event, a "failed" event, and a "complete" event. Cocoa also differentiates "event" vs "exit". That's all fodder for some other bug but it's on my brain here...

Comment 4

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Bah, so sometimes it's smooth, sometimes it's not, it depends on how the toggle happens (our menu = smooth vs native controls = not). I filed bug 739749 for more FS events which we could use to solve this sometime in the future.

Comment 5

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Attachment: Patch v0.1

The change here is based on the numbers Stephen gave me. This only addresses tabs on top, tabs on bottom requires a bit more work I and I couldn't get the colors looking right (toolbar looked too dark), amongst other issues.

We could use a transition for the height and make this look a little smoother, but we only want it when activating natively. If we use that and trigger FS ourselves, we're choppy the other way. Can't win either way.

Comment 6

[Dão Gottwald [:dao]]

Comment on attachment 612406 [details] [diff] [review]
Patch v0.1

This looks like it will behave quite annoyingly when previewing a persona in fullscreen mode.

Comment 7

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Attachment: Patch v0.2

What about this then? It also addresses my concerns from bug 716450 about the bg image shifting in the transition.

Comment 8

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Dão, any chance you could look at this sooner rather than later? I'd like to finish up the fullscreen loose ends as much as possible.

Comment 9

[Frank Yan (:fryn)]

Comment on attachment 612677 [details] [diff] [review]
Patch v0.2

This patch doesn't apply cleanly to mozilla-central.
Do I need to apply another patch first?

Comment 10

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Attachment: Patch v0.2 (unbitrotted)

Sorry, it bitrotted when we split browser.js up.

Comment 11

[Frank Yan (:fryn)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/23b2eb2ee380

Comment 12

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Thanks Frank!

Comment 13

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/23b2eb2ee380

Comment 14

[Reuben Morais [:reuben]]

D:

Why are we reducing screen state? Isn't the point of fullscreen to have all the space you can get for the content? I agree the previous style was a bit too cramped, but all that padding feels like a big waste of space.

Comment 15

[Jean-Philippe HALIMI]

I agree with Reuben Morais I don't know about Chrome but Opera doesn't use padding at the top and it runs just fine I am not annoyed by the top bar each time I want to click a tab I have opened.

Comment 16

[Cam]

Padding is the wrong way to do this anyway on OSX 10.7+ the menubar should push down the window chrome which is then still clickable if you overshot as it's not covered up. Just look at how safari does it.

I'd throw in parity safari on this as well.

I personally find the way FF, Opera, and Chrome handle the menubar in fullscreen mode incredibly annoying.