Unified titlebar/toolbar gradient broken after hiding toolbar

VERIFIED FIXED in Firefox 3.7a1

Status

()

VERIFIED FIXED
10 years ago
8 years ago

People

(Reporter: from.bmo.18, Assigned: mstange)

Tracking

(Blocks: 1 bug, {polish})

Trunk
Firefox 3.7a1
All
Mac OS X
polish
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [polish-easy] [polish-visual] [polish-p2])

Attachments

(3 attachments, 3 obsolete attachments)

(Reporter)

Description

10 years ago
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.
(Reporter)

Comment 1

10 years ago
Created attachment 344434 [details]
Illustration of the various states described, and Safari comparison
(Assignee)

Updated

10 years ago
Assignee: nobody → mstange
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Hardware: Macintosh → All
Version: unspecified → Trunk
(Assignee)

Comment 2

10 years ago
Created attachment 345510 [details] [diff] [review]
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.
Attachment #345510 - Flags: review?(gavin.sharp)
(Assignee)

Comment 3

10 years ago
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

10 years ago
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.
Keywords: polish
Whiteboard: [polish-easy] [polish-visual] [polish-high-visibility]
(Assignee)

Updated

10 years ago
Duplicate of this bug: 474509
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.
Whiteboard: [polish-easy] [polish-visual] [polish-high-visibility] → [polish-easy] [polish-visual] [polish-p2]
(Assignee)

Comment 7

9 years ago
Created attachment 402279 [details] [diff] [review]
wip
Attachment #345510 - Attachment is obsolete: true
Attachment #345510 - Flags: review?(gavin.sharp)
(Assignee)

Updated

9 years ago
Blocks: 517490
(Assignee)

Comment 8

9 years ago
Created attachment 411455 [details] [diff] [review]
v2
Attachment #402279 - Attachment is obsolete: true
Attachment #411455 - Flags: review?(joshmoz)
(Assignee)

Updated

9 years ago
Attachment #411455 - Flags: review?(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.
Attachment #411455 - Flags: review?(dao) → review-
(Assignee)

Updated

9 years ago
Blocks: 528830
(Assignee)

Comment 10

9 years ago
Created attachment 417059 [details] [diff] [review]
only the widget part
Attachment #411455 - Attachment is obsolete: true
Attachment #417059 - Flags: review?(joshmoz)
Attachment #411455 - Flags: review?(joshmoz)
(Assignee)

Updated

9 years ago
Depends on: 534152

Updated

9 years ago
Attachment #417059 - Flags: review?(joshmoz) → review+
(Assignee)

Comment 11

9 years ago
http://hg.mozilla.org/mozilla-central/rev/bde448aad47c
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.7a1
(Assignee)

Comment 12

9 years ago
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
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 13

9 years ago
That didn't fix the regression.
(Assignee)

Comment 14

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

Bug 534152 is the culprit of the regression.
Status: REOPENED → RESOLVED
Last Resolved: 9 years ago9 years ago
Resolution: --- → FIXED
(Assignee)

Updated

8 years ago
Depends on: 579250
Created attachment 460024 [details]
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?
(Assignee)

Comment 16

8 years ago
Both :)
Let's deal with that in bug 534152.
Ok, otherwise it looks good. Marking as verified fixed.
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b2) Gecko/20100720 Firefox/4.0b2
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.