Closed
Bug 579632
Opened 14 years ago
Closed 14 years ago
Fix OS X tabs-on-top styling and put tabs on top by default
Categories
(Firefox :: Theme, defect)
Tracking
()
RESOLVED
FIXED
Firefox 4.0b2
Tracking | Status | |
---|---|---|
blocking2.0 | --- | beta2+ |
People
(Reporter: dao, Assigned: dao)
References
Details
Attachments
(3 files, 1 obsolete file)
No description provided.
Assignee | ||
Comment 1•14 years ago
|
||
I *think* this works ok, but I couldn't test it on OS X.
Eventually this might end up being implemented differently, but I don't want to rush bug 547787 and bug 562733 into the beta.
Comment 2•14 years ago
|
||
I've found two problems with this:
- the "new tab" icon is squeezed vertically
- in tabs-on-top mode, the border of app tabs extends 1px into the lighter toolbar below.
Other than that it's a definitive improvement over the current state.
I'll fix up your patch and then rebase bug 547787 on top of this.
I hope you realize how frustrating this process is for me.
Comment 3•14 years ago
|
||
The new tab icon is squeezed because the toolbarbutton has 1px to much vertical padding. The app tab spilling is because the nav bar has margin-top: -1px, and thus overlays normal tabs but not position:fixed app tabs.
I've found another problem: The favicon on app tabs in 2px higher than on normal tabs. I'm not sure where this comes from, but it's directly related to position:fixed. Setting style="position:static" on the app tab fixes the vertical position of the favicon. Maybe position:fixed somehow interferes with box-align.
Assignee | ||
Comment 4•14 years ago
|
||
I think this fixes the mentioned issues, but again I'm not quite sure...
Attachment #458076 -
Attachment is obsolete: true
Attachment #458092 -
Flags: review?(mstange)
Attachment #458092 -
Flags: review?(gavin.sharp)
Attachment #458076 -
Flags: review?(gavin.sharp)
Comment 5•14 years ago
|
||
Comment on attachment 458092 [details] [diff] [review]
patch v2
Bingo, it does. Maybe add an XXX comment to the favicon position workaround since we're not sure why it's necessary.
The 5px box shadow blur worries me a little, but we can still change that if Txul complains, and it's (hopefully) only temporary anyway.
Attachment #458092 -
Flags: review?(mstange) → review+
Comment 6•14 years ago
|
||
And for the record, the app tab favicon issue wasn't caused by this patch, it's present on trunk already.
Assignee | ||
Comment 7•14 years ago
|
||
(In reply to comment #6)
> And for the record, the app tab favicon issue wasn't caused by this patch, it's
> present on trunk already.
Yep, it's also an issue on all platforms. box-align doesn't work there; I'm not sure whether we should expect it to work.
(In reply to comment #2)
> I hope you realize how frustrating this process is for me.
I'm sorry, I know about your efforts -- the result is quite fascinating in a way -- and I realize that you'd like to see that stuff land rather than spending more time it. However, there's no "looks great, let's land it" automation. I'm not sure what the app tab story is with your patches, but even if we can fix this and other issues that might arise before the fx4 release, I'd still be worried about what happens after that. We're unlikely to freeze the UI or tabbrowser features, so I'd like to make sure what lands is not a dead end. I'm still very skeptical about whether the styling is going to be manageable by anyone but yourself (or manageable by yourself even if a tricky enough problem pops up).
Comment 8•14 years ago
|
||
Markus: I also appreciate your patience and dedication - is there something we can do better to keep everyone on the same page vis-a-vis work? Our goals are shared, I feel like it's our workflows which are not.
Comment 9•14 years ago
|
||
Also: can someone post a screenshot of this for Steven's perusal? Or Steven, can you apply this patch and let us know if it's OK for beta 2?
Comment 10•14 years ago
|
||
Comment 11•14 years ago
|
||
Comment 12•14 years ago
|
||
(In reply to comment #8)
> is there something we
> can do better to keep everyone on the same page vis-a-vis work?
That's a good question, but at the moment I can't think of anything. I'll think about it some more.
I'm going to continue my conversation with Dão over email.
Comment 13•14 years ago
|
||
Markus: is that dependency just to get App-Tabs working properly?
Updated•14 years ago
|
blocking2.0: --- → beta2+
Comment 14•14 years ago
|
||
(In reply to comment #9)
> Also: can someone post a screenshot of this for Steven's perusal? Or Steven,
> can you apply this patch and let us know if it's OK for beta 2?
Looks a lot better than what we have now and is ok for beta 2 until bug 547787 and bug 562733 are ready.
Comment 15•14 years ago
|
||
(In reply to comment #13)
> Markus: is that dependency just to get App-Tabs working properly?
Right. We don't really depend on it in this bug because the patch in this bug contains a workaround for it, but it's necessary if we want to remove the workaround.
Updated•14 years ago
|
Attachment #458092 -
Flags: review?(gavin.sharp) → review+
Comment 16•14 years ago
|
||
Thanks, Gavin - Markus: want to check this in?
Assignee | ||
Comment 17•14 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b2
Comment 18•14 years ago
|
||
Tabs are still not on top by default in a new profile. Is a new bug needed?
Assignee | ||
Comment 19•14 years ago
|
||
(In reply to comment #18)
> Tabs are still not on top by default in a new profile. Is a new bug needed?
If that's true, then yes, a bug should be filed.
Comment 20•14 years ago
|
||
(In reply to comment #18)
> Tabs are still not on top by default in a new profile.
WFM, fwiw.
Comment 21•14 years ago
|
||
Yeah, WFM now too.
Assignee | ||
Comment 22•14 years ago
|
||
Flags: in-litmus+
You need to log in
before you can comment on or make changes to this bug.
Description
•