461291 - Unified titlebar/toolbar gradient broken after hiding toolbar

Status: VERIFIED FIXED

(Firefox :: Theme, defect)

Description

[Daniel Luz]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b2pre) Gecko/20081022 Minefield/3.1b2pre
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b2pre) Gecko/20081022 Minefield/3.1b2pre

After hiding the toolbar in a browser window, the gradient in the titlebar is not updated, and does not blend well with the rest of the interface.

Opening a new window without a toolbar renders a correct gradient, but also renders a thin separator line that should not be there, except maybe in the default theme when the toolbar is hidden and the bookmarks bar is visible (because it has its own gradient; does not apply to other toolbars, afaict). Adding the toolbar to said new window correctly redraws the titlebar gradient, however.

Reproducible: Always

Steps to Reproduce:
1. Open a new window with the navigation toolbar.
2a. Hide the navigation toolbar.
   -OR-
2b. Press the toolbar toggle button to the right of the window.
    (it probably has a proper name)
Actual Results:  
Ugly unfinished gradient.

Expected Results:  
The titlebar should feature a full gradient, blending well with whatever other toolbars are below it.

This is the full list of behaviors I could find:

* Hiding nav toolbar in a window: unfinished gradient, no separator.
* New window without nav toolbar: correct gradient, unnecessary separator.
* Toolbar toggle button (with nav toolbar): unfinished gradient, unnecessary separator (should blend with the tab bar).
* Toolbar toggle button (without nav toolbar): gradient stays correct, separator pops out of nothing, possibly creating a double separator effect.

Comment 1

[Daniel Luz]

Attachment: Illustration of the various states described, and Safari comparison

Comment 2

[Markus Stange [:mstange]]

Attachment: patch v1

The titlebar gradient is corrected whenever the unified toolbar is drawn. So instead of just hiding the toolbar completely we must ensure that at least one pixel of the toolbar is shown.

Comment 3

[Markus Stange [:mstange]]

The #nav-bar needs to have height: 2px; because its bottom pixel row is hidden by the tabbar when the bookmarks bar isn't visible.

display:none on the toolbar's children is necessary for isElementVisible(gURLBar) in browser.js - otherwise Cmd+L wouldn't bring up the "Open Location..." dialog when the toolbar is collapsed.

Comment 4

[Pavol Vaskovic]

I'm not sure if it is not a new bug, but I strongly believe, that collapsing the toolbar with the toolbar control (the "bubble" button in top right window corner) should be equivalent to and reflected in the state of menu View -> Toolbars -> Navigation Toolbar.

Comment 6

[Alex Faaborg [:faaborg] (Firefox UX)]

This bug's priority relative to the set of other polish bugs is:
P2 - Polish issue that is in a secondary interface, occasionally encountered, and is easily identifiable.

Impacts the main window, but requires toolbar customization.  The issue is clearly noticeable.

Comment 7

[Markus Stange [:mstange]]

Attachment: wip

Comment 8

[Markus Stange [:mstange]]

Attachment: v2

Comment 9

[Dão Gottwald [:dao]]

Comment on attachment 411455 [details] [diff] [review]
v2

>+#main-window[chromehidden~="toolbar"] > #navigator-toolbox > #nav-bar > #urlbar-container,
>+#nav-bar[collapsed="true"] > #urlbar-container {
>+  display: none;

The reason for this seems to apply to the search bar as well. You could add the same workaround for that, but a universal solution seems preferable.

Comment 10

[Markus Stange [:mstange]]

Attachment: only the widget part

Comment 11

[Markus Stange [:mstange]]

http://hg.mozilla.org/mozilla-central/rev/bde448aad47c

Comment 12

[Markus Stange [:mstange]]

Backed out:
http://hg.mozilla.org/mozilla-central/rev/974d9ffda7b6
http://hg.mozilla.org/mozilla-central/rev/1ef1d73eb050

Regression: SunSpider increase 3.42% on Leopard Firefox
http://graphs.mozilla.org/graph.html#tests=[{"machine":65,"test":25,"branch":1},{"machine":64,"test":25,"branch":1},{"machine":63,"test":25,"branch":1},{"machine":172,"test":25,"branch":1},{"machine":171,"test":25,"branch":1},{"machine":170,"test":25,"branch":1},{"machine":168,"test":25,"branch":1},{"machine":169,"test":25,"branch":1},{"machine":173,"test":25,"branch":1},{"machine":176,"test":25,"branch":1},{"machine":174,"test":25,"branch":1},{"machine":175,"test":25,"branch":1},{"machine":177,"test":25,"branch":1},{"machine":178,"test":25,"branch":1},{"machine":180,"test":25,"branch":1},{"machine":179,"test":25,"branch":1},{"machine":182,"test":25,"branch":1},{"machine":181,"test":25,"branch":1}]&sel=1263320340,1263493140

Regression: DHTML increase 1.83% on Leopard Firefox
http://graphs.mozilla.org/graph.html#tests=[{"machine":65,"test":18,"branch":1},{"machine":64,"test":18,"branch":1},{"machine":63,"test":18,"branch":1},{"machine":172,"test":18,"branch":1},{"machine":171,"test":18,"branch":1},{"machine":170,"test":18,"branch":1},{"machine":168,"test":18,"branch":1},{"machine":169,"test":18,"branch":1},{"machine":173,"test":18,"branch":1},{"machine":176,"test":18,"branch":1},{"machine":174,"test":18,"branch":1},{"machine":175,"test":18,"branch":1},{"machine":177,"test":18,"branch":1},{"machine":178,"test":18,"branch":1},{"machine":180,"test":18,"branch":1},{"machine":179,"test":18,"branch":1},{"machine":182,"test":18,"branch":1},{"machine":181,"test":18,"branch":1}]&sel=1263320340,1263493140

Comment 13

[Markus Stange [:mstange]]

That didn't fix the regression.

Comment 14

[Markus Stange [:mstange]]

Relanded: http://hg.mozilla.org/mozilla-central/rev/0055b66de683

Bug 534152 is the culprit of the regression.

Comment 15

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: Current screenshot

Markus, we still have this darker single line between the tabbar and the window title vs. content area, depending if you have selected "Tabs on Top" or not. Is that part a theme issue or still something we have to fix?

Comment 16

[Markus Stange [:mstange]]

Both :)
Let's deal with that in bug 534152.

Comment 17

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Ok, otherwise it looks good. Marking as verified fixed.

Comment 18

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b2) Gecko/20100720 Firefox/4.0b2