Last Comment Bug 714186 - Add padding to top of windows when fullscreen on Lion
: Add padding to top of windows when fullscreen on Lion
Status: RESOLVED FIXED
:
Product: Firefox
Classification: Client Software
Component: Theme (show other bugs)
: Trunk
: x86 Mac OS X
: -- normal with 1 vote (vote)
: Firefox 16
Assigned To: Paul O'Shannessy [:zpao] (not reading much bugmail, email directly)
:
Mentors:
Depends on: 944228 1130209 639705
Blocks: 738335
  Show dependency treegraph
 
Reported: 2011-12-29 14:57 PST by Paul O'Shannessy [:zpao] (not reading much bugmail, email directly)
Modified: 2015-05-18 08:31 PDT (History)
9 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
what crampy mcCramperson looks like (35.43 KB, image/png)
2011-12-29 14:57 PST, Paul O'Shannessy [:zpao] (not reading much bugmail, email directly)
no flags Details
Patch v0.1 (2.00 KB, patch)
2012-04-04 17:07 PDT, Paul O'Shannessy [:zpao] (not reading much bugmail, email directly)
dao+bmo: feedback-
Details | Diff | Splinter Review
Patch v0.2 (2.26 KB, patch)
2012-04-05 13:50 PDT, Paul O'Shannessy [:zpao] (not reading much bugmail, email directly)
no flags Details | Diff | Splinter Review
Patch v0.2 (unbitrotted) (2.37 KB, patch)
2012-06-16 09:43 PDT, Paul O'Shannessy [:zpao] (not reading much bugmail, email directly)
fryn: review+
Details | Diff | Splinter Review

Description Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2011-12-29 14:57:23 PST
Created attachment 584863 [details]
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) 2012-03-26 15:31:57 PDT
On top of looking cramped, I'm constantly triggering the menubar dropdown trying to click tabs.
Comment 2 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2012-03-27 11:22:25 PDT
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 reading much bugmail, email directly) 2012-03-27 11:46:50 PDT
(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 reading much bugmail, email directly) 2012-03-27 12:52:03 PDT
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 reading much bugmail, email directly) 2012-04-04 17:07:28 PDT
Created attachment 612406 [details] [diff] [review]
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] 2012-04-05 02:46:30 PDT
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 reading much bugmail, email directly) 2012-04-05 13:50:02 PDT
Created attachment 612677 [details] [diff] [review]
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 reading much bugmail, email directly) 2012-05-25 16:49:44 PDT
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) 2012-06-08 17:18:33 PDT
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 reading much bugmail, email directly) 2012-06-16 09:43:39 PDT
Created attachment 633821 [details] [diff] [review]
Patch v0.2 (unbitrotted)

Sorry, it bitrotted when we split browser.js up.
Comment 12 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2012-07-10 22:23:04 PDT
Thanks Frank!
Comment 13 Ed Morley [:emorley] 2012-07-11 09:31:20 PDT
https://hg.mozilla.org/mozilla-central/rev/23b2eb2ee380
Comment 14 Reuben Morais [:reuben] 2012-07-14 15:32:08 PDT
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 2012-07-22 09:40:07 PDT
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 2013-06-18 09:24:24 PDT
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.

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