Closed Bug 624292 Opened 9 years ago Closed 9 years ago

Don't completely override titlebar backgrounds (missing gradient when Firefox is maximized)

Categories

(Firefox :: Theme, defect)

x86
Windows XP
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 4.0b12

People

(Reporter: stdowa+bugzilla, Assigned: dao)

References

()

Details

Attachments

(5 files, 2 obsolete files)

User-Agent:       Mozilla/5.0 (Windows NT 5.1; rv:2.0b9pre) Gecko/20110109 Firefox/4.0b9pre
Build Identifier: Mozilla/5.0 (Windows NT 5.1; rv:2.0b9pre) Gecko/20110109 Firefox/4.0b9pre

The faux native with tabs-in-titlebar isn't displaying the "squares" bitmap between the titlebar and caption buttons.

Reproducible: Always

Steps to Reproduce:
1. Patch your XP install to allow non-signed themes, http://www.uxstyle.com/, if it works or use the older uxtheme.dll based patching
2. Extract WatercolorLite.msstyles from the zip and double click
3. Compare the normal & maximized titlebars
Attached image Normal titlebar
Attached image Maximized titlebar
Temporary fix, add this to userChrome.css or the Stylish extension:

  /*show normal titlebar image intstead of solid color*/
  #main-window[tabsintitlebar] #titlebar-content:not(:-moz-lwtheme),
  #main-window[tabsintitlebar]:not([inFullscreen]) #TabsToolbar:not(:-moz-lwtheme) {
    background-color: transparent !important;
    color: CaptionText;
  }
  #main-window[tabsintitlebar] #titlebar-content:not(:-moz-lwtheme):-moz-window-inactive,
  #main-window[tabsintitlebar]:not([inFullscreen]) #TabsToolbar:not(:-moz-lwtheme):-moz-window-inactive {
    background-color: transparent !important;
    color: InactiveCaptionText;
  }
Not a widget issue as originally thought.
Assignee: nobody → dao
Component: Widget: Win32 → Theme
Product: Core → Firefox
QA Contact: win32 → theme
Summary: Missing WatercolorLite titlebar->caption button transition bmp when maximized → Don't override titlebar backgrounds
Version: unspecified → Trunk
Assignee: dao → nobody
Duplicate of this bug: 626268
Blocks: 572160
Attached image fx_max_gradient_n1.png (obsolete) —
(In reply to comment #3)
> Temporary fix
works almost great but has a minor issue see attachment fx_max_gradient_n1.png
Summary: Don't override titlebar backgrounds → Don't override titlebar backgrounds, No color gradient on Application title bar when Firefox is maximized
Summary: Don't override titlebar backgrounds, No color gradient on Application title bar when Firefox is maximized → Don't override titlebar backgrounds (missing gradient when Firefox is maximized)
Attached patch patch (obsolete) — Splinter Review
This is a compromise, overlaying only the bottom of the title bar.
Assignee: nobody → dao
Status: NEW → ASSIGNED
Attachment #506215 - Flags: review?(felipc)
Attached image screenshots with patch
(In reply to comment #7)
> Created attachment 506215 [details] [diff] [review]
> patch
> 
> This is a compromise, overlaying only the bottom of the title bar.

Is the intended look a native titlebar (comment 3 & transparent) or not (patch & gradient)?
We can't go with a fully native title bar, as comment 6 indicates.
Screenshot with reporter's theme, looks pretty good
Comment on attachment 506215 [details] [diff] [review]
patch

Some notes to possibly improve these styles, now or in the future:

 - do we need to do this for the Luna/Olive/Silver XP themes? Seems like their titlebar are tall enough to work correctly, at least with default settings (if we remove these backgrounds and -moz-dialog from the tab bar). Although the end result isn't very different.

 - for the classic theme with a standard left-to-right gradient, we can increase the height of #titlebar and our theme code will draw it correctly.
The problem of course is calculating how many px to increase.. Maybe something easy could come out of the tabs-in-titlebar, together with a min-height.

with these two we would only need these compromises for custom themes.
Attachment #506215 - Flags: review?(felipc) → review+
(In reply to comment #12)
>  - do we need to do this for the Luna/Olive/Silver XP themes? Seems like their
> titlebar are tall enough to work correctly, at least with default settings (if
> we remove these backgrounds and -moz-dialog from the tab bar). Although the end
> result isn't very different.

I think a cleaner solution would be to let the TabsInTitlebar code set an attribute when the title bar is tall enough. I'll look into this in a followup bug.

>  - for the classic theme with a standard left-to-right gradient, we can
> increase the height of #titlebar and our theme code will draw it correctly.
> The problem of course is calculating how many px to increase.. Maybe something
> easy could come out of the tabs-in-titlebar, together with a min-height.

Sounds like slightly scary followup material :)
Attachment #506215 - Flags: approval2.0?
(In reply to comment #11)
> Created attachment 506574 [details]
> screenshot with reporter's theme
> 
> Screenshot with reporter's theme, looks pretty good

Well, if by "good" you mean non-native looking, not to mention the increased titlebar height which doesn't help, which I think Firefox kicked to the curb long ago but I haven't been following things as much as I used to.
(In reply to comment #14)

Feel free to toggle browser.tabs.drawInTitlebar to preserve the native title bar height. Tabs in the title bar obviously won't work when the title bar isn't tall enough.
Duplicate of this bug: 628835
Comment on attachment 506215 [details] [diff] [review]
patch

The comment about "scary followup" made me nervous and so I'm minusing. Feel free to ask for approval again with a clear risk/reward statement. Feels like the reward is prettttty low.
Attachment #506215 - Flags: approval2.0? → approval2.0-
Comment on attachment 506215 [details] [diff] [review]
patch

This patch alone is non-scary polish. The suggested followup isn't mandatory for it. It would just be a further visual improvement.
Attachment #506215 - Flags: approval2.0- → approval2.0?
Actually I'd like to avoid the patch due to the extra visual glitch when the menu bar is open for the XP themes.
We've already discussed that, right? The selector needs #toolbar-menubar[incative] ~ to take care of that case.
That didn't really help because with that the full top area switches from blue to gray when the menu is open
The menu bar is gray regardless of this patch, so the blue background behind the tab bar moving down is actually pretty nonsensical and ugly.
Attached patch patchSplinter Review
updated to tip, with the selector taking into account the menubar when it's temporarily shown
Attachment #506133 - Attachment is obsolete: true
Attachment #506215 - Attachment is obsolete: true
Attachment #512612 - Flags: approval2.0?
Attachment #506215 - Flags: approval2.0?
Comment on attachment 512612 [details] [diff] [review]
patch

a=beltzner
Attachment #512612 - Flags: approval2.0? → approval2.0+
http://hg.mozilla.org/mozilla-central/rev/6387a9f398f4
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Summary: Don't override titlebar backgrounds (missing gradient when Firefox is maximized) → Don't completely override titlebar backgrounds (missing gradient when Firefox is maximized)
Target Milestone: --- → Firefox 4.0b12
Depends on: 634975
Thanks on the fix Dão!
I filed a polish follow up as bug 634975.
(In reply to comment #12)
>  - do we need to do this for the Luna/Olive/Silver XP themes? Seems like their
> titlebar are tall enough to work correctly, at least with default settings (if
> we remove these backgrounds and -moz-dialog from the tab bar). Although the end
> result isn't very different.

filed bug 635063
Blocks: 635063
Mozilla/5.0 (Windows NT 5.1; rv:2.0b12pre) Gecko/20110221 Firefox/4.0b12pre

Verified issue and it's no longer present.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.