Closed
Bug 714186
Opened 13 years ago
Closed 12 years ago
Add padding to top of windows when fullscreen on Lion
Categories
(Firefox :: Theme, defect)
Tracking
()
RESOLVED
FIXED
Firefox 16
People
(Reporter: zpao, Assigned: zpao)
References
(Depends on 2 open bugs)
Details
Attachments
(2 files, 2 obsolete files)
(deleted),
image/png
|
Details | |
(deleted),
patch
|
fryn
:
review+
|
Details | Diff | Splinter Review |
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•13 years ago
|
||
On top of looking cramped, I'm constantly triggering the menubar dropdown trying to click tabs.
Assignee | ||
Comment 2•13 years ago
|
||
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.
Assignee: nobody → paul
Component: Themes → Theme
Product: Toolkit → Firefox
QA Contact: themes → theme
Assignee | ||
Comment 3•13 years ago
|
||
(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...
Assignee | ||
Comment 4•13 years ago
|
||
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.
Assignee | ||
Comment 5•13 years ago
|
||
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.
Attachment #612406 -
Flags: ui-review?(shorlander)
Attachment #612406 -
Flags: feedback?(dao)
Comment 6•13 years ago
|
||
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.
Attachment #612406 -
Flags: feedback?(dao) → feedback-
Assignee | ||
Comment 7•13 years ago
|
||
What about this then? It also addresses my concerns from bug 716450 about the bg image shifting in the transition.
Attachment #612406 -
Attachment is obsolete: true
Attachment #612406 -
Flags: ui-review?(shorlander)
Attachment #612677 -
Flags: feedback?(dao)
Assignee | ||
Updated•13 years ago
|
Attachment #612677 -
Flags: feedback?(dao) → review?(dao)
Assignee | ||
Comment 8•13 years ago
|
||
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.
Assignee | ||
Updated•12 years ago
|
Attachment #612677 -
Flags: review?(fryn)
Comment 9•12 years ago
|
||
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?
Assignee | ||
Comment 10•12 years ago
|
||
Sorry, it bitrotted when we split browser.js up.
Attachment #612677 -
Attachment is obsolete: true
Attachment #612677 -
Flags: review?(fryn)
Attachment #612677 -
Flags: review?(dao)
Attachment #633821 -
Flags: review?(fryn)
Updated•12 years ago
|
Attachment #633821 -
Flags: review?(fryn) → review+
Comment 11•12 years ago
|
||
Status: NEW → ASSIGNED
Target Milestone: --- → Firefox 16
Assignee | ||
Comment 12•12 years ago
|
||
Thanks Frank!
Comment 13•12 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Comment 14•12 years ago
|
||
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•12 years ago
|
||
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•11 years ago
|
||
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.
You need to log in
before you can comment on or make changes to this bug.
Description
•