We're missing a lot of elements from the extra bindings in classic/global/mac/globalBindings on suiterunner. This make the browser tabs looks pretty bad. Icons are stretched and the whole tab content (label, icon) looks misaligned. I was looking at this yesterday and I couldn't make the tabs look right with just styling the current elements. Basically, I think we need the elements that where styled in global/mac/browser.css (now navigator/tabbrowser.css). We also need different styling of tabbrowser tabs, the styles we get from Pinstripe's tabbox.css are for non-browser tabs and some of them don't fit well (like 3px padding on the tab element). In a long-time perspective, we shouldn't use native styling on mac tabbrowser tabs. Neither Camino, Safari or Firefox uses native browser tabs. It just don't look right... Also, if we skip the native styling on mac browser tabs, we won't have any problem with people breaking them (the tabs doesn't always behave as expected when you try to style them) and we don't have to think of what happens when bug 231313 is fixed.

Note: this patch assumes a 'mac' and a 'win' dir in themes/classic/navigator. This makes it look as it used to do. Comments are welcome. There might be some tweaks left to do in tabbrowser.css, but they're minor (new tab button is a bit to ffar at the right etc). I figured we could use the themes/classic/navigator/win/tabbrowser.css file for xpfe builds.

Comment on attachment 261008 [details] [diff] [review] Patch, needs testing on win/nix Take into account that I forgot 2 endifs in jar.mn...(should work anyway)

+ <xul:hbox class="tab-icon-container"> + <xul:image class="tab-icon" xbl:inherits="validate,src=image"/> + </xul:hbox> ...and the "tab-icon-container" should probably go (or I should use it in the mac binding as well).

So, I never checked before I mocked up the patch - of course this exists on xpfe as well

Hmm, right - I think I just need a mac tabbBrowserBindings.xml and a mac tabbrowser.css in classic/navigator/mac

OK, thought a bit of this and I think the best thing to do is probably: 1) copy themes/classic/global/mac/globalBindings.xml to themes/classic/navigator/mac/tabbrowserBindings.xml 2) copy themes/classic/navigator/tabbrowser.css to themes/classic/navigator/mac/tabbrowser.css and to themes/classic/navigator/win/tabbrowser.css 3) Patch tabbrowserBindings.xml and mac/tabbrowser.css

Changes made to the new classic/navigator/mac/tabbrowserBindings.xml (the diff is against globalBindings.xml)

I'm not finish with this one yet, but these are the changes I found necessary. Neil et al: Does this whole approach sounds reasonable?

Attaching 4 screenshots, comparing xpfe and suiterunner before/after (see attachment #261505 [details] [diff] [review] and attachment #261509 [details] [diff] [review] for changes). The original globalBindings.xml is left untouched in classic/mac/global.

Comment on attachment 261571 [details] Screenshots, before and after patch Btw, bug 361568 is the reason why some of the icons is cut-off.

Neil tested adding the elements from globalBindings.xml in tabbrowser.xml, and it appears that it doesn't break windows. This means that we don't need any bindings in the mac version of tabbrowser.css. Neither do we need tabbrowserBindings.xml

Composer uses native-styled bottom tabs. Basically, we have the same problem there - icons are stretched etc. With my patch the tabs looks even worse - pretty terrible, actually. I think it's because we seem to apply some bindings from classic's global.css... those are then styled with toolkit's tabbox.css. Not using the bindings in classic/global/mac/global.css will fix the issues introduced by me. The icons are still stretched, though. I'm not going to do anything about this atm, since I think that is a separate issue. Most likely the problem is that we need the same content and styling as tabbrowser-tab (tab-image-left, tab-image-middle, tab-icon etc).

Those are Neil's tabbrowser changes. The only thing I've done is adding the "overflow: -moz-hidden-unscrollable;" that accidently got lost. This patch will add elements from themes/classic/global/globalBindings.xml and revert the mac style rules in tabbrowser.css

This is the cvs copy changes - copy tabbrowser.css in classic/navigator to classic/navigator/win/ and classic/navigator/mac.

I'm going to play a little bit with my tabbrowser.css changes, but I think this should be solved in two steps: 1) get Neil's changes in and the cvs copying done 2) get the mac tabbrowser.css changes in

Comment on attachment 261691 [details] CVS copy Note that we could keep the windows version as it is and just copy one file from navigator/ to navigator/mac if you like (might be easier and faster). (not going to land anything until everything is settled)

Comment on attachment 261691 [details] CVS copy You might want to tell the copier that since the file is already a recent copy it isn't on any branches and we don't care about the dates in the cvs history.

Mac tabbrowser.css and jar.mn changes. It will look better with an updated tree (so the patch in bug 349409 is in). Karsten, in order to review this on mac you'll need to apply attachment #261689 [details] [diff] [review] and create a themes/classic/navigator/mac directory. Note that the drag-and drop stuff from windows is included here as well.

Here's a diff between the two tabbrowser.css files so you can see what I changes in the mac file. Note that it isn't perfect, but it'll do for now, I think.

Comment on attachment 261709 [details] [diff] [review] Take2 - mac changes sr=me if you don't move the tabbrowser entries in jar.mn - the new entries should be inserted at the same point where you deleted the old one.

Comment on attachment 261709 [details] [diff] [review] Take2 - mac changes (In reply to comment #20) > (From update of attachment 261709 [details] [diff] [review]) > sr=me if you don't move the tabbrowser entries in jar.mn - the new entries > should be inserted at the same point where you deleted the old one. > I'll update the patch once Karsten have looked at it.

Comment on attachment 261689 [details] [diff] [review] Neil's tabbrowser changes Does it look like the screenshot with this patch? If so, it's still messed up....

(In reply to comment #22) > (From update of attachment 261689 [details] [diff] [review]) > Does it look like the screenshot with this patch? If so, it's still messed > up.... You mean the cut-off icons? See comment #10.

We actually don't need the tabs-left-cap/tabs-right-cap spacers, so I removed them. It might have been more clever to not have 2 reviewers, so just tell me if you think Karsten should do this one too :-) (he will need the patch anyway when looking at Part2).

I just realized that I forgot to add the focus rules from classic's tabbox.css. Fixed the jar.mn according to Neil's comment (moving sr+).

jftr: I fixed Composer's tabs in bug 378202.

Comment on attachment 262035 [details] [diff] [review] Neil's tabbrowser changes, version 2 I'm going to let the cvs copying be done and then combine the two existing patches to one (easier for everyone).

OK, here's a new version that combines the previous 2 patches. This one patches the newly cvs copied tabbrowser.css files (bug 379060). Trying jag for the sr. The only "new" thing here is that I switched from tab {...} to .tabbrowser-tab {...} etc in the css files. Just thought that was more consistent. Note here that close button and new tab button is not positioned 100% correct. But I'll file a follow-up on that.

Comment on attachment 263372 [details] [diff] [review] Complete patch - fix classic tabbrowser look Looks very decent. :-) Just two nits to carry over to the close/new tab button bug: - both buttons don't give hover feedback, so you don't know they're buttons - the close buttons lacks a tooltip Sorry for the delay... :(

Checking in suite/browser/navigator.css; /cvsroot/mozilla/suite/browser/navigator.css,v <-- navigator.css new revision: 1.14; previous revision: 1.13 done Checking in suite/browser/tabbrowser.xml; /cvsroot/mozilla/suite/browser/tabbrowser.xml,v <-- tabbrowser.xml new revision: 1.181; previous revision: 1.180 done Checking in themes/classic/jar.mn; /cvsroot/mozilla/themes/classic/jar.mn,v <-- jar.mn new revision: 1.163; previous revision: 1.162 done Checking in themes/classic/navigator/mac/tabbrowser.css; /cvsroot/mozilla/themes/classic/navigator/mac/tabbrowser.css,v <-- tabbrowser.css new revision: 1.15; previous revision: 1.14 done Checking in themes/classic/navigator/win/tabbrowser.css; /cvsroot/mozilla/themes/classic/navigator/win/tabbrowser.css,v <-- tabbrowser.css new revision: 1.15; previous revision: 1.14 done

I'll remove the obsolete themes/classic/navigator/tabbrowser.css file once it has cycled.

(In reply to comment #31) > I'll remove the obsolete themes/classic/navigator/tabbrowser.css file once it > has cycled. > File is now removed. Filed bug 381915 for the New/close Tab issue.