User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b6pre) Gecko/20100901 Firefox/4.0b6pre Build Identifier: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b6pre) Gecko/20100901 Firefox/4.0b6pre See mockup. Reproducible: Always

Shorlander - I assert this is nice-to-have, but non-blocking. Disagree?

(In reply to comment #1) > Shorlander - I assert this is nice-to-have, but non-blocking. Disagree? I was going to say "agreed" but then I looked at it and it is looking pretty bad at the moment. Bug 589259 and 588764 might mitigate that some but I don't think we should ship it the way it looks right now.

I'd say the correct code would be: #main-window[sizemode="normal"] #navigator-toolbox[tabsontop="false"] > toolbar:not(#toolbar-menubar):not(#nav-bar):not([inFullscreen="true"]):not(:-moz-lwtheme) { border-left: 1px solid rgba(10%,10%,10%,.4); border-right: 1px solid rgba(10%,10%,10%,.4); } #nav-bar + #customToolbars + #PersonalToolbar[collapsed="true"] + #TabsToolbar[tabsontop="false"]:last-child:not(:-moz-lwtheme) { border-left: none !important; border-right: none !important; } #navigator-toolbox[tabsontop="false"] > #PersonalToolbar:not(:-moz-lwtheme) { border-top: 1px solid rgba(10%,10%,10%,.4) !important; background-image: -moz-linear-gradient(rgba(255,255,255,.5), rgba(255,255,255,0)); } This adds a border and a gradient. The colors are hardcoded in my code, which is wrong I believe. I don't know how to create patches though, so this is merely the code, it might help... An additional note: Adding custom toolbars has severe problems with tabs on bottom; there is a gap between custom bars and the bookmarks bar, the border-radius is applied to the bookmarks toolbar, etc. I tried to add the gradient and border to the correct toolbar (the one below nav-bar), but I couldn't get it done. So now it only worsens the problems, but it seems hard if not impossible to fix in css.

Here is a screenshot how old(left) and new(right) look. Compared to the mockup, the border color should be a little lighter maybe.

Unowned blocker, over to Frank.

The patch uses parts of the code suggested by pino_sb@live.com (thanks, pino_sb!) and implements the look shown in the attachment "before and after". It also makes sure that the UI doesn't move around when previewing/applying a persona.

Stephen suggested that the bottom border of the tabs toolbar (aka the top border of the content area) (aka #navigator-toolbox::after) be opaque also in tabs-on-bottom mode and that the top left/right corner pixels remain glassed. Patch v2 addresses this.

with this patch applied, if I add a new toolbar from the window "Customize Toolbar", this remains detached from the the other toolbars

(In reply to comment #9) > Created attachment 507065 [details] > Toolbar detached > > with this patch applied, if I add a new toolbar from the window "Customize > Toolbar", this remains detached from the the other toolbars Yea, that's an issue I mentioned in comment 3 as well. It is not caused by this patch however. The same thing happens without it. It seems noone has thought about the possibility of custom toolbars, since a lot of css is flawed when you use them with tabs on bottom (having a custom toolbar hidden can still affect looks for example). I did try to make the code for this bug work with custom toolbars, but I didn't manage. The background of the tab bar depends on the existence of toolbars above it, but with custom toolbars there are way too many possibilities to adjust for.

have you tried to change the value of -moz-box-ordinal-group for the toolbars? the correct order should be: - menubar - nav-bar - bookmarks-toolbar - custom-toolbar - tabs toolbar

(In reply to comment #9) > Created attachment 507065 [details] > Toolbar detached > > with this patch applied, if I add a new toolbar from the window "Customize > Toolbar", this remains detached from the the other toolbars (In reply to comment #11) > have you tried to change the value of -moz-box-ordinal-group for the toolbars? I think both of these issues should be combined into a separate bug entitled something like "(tabs on bottom) custom toolbars should be grouped between bookmarks toolbar and tabs toolbar". I don't know of an existing bug that addresses them, so please file one, and make it depend on this bug.

I have filed bug 629339 for the position of the toolbar

Comment on attachment 506647 [details] [diff] [review] patch v2 > #toolbar-menubar:not(:-moz-lwtheme), > #TabsToolbar[tabsontop="true"]:not(:-moz-lwtheme), > #navigator-toolbox[tabsontop="false"] > #nav-bar:not(:-moz-lwtheme), > #nav-bar + #customToolbars + #PersonalToolbar[collapsed="true"] + #TabsToolbar[tabsontop="false"]:last-child:not(:-moz-lwtheme) { > background: transparent !important; > color: black; > text-shadow: 0 0 .5em white, 0 0 .5em white, 0 1px 0 rgba(255,255,255,.4); > } >+ #nav-bar + #customToolbars + #PersonalToolbar[collapsed="true"] + #TabsToolbar[tabsontop="false"]:last-child:not(:-moz-lwtheme) { >+ border-left: none !important; >+ border-right: none !important; >+ } It looks like it might be possible to merge this with the rule above it. nit: use border-*-style >+ #main-window[sizemode="normal"] #navigator-toolbox[tabsontop="true"] > toolbar:not(#toolbar-menubar):not(#TabsToolbar):not([inFullscreen="true"]):-moz-lwtheme, >+ #main-window[sizemode="normal"] #navigator-toolbox[tabsontop="false"] > toolbar:not(#toolbar-menubar):not(#nav-bar):not([inFullscreen="true"]):-moz-lwtheme { >+ border-color: transparent !important; >+ } Why not just #main-window[sizemode=normal] #navigator-toolbox > toolbar:-moz-lwtheme? >- #navigator-toolbox[tabsontop="false"]::after, > #main-window[disablechrome] #navigator-toolbox[tabsontop="true"]::after { > background-color: @glassToolbarBorderColor@; > } Given the current direction of bug 631698, can you avoid this change for now? > #navigator-toolbox:not([tabsontop="true"]) > #PersonalToolbar { > padding-top: 0 !important; >- margin-top: 3px; >+ margin-top: 2px; >+ } >+ #navigator-toolbox:not([tabsontop="true"]) > #PersonalToolbar:not(:-moz-lwtheme) { >+ border-top: 1px solid @glassToolbarBorderColor@ !important; >+ background-image: -moz-linear-gradient(rgba(255,255,255,.5), rgba(255,255,255,0)); >+ } >+ #navigator-toolbox:not([tabsontop="true"]) > #PersonalToolbar:-moz-lwtheme { >+ border-top: 1px solid transparent !important; > } I think this would be simpler this way: #navigator-toolbox:not([tabsontop="true"]) > #PersonalToolbar { padding-top: 0 !important; margin-top: 3px; } #navigator-toolbox:not([tabsontop="true"]) > #PersonalToolbar:not(:-moz-lwtheme) { margin-top: 2px; border-top: 1px solid @glassToolbarBorderColor@ !important; background-image: -moz-linear-gradient(rgba(255,255,255,.5), rgba(255,255,255,0)); } And you should use @toolbarHighlight@ instead of rgba(255,255,255,.5).

Is there a good reason that this blocks betaN? If not, it should be moved over to final+.

Everything in comment 14 addressed.

forgot to configure .hgrc now with 8 lines of context

er, i'll remove that accidental extra line of whitespace when pushing this if r+'d.

Comment on attachment 511696 [details] [diff] [review] patch v3 Given this: >+ #main-window[sizemode=normal] #navigator-toolbox > toolbar:-moz-lwtheme { >+ border-color: transparent !important; >+ } Why not apply the border regardless of :-moz-lwtheme (and without !important): > #navigator-toolbox:not([tabsontop="true"]) > #PersonalToolbar { > padding-top: 0 !important; > margin-top: 3px; > } >+ #navigator-toolbox:not([tabsontop="true"]) > #PersonalToolbar:not(:-moz-lwtheme) { >+ margin-top: 2px; >+ border-top: 1px solid @toolbarShadowColor@ !important; >+ background-image: -moz-linear-gradient(@toolbarHighlight@, rgba(255,255,255,0)); >+ } Also, you can use [tabsontop=false] here and [inFullscreen] instead of [inFullscreen=true] elsewhere.

Comments addressed. In fullscreen mode, #main-window[sizemode=fullscreen] always seems to be true. Why do we need :not([inFullscreen]) with [sizemode=normal] or [sizemode=maximized] ?

(In reply to comment #20) > In fullscreen mode, #main-window[sizemode=fullscreen] always seems to be true. > Why do we need :not([inFullscreen]) with [sizemode=normal] or > [sizemode=maximized] ? It makes going fullscreen less jarring.

Comment on attachment 511806 [details] [diff] [review] patch v4 >+ #navigator-toolbox[tabsontop=false] > #PersonalToolbar { > padding-top: 0 !important; >- margin-top: 3px; >+ margin-top: 2px; >+ border-top: 1px solid @toolbarShadowColor@; >+ background-image: -moz-linear-gradient(@toolbarHighlight@, rgba(255,255,255,0)); > } Doesn't this apply the gradient when using a persona?

Pushed. http://hg.mozilla.org/mozilla-central/rev/2a4dc060de2b

Mozilla/5.0 (Windows NT 6.1; rv:2.0b12pre) Gecko/20110216 Firefox/4.0b12pre Verified issue and it's no longer present.

#navigator-toolbox[tabsontop=false] > #PersonalToolbar { padding-top: 0 !important; margin-top: 2px; border-top: 1px solid @toolbarShadowColor@; } In the following config, the above border-top part of the patch adds a line that should not be there: bookmarks toolbar + tabsontop false + personas used + maximized window

(In reply to comment #26) Please file a new bug.

Verified fixed on Mozilla/5.0 (Windows NT 6.1; rv:2.2a1pre) Gecko/20110404 Firefox/4.2a1pre