403147 - Style the library window like a Media collection app for Vista Aero

Status: VERIFIED FIXED

(Firefox :: Bookmarks & History, defect)

Description

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

Change the background color of the bookmarks organizer on Vista to (238,243,250).  This is to visually integrate with other application designed for organizing media objects.

See bug# 393514 for a mockup.

Comment 1

[Lim Chee Aun]

The code, tested using userChrome.css:

#placesView,
#placesList,
#placeContent{
	background-color: #EEF3FA !important;
}

Comment 2

[Recall]

I love it! Is this going to happen for Firefox 3?

Comment 3

[Ryan A. C.]

There is no system color for this.
if this is to be hardcoded then we should hardcode the text along with it.

Comment 4

[Wesley Johnston (:wesj)]

Just playing with this, it makes it hard to see selected but not focused items (for instance, select and item and then click in the searchbar). If you're going to hardcode colors though, you can fix that I guess.

Comment 5

[Wesley Johnston (:wesj)]

Attachment: selected elements with and without media background style

Just to clarify a bit more, this a screenshot of the current organizer (top), and the organizer with the above background color applied (bottom). Focus is in the search bar in both cases. Its actually hard to tell what's selected in both cases, so maybe its not a huge problem?

Comment 6

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

Can we override the inactive selection color?  If not, I don't think this is enough of a problem to alter the overall design of the window since the inactive selection color is light gray in other media collection applications on vista (although with a slight gradient).

Comment 7

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Attachment: screenshot - experimental css

alex, this is an experimental styling with background changed, 1px splitter, media-toolbox toolbar, plus fix to selection style (no gradients though). As you can see there is still some glitch (some glyph / icon is not good fot dark background). 
What do you think?

Comment 8

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

I'll be landing a new views icon for vista, as well as new drop marker icons.  Overall looks great.  A few suggestions:

-set toolbar to 37 pixels tall
-set context bar to the same background color

Comment 9

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Attachment: patch

This is a patch for you to tryout with the above fixes, i've used the new selector to apply this only to windows default theme.

making toolbar 37px is better, done for both xp and vista.

notice that the splitter is really 3px wide, having it 1px made it really difficult to grab without attention, so it has a colored left border (and that explain the slightly padding in the right pane, if you put this in the left pane you end up with a detached scrollbar so it's worst).

Comment 10

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

I'm not set up to build on my vista machine at the moment, can you attach another screen shot?

Comment 11

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Attachment: updated screenshot

Comment 12

[Dão Gottwald [:dao]]

Comment on attachment 315571 [details] [diff] [review]
patch

>+#placesView > splitter:-moz-system-metric(windows-default-theme) {
>+  width: 3px !important;
>+  min-width: 3px;
>+  -moz-border-left-colors: #A9B7C9;
>+  border-right: none;
>+  background-color: transparent;

Why transparent rather than #EEF3FA?

Comment 13

[Dão Gottwald [:dao]]

-moz-border-left-... is not RTL sensitive either.

Comment 14

[Wesley Johnston (:wesj)]

Can't review, but it seems like the splitter border should probably be on the right not the left, so that its flush with the info pane.

I also don't think you need to hardcode the selected+focus colors, as -moz-menuhover and -moz-menuhovertext work fine there. Whatever you do though, should carry over to both tree's.

Is the appearance from Bug 428031 going to carry over?

Comment 15

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

(In reply to comment #12)
> Why transparent rather than #EEF3FA?
i'd prefer setting the correct background in placesView and having the splitter transparent, not a real reason though

> -moz-border-left-... is not RTL sensitive either.
needs to be fixed

also probably searchModifiers should use the same style selection as tree rows, should feel like more consistent.

(In reply to comment #14)
> Can't review, but it seems like the splitter border should probably be on the
> right not the left, so that its flush with the info pane.

as explained before, if you put it on the left you will have a flying scrollbar, detached from the border (very awful)

> I also don't think you need to hardcode the selected+focus colors, as
> -moz-menuhover and -moz-menuhovertext work fine there. Whatever you do though,
> should carry over to both tree's.

is carried over in both trees. thank you for hint on menuhover

> Is the appearance from Bug 428031 going to carry over?

having a dialog (white) background doesn't apply well on vista, feel free to report your ideas.

Comment 16

[Jim Jeffery not reading bug-mail 1/2/11]

What makes you say white background doesn't apply well on Vista ? 
Here is a screen shot from 
https://bugzilla.mozilla.org/show_bug.cgi?id=420236#c76 
showing various Vista windows and they have white backgrounds.  
I use Vista HP, and the TaskManager, Explorer etc.. all have a white background, not powderblue of whatever it is. 

Comment 17

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

i'm also using vista from more than 1 year. still this dialog is following media player style rather than explorer style, see mockup in bug 393514. Hwv i was talking about the infoPane.

Comment 18

[Wesley Johnston (:wesj)]

> having a dialog (white) background doesn't apply well on vista, feel free to
> report your ideas.

Sorry about the splitter comments. I guess I was skimming to fast (toolkit could use a better splitter object it seems).

I have to use XP right now, but I think I had just set the background to transparent and got the gray color from the window background. I don't have a huge preference for one or the other though. It does help split up the window (better than blue and white, which looks unfinished), but the combo of blue and gray isn't really my favorite either.

Comment 19

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Attachment: updated screenshot

more experimentation with new toolbar icons:
- added a border around the full window contents (media player like)
- removed bottom border under the toolbar
- toolbar height at 36px (37 causes a 1px disabled icons move, to investigate)
- selected state simulated with background gradient images (i can't make vista menu background working, ideas?)
- used splitter code mostly from bug 426000
- used new libraryNavigation.png
- save button no more class small

Comment 20

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Attachment: patch

Comment 21

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Attachment: selection backgrounds (zipped pngs)

Comment 22

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

about the window border: the accelerated version of Aero has it, while the non accelerated one does not have a border. Still the window without the border is really awful (imo) and appear less clean... but the same is true for the download manager, so could be a consistency problem.

Comment 23

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Comment on attachment 317022 [details]
updated screenshot

Mike, any comment / idea?

Comment 24

[Victor Bielawski]

(In reply to comment #22)
The added border prevents the scrollbar from being mile-wide, and will look really bad on Aero.
(Also, why are the search scope buttons not native?)

Comment 25

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 317022 [details]
updated screenshot

So, I definitely like the way this looks, but I don't think we should be packaging our own gradients for the selection of items.

Let's take that part out, and just use standard selection until we can get -moz-appearance: menuItem working properly

Comment 26

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

(In reply to comment #25)
> Let's take that part out, and just use standard selection until we can get
> -moz-appearance: menuItem working properly

the only remaining issue about this is that when a tree has NO FOCUS the selected item will be mostly unrecognizable since it has a light grey very near to the background.
So we should at least overload the selected unfocused grey, do you have a valid color for that?

Comment 27

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Attachment: selection screenshot

since aero defines 2 colors for the border, what about taking the internal border color as background, without gradients? This way the selection is quite clear in both status (focus / unfocus) and is more similar to the real one.
We could also use the same styling for search modifiers, so hover would be gray, checked would be blue-ish.
Or alternatively we have to find a valid gray for the unfocused selection.

Comment 28

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Attachment: search modifiers screenshot

how search modifiers would appear using the same selection style

Comment 29

[Dão Gottwald [:dao]]

(In reply to comment #27)
> Or alternatively we have to find a valid gray for the unfocused selection.

That sentence indicates that you already know this, but I'll say it anyway: A border color isn't such a valid gray since, well, since it's a border color, and there's usually no text on borders.

I suppose adding a border is actually enough to fix the problem in attachment 315397 [details].

Comment 30

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Attachment: patch

a try-it-by-yourself patch for testing
fixes splitters color too (they are still threeddarkshadow for me), remove small save button style (should be styled using its identifier, i suggest using only a save icon though to gain some space)

Comment 31

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

Attachment: Native media collection tree selection

Just as a point of reference, here is the native appearance for tree selection in media collection applications on Vista.  Left is focused, right is unfocused.

Comment 32

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

Given beltzner's ui-r+ let's get this landed without the changes to tree view selection, and file a follow up bug on that.

I'm modifying the summary since this patch will make a large number of changes to the window's appearance.

Comment 33

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Attachment: same patch - without selection changes

as requested by Alex, same patch as above without selection changes.
Asking review to Dao

Comment 34

[Dão Gottwald [:dao]]

Comment on attachment 318177 [details] [diff] [review]
same patch - without selection changes

>Index: browser/themes/gnomestripe/browser/places/organizer.css

>-.small, .small[disabled="true"] {
>-  font-size: x-small;
>-  min-width: 0px;
>-  padding: 0px 4px 0px 4px;
>-  margin: 0px;  
>-  border: 0px;
>-}
>-
>-.small .button-text,
>-.small .button-box {
>-  padding: 0px;
>-  border: 0px;
>-}
>-

Why is that part of this bug? I see no rationale for this other than "should be styled using its identifier", which you aren't doing.

>Index: browser/themes/winstripe/browser/browser-aero.css

> #sidebar-splitter:-moz-system-metric(windows-default-theme) {
>   border: 0;
>-  -moz-border-end: 1px solid #a9b7c9;
>+  -moz-border-end: 1px solid #A9B7C9;
>+  -moz-border-right-colors: #A9B7C9;

-moz-border-left-colors is missing, and you can remove the color from -moz-border-end.
(same in organizer-aero.css)

>Index: browser/themes/winstripe/browser/places/organizer-aero.css

>+#placesToolbar:-moz-system-metric(windows-default-theme) {
>+  -moz-appearance: media-toolbox !important;
>+  color: white !important;
>+}
>+
>+#placesMenu > menu:-moz-system-metric(windows-default-theme) {
>+  color: white !important;
>+}
>+

Please drop :-moz-system-metric(windows-default-theme) and use -moz-win-mediatext as the text color once bug 427045 has landed.
Are all the !important flags needed?

Comment 35

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

(In reply to comment #34)
> (From update of attachment 318177 [details] [diff] [review])
> >Index: browser/themes/gnomestripe/browser/places/organizer.css
> 
> >-.small, .small[disabled="true"] {
> >-  font-size: x-small;
> >-  min-width: 0px;
> >-  padding: 0px 4px 0px 4px;
> >-  margin: 0px;  
> >-  border: 0px;
> >-}
> >-
> >-.small .button-text,
> >-.small .button-box {
> >-  padding: 0px;
> >-  border: 0px;
> >-}
> >-
> 
> Why is that part of this bug? I see no rationale for this other than "should be
> styled using its identifier", which you aren't doing.

there was previous requests to restore the save styling (that is completely wrong actually), so i put this as a first step to restore it to default. Can be removed though clearly.

> >Index: browser/themes/winstripe/browser/browser-aero.css
> 
> > #sidebar-splitter:-moz-system-metric(windows-default-theme) {
> >   border: 0;
> >-  -moz-border-end: 1px solid #a9b7c9;
> >+  -moz-border-end: 1px solid #A9B7C9;
> >+  -moz-border-right-colors: #A9B7C9;
> 
> -moz-border-left-colors is missing, and you can remove the color from
> -moz-border-end.
> (same in organizer-aero.css)

there is not a left-border, however easily settable to transparent

> 
> >Index: browser/themes/winstripe/browser/places/organizer-aero.css
> 
> >+#placesToolbar:-moz-system-metric(windows-default-theme) {
> >+  -moz-appearance: media-toolbox !important;
> >+  color: white !important;
> >+}
> >+
> >+#placesMenu > menu:-moz-system-metric(windows-default-theme) {
> >+  color: white !important;
> >+}
> >+
> 
> Please drop :-moz-system-metric(windows-default-theme) and use
> -moz-win-mediatext as the text color once bug 427045 has landed.

sure, waiting for that

> Are all the !important flags needed?

mostly, but can do a final check to see if i can reduce them to a minimum

Comment 36

[Dão Gottwald [:dao]]

(In reply to comment #35)
> > >Index: browser/themes/winstripe/browser/browser-aero.css
> > 
> > > #sidebar-splitter:-moz-system-metric(windows-default-theme) {
> > >   border: 0;
> > >-  -moz-border-end: 1px solid #a9b7c9;
> > >+  -moz-border-end: 1px solid #A9B7C9;
> > >+  -moz-border-right-colors: #A9B7C9;
> > 
> > -moz-border-left-colors is missing, and you can remove the color from
> > -moz-border-end.
> > (same in organizer-aero.css)
> 
> there is not a left-border, however easily settable to transparent

There's a left border in RTL, since you're using -moz-border-end.

Comment 37

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Attachment: patch

reduced to a minimum (no more save button styling).
fixed Dao's comments (thank you)
i've seen bug 427045 is checkin-needed so i've already inserted -moz-win-mediatext

Comment 38

[Dão Gottwald [:dao]]

Comment on attachment 318342 [details] [diff] [review]
patch

>Index: browser/themes/winstripe/browser/places/organizer-aero.css

>+#searchModifiers:-moz-system-metric(windows-default-theme) {
>+  border-bottom: 1px solid;
>+  -moz-border-bottom-colors: #A9B7C9;

border-bottom: 1px solid #A9B7C9;

>+#infoPaneBox:-moz-system-metric(windows-default-theme) {
>+  -moz-border-top-colors: #A9B7C9;

border-top-color: #A9B7C9;

>Index: browser/themes/winstripe/browser/places/organizer.css

>-#organizerScopeBar> toolbarbutton:not([disabled="true"]):hover {
>+#organizerScopeBar> toolbarbutton:not([disabled="true"]):not([checked="true"]):hover {
>   border-color: ThreeDShadow;
> }

nit: space after #organizerScopeBar missing

> #organizerScopeBar > toolbarbutton[checked="true"] {
>   border-color: ThreeDDarkShadow !important;
> }

If you add :not([checked="true"]) in the above rule, you can remove "!important" here, right?

Comment 39

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Attachment: patch

(In reply to comment #38)
> If you add :not([checked="true"]) in the above rule, you can remove
> "!important" here, right?

no that's still needed to have a flat look, however this should be restyled (in another bug) to appear more OS consistent on Vista (as seen in above screenshots).

I hope Dao's review is enough to have approval since this is more a theme bug than a places' one.

Final patch contains:
- library background
- splitters correct color
- toolbar media styling (dark background, white text)

everything other should be addressed in followups

Comment 40

[Kai Liu]

Just wanted to give you a heads-up about bug 431309, which will prepend -win- to the media-toolbox keyword, in case that ends up landing before this bug.

Comment 41

[Mike Beltzner [:beltzner, not reading bugmail]]

Attachment: screenshot of patch applied to build on vista

(In reply to comment #39)
> Final patch contains:
> - library background

This took me a few minutes to get used to, but I agree that it's consistent. It doesn't carry over to the sidebars, but that's OK, I guess. 

> - splitters correct color
> - toolbar media styling (dark background, white text)

I'm not seeing the white text (see attachment) - we definitely need that.

Comment 42

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

could require an !important, are we still using color: -moz-win-mediatext; or the name has changed?

Comment 43

[Kai Liu]

(In reply to comment #42)
> could require an !important, are we still using color: -moz-win-mediatext; or
> the name has changed?
> 

No, the keyword hasn't been renamed (or was it slated for renaming).  Take a look at bug 427045 comment 31.  Was that patch tested before it was landed?

Comment 44

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

so, let's wait for bug 427045 correct patch

Comment 45

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

Note that the white drop down arrow for aero has now landed, we should use it when using the black media toolbar:

http://mxr.mozilla.org/seamonkey/source/browser/themes/winstripe/browser/places/dropDown-aero.png

Comment 46

[Dão Gottwald [:dao]]

(In reply to comment #45)
> Note that the white drop down arrow for aero has now landed, we should use it
> when using the black media toolbar:
> 
> http://mxr.mozilla.org/seamonkey/source/browser/themes/winstripe/browser/places/dropDown-aero.png

Can a new bug be filed for this? Or handle that leftover in bug 403137. This bug already has a patch that should be ready to go. Note that the white dropmaker should have some darker outline, since we're going to use the media toolbox appearance regardless of whether it's the default theme.

Comment 47

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

>since we're going to use the media
>toolbox appearance regardless of whether it's the default theme.

The toolbar will gracefully degrade back to a normal toolbar for windows classic, but what about the background color of the window?  It seems like we should make all of these changes to be just for Aero to maintain the correct appearance for classic.

Comment 48

[Dão Gottwald [:dao]]

Yes, the hardcoded background and border colors use the windows-default-theme metric.

Comment 49

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Comment on attachment 318375 [details] [diff] [review]
patch

as Dao said let's fix that in a followup since here we have review and a tested patch. Moreover I don't know how to access the menu glyph, so i'll leave that to Dao.

I tested the patch with lastest hurly, and the text is now correctly white, so reasking approval.

Comment 50

[Ryan A. C.]

It's too bad we can't just draw the dropdown in SVG using the text color, that way it would automatically change color on the media-toolbox for classic & aero.

I suppose the easiest way though is to draw the glyph more like this:
http://mxr.mozilla.org/seamonkey/source/toolkit/themes/winstripe/global/tree/twisty-clsd-aero.png
since that would work on almost any color.

Comment 51

[Kai Liu]

What needs to be done for the next version is for dropmarker arrows to be drawn directly (either with SVG or with a -moz-appearance: dropmarker), so that the correct foreground color could be used and kludges like bug 413961, which is the cause of the smaller arrows in Firefox 3, can be avoided.  In the meantime, a hard-coded white arrow (no border needed) would be good enough, as the windows-default-theme metric would ensure that it won't get used on a light background.

Comment 52

[Mike Beltzner [:beltzner, not reading bugmail]]

FWIW, I still can't get this patch to apply such that it creates white text. What am I missing?

Comment 53

[Kai Liu]

(In reply to comment #52)
> FWIW, I still can't get this patch to apply such that it creates white text.
> What am I missing?
> 

What do you see when you look at attachment 318835 [details]?

Comment 54

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 318375 [details] [diff] [review]
patch

a=beltzner for 1.9, there must have been something wrong with my build, works fine now

Comment 55

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

probably you were lacking the last fix for bug 427045 since it has been checked-in in 2 pieces?

Comment 56

[Dietrich Ayala (:dietrich)]

Checking in browser/themes/winstripe/browser/browser-aero.css;
/cvsroot/mozilla/browser/themes/winstripe/browser/browser-aero.css,v  <--  browser-aero.css
new revision: 1.13; previous revision: 1.12
done
Checking in browser/themes/winstripe/browser/places/organizer-aero.css;
/cvsroot/mozilla/browser/themes/winstripe/browser/places/organizer-aero.css,v  <--  organizer-aero.css
new revision: 1.2; previous revision: 1.1
done
Checking in browser/themes/winstripe/browser/places/organizer.css;
/cvsroot/mozilla/browser/themes/winstripe/browser/places/organizer.css,v  <--  organizer.css
new revision: 1.15; previous revision: 1.14
done

Comment 57

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

(In reply to comment #31)
> Created an attachment (id=317943) [details]
> Native media collection tree selection
> 
> Just as a point of reference, here is the native appearance for tree selection
> in media collection applications on Vista.  Left is focused, right is
> unfocused.

Alex, in comment 31 you give us a screen shot with an example. The unfocused folder selection looks much better there as what we have currently. If I select a folder in the left pane and select a bookmark within the right pane, it's really hard to see which folder in the left pane is selected. Will there be an update of the CSS?

Comment 58

[Kai Liu]

(In reply to comment #57)
> Will there be an update of the CSS?
> 
To make it look like the screenshot, it'll have to be done in the native widget code.  All that could be done with CSS is to change the highlight color (it'll have to be hard-coded, but the background is already hard-coded...)

Comment 59

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

(In reply to comment #58)
> To make it look like the screenshot, it'll have to be done in the native widget
> code.  All that could be done with CSS is to change the highlight color (it'll
> have to be hard-coded, but the background is already hard-coded...)

Kai, shall I file a new bug about? Or is no further change planned?

Meanwhile verified with Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9pre) Gecko/2008050806 Minefield/3.0pre ID:2008050806. Everything looks like it was given with the screenshots.

Comment 60

[Kai Liu]

(In reply to comment #59)
> Kai, shall I file a new bug about? Or is no further change planned?
> 
To do this, we need to implement -moz-appearance: treeitem for Windows.  The CSS keyword is already present, but it doesn't do anything on Windows right now.  Probably something for the next release, but if you want, you can file a bug about it so that it's less likely to be forgotten.

Comment 61

[Gervase Markham [:gerv]]

Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h".

In Thunderbird 3.0b, you do that as follows:
Tools | Message Filters
Make sure the correct account is selected. Click "New"
Conditions: Body   contains   places-to-b-and-h
Change the action to "Delete Message".
Select "Manually Run" from the dropdown at the top.
Click OK.

Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter.

Gerv