The splitters for the bookmarks and history sidebars should be 1 pixel wide on Vista. This is similar to the changes made in the Library window (bug 403138). Note that the splitters should be visually 1 pixel wide, their actual width can remain larger for usability. The color of the 1 pixel splitter should be #a9b7c9

The CSS fix should be this, tried using Stylish on Firefox3b4: #sidebar-splitter{ border: 0 !important; -moz-border-right-colors: none !important; border-right: 1px solid #a9b7c9 !important; background-color: transparent !important; min-width: 0 !important; } This applies to *any* sidebar of the browser content area. Might affect sidebar-related extensions as well.

From an email conversation with beltzner: >>-style vista sidebars > >blocking: this is just the CSS fixes? Yes, and one image change (bug 425998)

Note that we need to fall back to a system color for accessibility, setting to depend on 425598

(In reply to comment #3) > Note that we need to fall back to a system color for accessibility Which system color?

For a theme that isn't Aero, we need to use ThreeDFace, but this will result in black on black for a high contrast theme, so for the accessibility pref we need to use ThreeDHighlight, which I believe will result in white on black. The two related bugs are bug 425598 and bug 426660

We should use ThreeDLightShadow, ThreedShadow or ThreedDarkShadow. Neither ThreeDFace nor ThreeDHighlight are right for this.

Sure, fwiw Vista's Windows explorer uses ThreeDFace in classic mode for splitters, but it certainly doesn't look like they put too much thought into it. I just went with what matched the file system view in classic mode.

It's possible that ThreeDLightShadow looks just like ThreeDFace; probably depends on the theme. That said, it's also entirely possible that they messed it up in Explorer.

Here is what explorer looks like in classic mode.

Oh, right, so it's not 1 pixel wide. I this case, it's debatable whether this is a border or part of the face.

I guess with our fancy new selectors we can make the splitter wider if we we are not in Aero. Perhaps matching our style on XP would look the best on classic in Vista.

Or the style of the splitters for the sidebars right now, library window on XP is using collapsible splitters at the moment.

(In reply to comment #10) With high contrast themes in mind, I think I'd still prefer to see it as a border and use ThreeDLightShadow. (In reply to comment #12) > Or the style of the splitters for the sidebars right now, library window on XP > is using collapsible splitters at the moment. In that case, we'd probably just have to add this: #sidebar-splitter:-moz-system-metric(windows-default-theme) { border: none !important; min-width: 1px !important; background-color: #a9b7c9 !important; } (Styling based on comment 1. Not sure if the "important" flags are needed.)

(In reply to comment #0) > their actual width can remain larger for usability. Without this, there would really be usability problem. But I imagine this would produce a visible gap between the splitter and the sidebar header, or between the splitter and the tab bar / content area.

(In reply to comment #11) > I guess with our fancy new selectors we can make the splitter wider if we we > are not in Aero. Perhaps matching our style on XP would look the best on > classic in Vista. I'm really wondering if this new selectors thing can work on-the-fly? I mean say, I'm using Aero and the splitter is 1px wide. While doing that, I change to Classic, the splitter changes to 3-4px wide?

Ehsan, do you have a plan for addressing the usability problem? If not, I think blocking needs to be reconsidered.

My impression was that the userChrome in comment #1 keeps the actual target area the same, with a one pixel visual target area, which (if it works) should be fine from a usability perspective. >I'm really wondering if this new selectors thing can work on-the-fly? I mean >say Theme changing in windows doesn't really work on the fly for most apps, it is usually accepted that closing and reopening the app after a theme change is required to pick up the new appearance, especially on Vista. On linux you have to reopen Firefox as well for a theme change, so I don't think that alone should stop us from making a better default appearance.

(In reply to comment #17) > My impression was that the userChrome in comment #1 keeps the actual target > area the same, with a one pixel visual target area, which (if it works) should > be fine from a usability perspective. It makes the actual target area one pixel wide, which I find easy to miss.

(In reply to comment #17) > My impression was that the userChrome in comment #1 keeps the actual target > area the same, with a one pixel visual target area, which (if it works) should > be fine from a usability perspective. I don't see how it would be possible to make the visual target area 1px wide while keeping the actual target area >1px wide in CSS... If the splitter element is supposed to be more than 1px wide, then rendering it to 1px only would leave some left-over space unrendered (possibly with the background color, which is white by default) and it could be very ugly, unless we do something like this: set the width of the element to 5px (for example) set the right border to 1px solid ThreeDLightShadow (or whatever) set the background color to transparent make sure the splitter is positioned on top of the other elements (for example by setting its position to absolute, and adjusting its position and z-index) This should imitate the Vista splitter in Explorer pretty closely I suppose.

(In reply to comment #19) > make sure the splitter is positioned on top of the other elements (for example > by setting its position to absolute, and adjusting its position and z-index) Even if that's possible with XUL elements (I don't think it is), it would probably prevent the splitter from functioning.

(In reply to comment #20) > (In reply to comment #19) > > make sure the splitter is positioned on top of the other elements (for example > > by setting its position to absolute, and adjusting its position and z-index) > > Even if that's possible with XUL elements (I don't think it is) I'm also not sure how to do it... > it would > probably prevent the splitter from functioning. Why?

(In reply to comment #21) > > it would > > probably prevent the splitter from functioning. > > Why? I've tried what you said. And the splitter can't function. By setting margin-left: -[X]px;, it moves the splitter a bit but the sidebar overlaps on it. The larger the [X], the splitter goes 'behind' the sidebar. I've tried margin-right, but the splitter goes 'behind' the web page content area as well.

(In reply to comment #22) > I've tried what you said. And the splitter can't function. By setting > margin-left: -[X]px;, it moves the splitter a bit but the sidebar overlaps on > it. The larger the [X], the splitter goes 'behind' the sidebar. I've tried > margin-right, but the splitter goes 'behind' the web page content area as well. Can you share the stylesheet you're using? Have you tried z-index: some_large_number on the splitter?

(In reply to comment #23) > (In reply to comment #22) > > I've tried what you said. And the splitter can't function. By setting > > margin-left: -[X]px;, it moves the splitter a bit but the sidebar overlaps on > > it. The larger the [X], the splitter goes 'behind' the sidebar. I've tried > > margin-right, but the splitter goes 'behind' the web page content area as well. > > Can you share the stylesheet you're using? Have you tried z-index: > some_large_number on the splitter? I've tried z-index, but to no avail. Example: #sidebar-splitter{ border: 0 !important; -moz-border-right-colors: none !important; border-right: 1px solid #a9b7c9 !important; margin-left: -10px; }

(In reply to comment #24) > I've tried z-index, but to no avail. Example: > > #sidebar-splitter{ > border: 0 !important; > -moz-border-right-colors: none !important; > border-right: 1px solid #a9b7c9 !important; > margin-left: -10px; > } I'm not sure what you've tried exactly, since your example doesn't include z-index, but the following userChrome.css changes work beautifully for me: #sidebar-splitter{ border: 0 !important; -moz-border-right-colors: none !important; border-right: 1px solid #a9b7c9 !important; min-width: 0 !important; width: 3px !important; background-color: transparent !important; margin-left: -3px; position: relative; z-index: 10; } I'll attach some screenshots. The splitter remains fully functional, and the visual target area is just a 1px wide border-right, while the actual target area is 3px wide. Wider target areas (such as 5px) interfere with the operation of the scrollbar when the history sidebar is expanded, but we could easily try them if desired.

z-index works with position:fixed|absolute|relative only. (In reply to comment #21) > > it would > > probably prevent the splitter from functioning. > > Why? I suppose the code that resizes the siblings assumes that the splitter is laid out between rather than on top of them.

Neither GMIP nor PrintScreen could grab screenshots including the mouse pointer, but this should be trivial to test in order to get a feel of the target area.

(In reply to comment #26) > (In reply to comment #21) > > > it would > > > probably prevent the splitter from functioning. > > > > Why? > > I suppose the code that resizes the siblings assumes that the splitter is laid > out between rather than on top of them. Note that this applies to absolute/fixed positioning. If relative works, that's probably fine.

(In reply to comment #26) > z-index works with position:fixed|absolute|relative only. No, it only works with positioned elements (that is, any element with position set to anything other than auto I guess). > (In reply to comment #21) > > > it would > > > probably prevent the splitter from functioning. > > > > Why? > > I suppose the code that resizes the siblings assumes that the splitter is laid > out between rather than on top of them. Have you tried the code I posted in comment 25? It works perfectly. The splitter is fully functional...

(In reply to comment #29) > (In reply to comment #26) > > z-index works with position:fixed|absolute|relative only. > > No, it only works with positioned elements (that is, any element with position > set to anything other than auto I guess). It doesn't work with position:static. > Have you tried the code I posted in comment 25? It works perfectly. The > splitter is fully functional... see comment 28. position:relative leaves the surrounding layout in place as if the element was positioned statically.

(In reply to comment #25) > I'm not sure what you've tried exactly, since your example doesn't include > z-index, but the following userChrome.css changes work beautifully for me: > > #sidebar-splitter{ > border: 0 !important; > -moz-border-right-colors: none !important; > border-right: 1px solid #a9b7c9 !important; > min-width: 0 !important; > width: 3px !important; > background-color: transparent !important; > margin-left: -3px; > position: relative; > z-index: 10; > } Alright, I tried this and it surprisingly works. But, 'position: relative' seems dangerous...

(In reply to comment #30) > see comment 28. position:relative leaves the surrounding layout in place as if > the element was positioned statically. So I guess you agree that this approach is fine, then.

(In reply to comment #31) > Alright, I tried this and it surprisingly works. But, 'position: relative' > seems dangerous... How come?

From my understanding, XUL UI is grid-based layout, so relative positioning doesn't sound right, at least for me.

(In reply to comment #32) > (In reply to comment #30) > > see comment 28. position:relative leaves the surrounding layout in place as if > > the element was positioned statically. > > So I guess you agree that this approach is fine, then. Yes, although I also agree that it looks fragile / potentially dangerous. But if you tested it and get it (ui-)reviewed, that should be fine.

Comment on attachment 313583 [details] Screenshot of the code in comment 25 Requesting ui-review on both the look of the implementation and its behavior. Once it gets ui-r+ and bug 426660 lands, I'll provide a patch for review.

I think whoever does the UI review needs a patch in order to test if the target are is big enough, doesn't steal space from the scroll bar, etc.

(In reply to comment #37) > I think whoever does the UI review needs a patch in order to test if the target > are is big enough, doesn't steal space from the scroll bar, etc. You're right. Here's the patch to evaluate this.

Comment on attachment 313583 [details] Screenshot of the code in comment 25 Switching ui-review to beltzner to get his feedback.

I moved the ui-review request to the patch to make it clear that the behavior needs to be tested.

Comment on attachment 313913 [details] [diff] [review] Patch (v1) This is fine, yes, with two nits: Nit1: On Vista there's actually a 1px pad between the keyline splitter and any adjescent UI. Can we get that here, as well? Nit2: This needs to also be used for the splitters used in the Library, which I think are a slightly different widget?

(In reply to comment #41) > Nit2: This needs to also be used for the splitters used in the Library, which I > think are a slightly different widget? i've merged this splitter code in my Library experimental patch for Vista (bug 403147) and feels like working fine (apart putting margin-left: -4px; feels like more coherent with the OS, with 3px there's too much distance between scrollbar and the splitter line). code is already there so can be easily plugged off, moved, merged, as you will.

Looks like a new patch is needed here?

(In reply to comment #41) > Nit1: On Vista there's actually a 1px pad between the keyline splitter and any > adjescent UI. Can we get that here, as well? I don't know how that can be done. > Nit2: This needs to also be used for the splitters used in the Library, which I > think are a slightly different widget? Nope, it's the same widget. The new patch addresses the splitter in the Library as well, and also makes the code RTL-friendly.

should not you use #placesView > splitter:-moz-system-metric(windows-default-theme) { and maybe also in the sidebar... Are we going to hardcode color and one line also for classic theme?

Let's have Beltzner's idea on this.

Comment on attachment 317483 [details] [diff] [review] Patch (v1.1) Yeah, we need -moz-metric(windows-default-theme) here. On classic (/me shakes fist!) the sidebars are as they ever were.

(In reply to comment #47) > (From update of attachment 317483 [details] [diff] [review]) > Yeah, we need -moz-metric(windows-default-theme) here. On classic (/me shakes > fist!) the sidebars are as they ever were. Done.

Comment on attachment 317562 [details] [diff] [review] Patch (v1.2) r+uir+a=beltzner

mozilla/browser/themes/winstripe/browser/browser-aero.css 1.8 mozilla/browser/themes/winstripe/browser/jar.mn 1.92 mozilla/browser/themes/winstripe/browser/places/organizer-aero.css 1.1

Verified with Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9pre) Gecko/2008050606 Minefield/3.0pre ID:2008050606