Closed Bug 599215 Opened 9 years ago Closed 9 years ago

Update addon-bar style for aero glass theme

Categories

(Firefox :: Theme, defect)

x86
Windows 7
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 4.0b12
Tracking Status
blocking2.0 --- betaN+

People

(Reporter: mozbugz, Assigned: dao)

References

Details

(Whiteboard: [addon bar][hardblocker])

Attachments

(2 files, 9 obsolete files)

User-Agent:       Mozilla/5.0 (Windows NT 6.1; rv:2.0b7pre) Gecko/20100922 Firefox/4.0b7pre
Build Identifier: 

Addon bar looks like the old status bar for default theme, make it similar to when the bookmarks/nav bar or tabs on top/tabs on bottom with glass.

Reproducible: Always

Actual Results:  
looks old

Expected Results:  
updates with the theme.

this would be nice polish
Blocks: 574688
blocking2.0: --- → ?
Version: unspecified → Trunk
Perhaps we mark this as a dupe of bug 597949 and go for something closer to attachment 478239 [details]?
Duplicate of this bug: 597949
Blocking on Shorlander for a decision, even if that decision is WONTFIX :)
Assignee: nobody → shorlander
Status: UNCONFIRMED → NEW
blocking2.0: ? → betaN+
Ever confirmed: true
Attached image Addon Bar with Aero Applied (obsolete) —
It looks far superior in maximised mode, so it would be a suggestion that maybe only apply Aero when maximised? Anyway, this is what it looks like.
Shorlander, is this intended for Fx4.0 or not?
(In reply to comment #4)
> Created attachment 483873 [details]
> Addon Bar with Aero Applied
> 
> It looks far superior in maximised mode, so it would be a suggestion that maybe
> only apply Aero when maximised? Anyway, this is what it looks like.

I've played with this style a bit more and got it to look good both maximised and in normal mode. Do I need to file a new bug to propose the style formerly?
Whiteboard: [addon bar]
Whiteboard: [addon bar] → [addon bar][hardblocker]
This seems to be a dupe of bug 616018, or the other way around really.
Perhaps this should become the META bug. As this actually attempts to encompass both the glassing and the non-Vista/Win7 styling. I think we're all agree that the current white doesn't actually fit with anything else in the browser.
(In reply to comment #6)
> I've played with this style a bit more and got it to look good both maximised
> and in normal mode. Do I need to file a new bug to propose the style formerly?

Post a patch!

Stephen, this is currently assigned to you. What else needs to be done here? Or could we assign it to Paul, have you do UI-review, and then handoff to dev for implementation?
Duplicate of this bug: 616018
(In reply to comment #9)
> Perhaps this should become the META bug. As this actually attempts to encompass
> both the glassing and the non-Vista/Win7 styling. I think we're all agree that
> the current white doesn't actually fit with anything else in the browser.

I duped the other against this one, since there was overlap. Let's start the work for all the windows stylings here, filing spinoffs if necessary.
(In reply to comment #10)
> Stephen, this is currently assigned to you. What else needs to be done here? Or
> could we assign it to Paul, have you do UI-review, and then handoff to dev for
> implementation?

Not sure how this got assigned to me actually :)

Regardless I can make a patch with the design from bug 616018.
(In reply to comment #13)
> (In reply to comment #10)
> > Stephen, this is currently assigned to you. What else needs to be done here? Or
> > could we assign it to Paul, have you do UI-review, and then handoff to dev for
> > implementation?
> 
> Not sure how this got assigned to me actually :)
> 
> Regardless I can make a patch with the design from bug 616018.

Go for it, it'd probably be a lot quicker than me giving it a go. Do you think you could make it a glassed independent strip as per bug 616018 comment 6?
Whiteboard: [addon bar][hardblocker] → [addon bar][hardblocker][needs patch]
Assignee: shorlander → felipc
Attached image Screenshot (obsolete) —
I think this is the look we were going for. Let me know if I missed something or if there are new details to cover.
Attachment #505662 - Flags: ui-review?
Attachment #505662 - Flags: ui-review? → ui-review?(shorlander)
Attached patch Patch (obsolete) — Splinter Review
Patch that implements the screenshot above.

The -moz-appearance: none is to remove the standard border separators that our native theme draws between toolbars, and ended up looking wrong after the findbar was open for the first-time (as :first-child stops matching the toolbar#addon-bar)

The glass border is supposed to go around the chrome, while the findbar look is attached to the content so it gets the ThreeDShadow border-top as separator.

Also included small improvement to the look of the toolbarspring in customization mode as it looked kinda broken as a opaque box over the glass areas (which now includes the addon bar)
Attachment #505665 - Flags: review?(dao)
Status: NEW → ASSIGNED
Whiteboard: [addon bar][hardblocker][needs patch] → [addon bar][hardblocker]
(In reply to comment #15)
> Created attachment 505662 [details]
> Screenshot
> 
> I think this is the look we were going for. Let me know if I missed something
> or if there are new details to cover.

It's missing border. This doesn't look good.
Comment on attachment 505662 [details]
Screenshot

There are a few improvements we could make. First of all could we detach the Addon Bar from the view port. I really don't think it looks right attached, not on glass anyway (see bug 616018 comment 6). Secondly it gives the impression of being too wide. That's actually an issue which processed this bug. That said, could we bring in the left and right by a single pixel each (2px overall) as  aesthetically.
Comment on attachment 505665 [details] [diff] [review]
Patch

This patch removes the bottom border when the add-on bar is hidden in non-maximized windows.

The screenshot shows some ugly form of grayscale anti-aliased text -- not cool.

I'm not sure I like the glass at the bottom of maximized windows. Doesn't seem like a definite step forward.

Also looks rather weird with the find bar open.

>--- a/browser/base/content/browser.css
>+++ b/browser/base/content/browser.css

> #status-bar {
>+  background-color: transparent;

This doesn't belong in here.
Attachment #505665 - Flags: review?(dao) → review-
This bug lacks a reasonable summary.
OS: All → Windows 7
Summary: Update Addon Bar with Default theme styling. → Make the add-on bar translucent on aero glass theme
(In reply to comment #20)
> Comment on attachment 505665 [details] [diff] [review]
> Patch
> 
> This patch removes the bottom border when the add-on bar is hidden in
> non-maximized windows.

I missed this, will post new patch soon

> 
> The screenshot shows some ugly form of grayscale anti-aliased text -- not cool.

Is there any way to get around this, which is due to the non-opaque background-color? What was done to the tab bar?

> >--- a/browser/base/content/browser.css
> >+++ b/browser/base/content/browser.css
> 
> > #status-bar {
> >+  background-color: transparent;
> 
> This doesn't belong in here.

I did this because I think now that the status-bar is a shim, there's no case for it to ever be non-transparent. Either the background-color is the same and it will match (so there's no point), or it's different and will look wrong.

Where should I move this to?

> 
> I'm not sure I like the glass at the bottom of maximized windows. Doesn't seem
> like a definite step forward.
> 
> Also looks rather weird with the find bar open.
> 

Shorlander, here's a tryserver build of the patch to see how the current look feels (maximized and non-maximized)
http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/felipc@gmail.com-c5f1db25ca55/
(In reply to comment #21)
> This bug lacks a reasonable summary.

The current summary seems to be ignorant of the fact that this bug was supposed to cover all Windows styles at the very least.
(In reply to comment #23)
> The current summary seems to be ignorant of the fact that this bug was supposed
> to cover all Windows styles at the very least.

This is all that there's left to this bug now as a hardblocker
So we're going full glass, if we're using frost, then this doesn't appear to match the menu bar frost.  If we go with glass only for Aero, then it would match the download manager window.  I know Aero Basic doesn't use the same color as the other toolbars.  I think we should have that updated.   The addon bar should also be only as wide as the border as well.  

Its kinda got a weird effect with this patch now you resize the bottom border, which was sort of there already, but the patch changes it.  The addon bar gets white behind the bar when resizing w/o plugins of the tab, it gets grey and it gets frosted.  If your on a tab with plugins like youtube, you see glass and black and frost.  I think it would be nice if we got that fixed too which I think Roc would have to un-wire it, since I believe that issue started when retain layers changed it up.
That didn't quite make sense, but I opened this bug to get some consistency for the addon bar on our default theme with various windows themes classic/aero basic/aero glass, since it's off in its own world :)
(In reply to comment #25)
> So we're going full glass, if we're using frost, then this doesn't appear to
> match the menu bar frost.  If we go with glass only for Aero, then it would
> match the download manager window.  I know Aero Basic doesn't use the same
> color as the other toolbars.  I think we should have that updated.   The addon
> bar should also be only as wide as the border as well.  
> 
> Its kinda got a weird effect with this patch now you resize the bottom border,
> which was sort of there already, but the patch changes it.  The addon bar gets
> white behind the bar when resizing w/o plugins of the tab, it gets grey and it
> gets frosted.  If your on a tab with plugins like youtube, you see glass and
> black and frost.  I think it would be nice if we got that fixed too which I
> think Roc would have to un-wire it, since I believe that issue started when
> retain layers changed it up.

By default on all platforms the Addon Bar should match the colour of the Navigation Bar. On systems that support Glass,it will be frosted. However, on said systems (Win7/Aero) the Addon Bar should be full glass when in a maximised stated.
(In reply to comment #22)
> > The screenshot shows some ugly form of grayscale anti-aliased text -- not cool.
> 
> Is there any way to get around this, which is due to the non-opaque
> background-color? What was done to the tab bar?

We made sure that tabs are opaque (bug 612854).

> I did this because I think now that the status-bar is a shim, there's no case
> for it to ever be non-transparent. Either the background-color is the same and
> it will match (so there's no point), or it's different and will look wrong.
> 
> Where should I move this to?

To the theme's browser.css.
Try this:

#browser-bottombox {
  -moz-appearance: none !important;
  background: rgba(255, 255, 255, .5) padding-box !important; 
  border-radius: 0 0 4px 4px !important;
}

#addon-bar, #FindToolbar, #status-bar {
  -moz-appearance: none !important;
  background: none !important;
  border: 0 !important;
  margin: 0 !important;
}

#browser-bottombox > :first-child,
#browser-bottombox > :first-child[hidden="true"] ~ :not([hidden="true"]):not([collapsed="true"]),
#browser-bottombox > :first-child[collapsed="true"] ~ :not([hidden="true"]):not([collapsed="true"]) {
  border-top: 1px solid rgba(10%,10%,10%,.4) !important;
}
This is better:

#browser-bottombox:not(:-moz-lwtheme) {
  -moz-appearance: none !important;
  background: -moz-linear-gradient(top, rgba(255,255,255,.5), rgba(255,255,255,.3)) padding-box !important;  
  border-radius: 0 0 4px 4px !important;
}

#addon-bar, #FindToolbar {
  -moz-appearance: none !important;
  border: none !important;
  border-top: 1px solid rgba(10%,10%,10%,.4)  !important;
}

statusbar {
  background: none !important;
}
Attached image Screenshot - v2 (obsolete) —
Changes as discussed with shorlander:

 - Added 1px left and right softer borders to remove the feeling that the add-on bar is wider than the  content
 - Maximized mode now uses toolbar background style

We want to keep the transparency on normal mode even if it affects anti-aliasing, as most add-ons only use icons, and when needed an add-on can choose an opaque color for its own area.
Attachment #505662 - Attachment is obsolete: true
Attachment #506552 - Flags: ui-review?(shorlander)
Attachment #505662 - Flags: ui-review?(shorlander)
Also adding that the look & feel of the findbar is to be something integrated into the content, whereas the add-on bar is a global toolbar. That's why the findbar stays inside the browser's borders and the add-on bar outside
Attached patch Patch v2 (obsolete) — Splinter Review
Patch v2

Fixed questions from last patch, design points discussed with shorlander and explained on previous comments
Attachment #505665 - Attachment is obsolete: true
Attachment #506553 - Flags: review?(dao)
1. AOB without borders looks like something that just hangs at the bottom of window.
2. May I suggest that AOB blends with AOM?
Comment on attachment 506553 [details] [diff] [review]
Patch v2

Gotta add a softer bottom-border, and handle the Sync notification bar
Attachment #506553 - Flags: review?(dao)
Attached patch Patch v3 (obsolete) — Splinter Review
this was tricky considering all the switches between add-on bar visible and invisible and what elements we can use to style in each situation.

the breakdown is:

 * no elements on the bottom:
   - #browser-bottombox gets the glass border if sizemode=normal

 * only add-on bar
   - glass border on top of add-on bar is #browser-bottombox
   - #add-on border have a soft rgba(255,255,255,.1) left,right,bottom border

 * more elements on bottom (findbar and sync notifications), with or without add-on bar
   - #browser-bottombox with the glass border is at the top of the elements and hidden by margin-top: -1px (visually replaced by border-top: ThreeDShadow of the element)
   - last element before add-on bar (findbar or last notification) gets the glass border
Attachment #506924 - Flags: review?(dao)
Can we just use the same toolbar styling for sizemode=normal and maximized?
(In reply to comment #37)
> Can we just use the same toolbar styling for sizemode=normal and maximized?

If we're going with frosting then it's not a good idea. If we're going with plain glass, it's not preferable but it's reluctantly acceptable. I'm not sure what the issue is here. It's essentially a few extra lines of CSS.
(In reply to comment #37)
> Can we just use the same toolbar styling for sizemode=normal and maximized?

The appearance here is following UX's directions and mockup
... which I'm questioning. The grayscale AA on glass is ugly, the style switch when maximizing the window is inconsistent.
(In reply to comment #39)
> (In reply to comment #37)
> > Can we just use the same toolbar styling for sizemode=normal and maximized?
> 
> The appearance here is following UX's directions and mockup

After trying the frosted appearance in maximized mode for a while I don't think it's a problem to keep the normal and maximized styling consistent.
(In reply to comment #41)
> (In reply to comment #39)
> > (In reply to comment #37)
> > > Can we just use the same toolbar styling for sizemode=normal and maximized?
> > 
> > The appearance here is following UX's directions and mockup
> 
> After trying the frosted appearance in maximized mode for a while I don't think
> it's a problem to keep the normal and maximized styling consistent.

The logic is that against a huge glass Taskbar, there should be some consistency to make Firefox look like it belongs, in the same manner that we implemented the Firefox button in a bid to integrate into a unified look and feel for Windows 7. To have frost against that it just looks odd and while there are questions about inconsistencies, some actually add to the nuance of a program (Tabs in Titlebar is a perfect example of that). If we are saying that some buttons look 'ugly' then the issue should be with fixing the buttons.

If the window is glass, the tab/page-functions are blue and the page is white, you have to ask yourself where the Addon Bar falls. It's clearly part of the window. This is the train-of-thought that drove the tabs on top process. So there's no excuse to be ignorant of that now. The Addon Bar in full screen should match the Windows 7 Taskbar.
Attached patch Patch v4 (obsolete) — Splinter Review
This version has the frosted style for both modes
No, glass in maximized mode is not what I was asking for. No gain there, only loss (ugly text, overall strangeness).
Whiteboard: [addon bar][hardblocker] → [addon bar][hardblocker][needs review dao]
> The screenshot shows some ugly form of grayscale anti-aliased text -- not cool.

Shorlander and I have discussed this and are recommending ignoring the poor text rendering issue. 

 The new APIs for add-ons are mostly focused on adding icons and buttons on the add-on bar, so displaying text in the add-on bar itself should be fairly rare.  While adding text to the add-on bar will happen in some cases, an add-on could set a solid background around it to prevent the poor rendering issue.  Additionally, felipe  says subpixel AA can't happen on glass, at least in the 4.0 timeframe, and this surely isn't a common enough case to change or remove aero glass for.  Let's not change the design for the benefit of a few rare cases for now, and perhaps we can fix this problem in a future release.
Comment on attachment 507246 [details] [diff] [review]
Patch v4

Just marking the review on the patch with the current intended style. The difference between both are minimal (just the background color in maximized mode) and we can switch later with feedback if needed.
Attachment #507246 - Flags: review?(dao)
(In reply to comment #45)
> > The screenshot shows some ugly form of grayscale anti-aliased text -- not cool.
> 
> Shorlander and I have discussed this and are recommending ignoring the poor
> text rendering issue. 
> 
>  The new APIs for add-ons are mostly focused on adding icons and buttons on the
> add-on bar, so displaying text in the add-on bar itself should be fairly rare.

The only add-on using the new API that I have across (the "Hard Blocker counter") was adding text. And then there's bug 629304...

I've also noticed that the patch makes the add-on bar flash black when closing it or when maximizing the window. It's messy, I'd like to avoid it.
(In reply to comment #47)
> I've also noticed that the patch makes the add-on bar flash black when closing
> it or when maximizing the window. It's messy, I'd like to avoid it.

It's not clear what you mean from this comment.

What's messy? and can you say what you'd like to avoid? The whole patch? Or the black flashing (I'd like to avoid that too, but I bet Felipe can investigate!).
The patch itself is messy, as Felipe mentioned in comment 36, and the outcome is messy (see rough edges mentioned in previous comments).
Comment on attachment 507246 [details] [diff] [review]
Patch v4

This seems like a complete step backwards for maximized windows.
Attachment #507246 - Flags: review?(dao) → review-
The flashing problem is the new glass area being resized to accommodate the larger window size. This is a general Windows problem that can be seen in other programs too that uses transparent areas in the UI.
(Our tab bar already flashes black when maximizing too)
I'm not sure if this bug is a good idea.. it adds a lot of noise to the window since the desktop or any underlying applications shines through.. really felt weird in Opera as well, when they added their new default theme. 

btw: I hate the "frost"-look of buttons and labels (like its used on the traditional menu) - looks lame and destroys the sense of having glass at the first place. Why not add a stronger text-shadow like Windows is doing it by default?
Attached patch Patch v5 (obsolete) — Splinter Review
This patch adds some JS code to greatly simplify the CSS needed. The CSS is all about the visual styling now, no need to carefully handle the visibility states.

Same style as previous patch, pending decisions on the final appearance.
(In reply to comment #52)
Why not add a stronger text-shadow like Windows is doing it by default?

Actually they are just using GDI without hw acceleration where you see that.
Comment on attachment 508034 [details] [diff] [review]
Patch v5

Requesting feedback on this new approach
Attachment #508034 - Flags: feedback?(dao)
Whiteboard: [addon bar][hardblocker][needs review dao] → [addon bar][hardblocker][needs review dao][has patch]
Whiteboard: [addon bar][hardblocker][needs review dao][has patch] → [addon bar][hardblocker][needs feedback dao][has patch]
Attached image Screenshot 3
FWIW this is a screenshot with the toolbar style applied to the add-on bar
(In reply to comment #56)
> Created attachment 508590 [details]
> Screenshot 3
> 
> FWIW this is a screenshot with the toolbar style applied to the add-on bar

+1

More internally consistent, doesn't suffer from the technical constraints.
Summary: Make the add-on bar translucent on aero glass theme → Update addon-bar style for aero glass theme
Whiteboard: [addon bar][hardblocker][needs feedback dao][has patch] → [addon bar][hardblocker][needs review dao][has patch]
Attachment #508034 - Flags: feedback?(dao)
Attached patch Patch v6 (obsolete) — Splinter Review
After talking with Shorlander we decided to go with the toolbar style as displayed in screenshot 3.
Attachment #506553 - Attachment is obsolete: true
Attachment #506924 - Attachment is obsolete: true
Attachment #507246 - Attachment is obsolete: true
Attachment #508887 - Flags: review?(dao)
Comment on attachment 508887 [details] [diff] [review]
Patch v6

This seems unnecessarily complex, and I'm concerned about things that extensions might add to browser-bottombox. I'm working on a simpler patch that should basically accomplish the same.
Attachment #508887 - Flags: review?(dao) → review-
Assignee: felipc → dao
Whiteboard: [addon bar][hardblocker][needs review dao][has patch] → [addon bar][hardblocker]
Attachment #483873 - Attachment is obsolete: true
Attachment #506552 - Attachment is obsolete: true
Attachment #508034 - Attachment is obsolete: true
Attached patch patchSplinter Review
Not sure how to easily invoke the Sync notification. (I don't have an account.)
I suspect that it doesn't look quite right but not disastrous either, and that it can be handled in a followup bug.
Attachment #508887 - Attachment is obsolete: true
Attachment #509865 - Flags: review?(felipc)
Whiteboard: [addon bar][hardblocker] → [addon bar][hardblocker][has patch]
Comment on attachment 509865 [details] [diff] [review]
patch

I find it a bit weird that opening the find bar rounds the bottom borders, but I guess that's fine. (That was one of the reasons I was using the JS switch btw)

>+  #main-window[sizemode=normal] #browser-bottombox:not(:-moz-lwtheme),
>+  #main-window[sizemode=normal] #addon-bar:not(:-moz-lwtheme) {
>+    border-bottom-left-radius: 4px;
>+    border-bottom-right-radius: 4px;
>+  }

I think the addon-bar rule could use :last-child here (or even better #browser-bottombox > :last-child)

The sync notifications look pretty broken (http://i.imgur.com/a9461.png). You can invoke them in the error console by calling: "top.opener.gSyncUI.onSyncDelay(); top.opener.gSyncUI.initUI();". Your call if we should leave that for a follow-up.

Also I'm not sure if the threedhighlight is supposed to exist in these toolbars (sync and findbar). I had them removed on my patch because the findbar looks odd (specially with a persona theme applied)
Attachment #509865 - Flags: review?(felipc) → review+
(In reply to comment #61)
> >+  #main-window[sizemode=normal] #browser-bottombox:not(:-moz-lwtheme),
> >+  #main-window[sizemode=normal] #addon-bar:not(:-moz-lwtheme) {
> >+    border-bottom-left-radius: 4px;
> >+    border-bottom-right-radius: 4px;
> >+  }
> 
> I think the addon-bar rule could use :last-child here (or even better
> #browser-bottombox > :last-child)

That's not going to be as useful as it might seem, since the last child could be hidden.

> The sync notifications look pretty broken (http://i.imgur.com/a9461.png). You
> can invoke them in the error console by calling:
> "top.opener.gSyncUI.onSyncDelay(); top.opener.gSyncUI.initUI();". Your call if
> we should leave that for a follow-up.

Yeah, doesn't quite belong in this bug.

> Also I'm not sure if the threedhighlight is supposed to exist in these toolbars
> (sync and findbar). I had them removed on my patch because the findbar looks
> odd (specially with a persona theme applied)

This should probably be fixed in findBar.css, regardless of aero glass.
http://hg.mozilla.org/mozilla-central/rev/dd6f4de45152
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [addon bar][hardblocker][has patch] → [addon bar][hardblocker]
Target Milestone: --- → Firefox 4.0b12
Find bar now looks bad, should use a similar style.
Yep, now that the addon-bar looks fine, the find-bar looks very much out of place.
Either one looks o.k. on its own, but the discrepancy becomes apparent if both are enabled ... the find-bar should get the same treatment.
Depends on: 631884
(In reply to comment #64)
> Find bar now looks bad, should use a similar style.

Filed: Bug 631884
Blocks: 631884
No longer depends on: 631884
It isn't just the find-bar, either.
Try opening the add-on bar, the find-bar and then open a new page and you'll actually have _three_ differently styled UI-elements on the bottom of your browser window:
* the visually improved add-on bar in blue
* the find-bar in its classic grey
* the page loading url indicator thingy in a very different shade of white and grey

Neither really matches the style of the other two elements, only the add-on bar currently matches the style of the rest of the browser on Aero.
No longer blocks: 631884
Blocks: 632252
You need to log in before you can comment on or make changes to this bug.