414781 - Top of Bookmark Sidebar gutter isn't rounded in new Mac theme

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

[Al Billings [:abillings - ex-MoCo]]

Attachment: The scrollbar button's top...

In the new post-proto integrated version of the Mac theme for trunk, the top of the "gutter" for the scrollbar for the bookmark sidebar is not rounded. (The bottom is rounded.) Since the scrollbar button in the sidebar is now styled at the top and bottom to be wonderfully mac rounded, this means that the round top of the button meets the square top of the gutter.

See attached image that I made.

I see this in Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9b3pre) Gecko/2008012904 Minefield/3.0b3pre

Comment 1

[Asa Dotzler [:asa]]

I'm confused as to what's not right here. Al, can you explain what it's supposed to look like? Maybe a screenshot of the control implemented in another place in Firefox or some other app that's correct?

Comment 2

[Stephen Horlander [:shorlander]]

I don't understand the issue here either.

Comment 3

[Al Billings [:abillings - ex-MoCo]]

It's a difference of opinion about how the theme's style should look at the top of the scrollbar in the sidebar.

Comment 4

[Al Billings [:abillings - ex-MoCo]]

In other words, at the top of the sidebar, the scrolling gutter ends in a square edge even though there is not a straight line that it is butting up against. In the scrollbar on the right of the browser window, it also ends in a square edge, but in that instance, it is ending against a flat line that runs the width of the entire browser window. Have this single square edge at the top of the sidebar's scrollbar looks bad.

Comment 5

[Stephen Horlander [:shorlander]]

Attachment: Sidebar Examples

I agree it does look a little strange.

In the early Proto mockups I had things segmented out so this wouldn't be a problem. Somewhere along the way we decided to go with a more default OS X look doing away with the segmentation.

Mail.app has a pane at the bottom for activity and merely separates with with a line. We could try something similar. Aside from separating these panes visually though I am not sure what else could be done to alleviate the problem. Anything I can think of would require reworking the sidebar.

Comment 6

[Al Billings [:abillings - ex-MoCo]]

I think just adding the line, as you show, looks a bit better and that would be little work (which makes it a good choice!).

Comment 7

[Markus Amalthea Magnuson]

Attachment: Add separator between search field and tree in bookmark/history sidebar.

Seems like the separator was already there, as a top border on the sidebar tree, but overridden by other CSS. Adding !important makes the separator visible again, which is what this simple patch does.

I am thinking that maybe we should also add some top padding to make the margins equal between searchbox and separator, and separator and tree.

Comment 8

[Dão Gottwald [:dao]]

Comment on attachment 577107 [details] [diff] [review]
Add separator between search field and tree in bookmark/history sidebar.

There's border: none !important; right at the top of this file. Can you remove the presumably-unneeded !important there and merge these two selectors?

Comment 9

[Markus Amalthea Magnuson]

Attachment: Add separator between search field and tree in bookmark/history sidebar.

(In reply to Dão Gottwald [:dao] from comment #8)
> Comment on attachment 577107 [details] [diff] [review] [diff] [details] [review]
> Add separator between search field and tree in bookmark/history sidebar.
> 
> There's border: none !important; right at the top of this file. Can you
> remove the presumably-unneeded !important there and merge these two
> selectors?

Actually, it seems like the whole clause at row 5-9 is unnecessary:
http://mxr.mozilla.org/mozilla-central/source/browser/themes/pinstripe/places/places.css#5

The same rules are applied to the class .sidebar-placesTree in the clause at row 19-27:
http://mxr.mozilla.org/mozilla-central/source/browser/themes/pinstripe/places/places.css#19

Attached is a patch that removes that first clause instead of adding an !important to the border-rule, with the same result.

Comment 10

[Dão Gottwald [:dao]]

Comment on attachment 577149 [details] [diff] [review]
Add separator between search field and tree in bookmark/history sidebar.

Thanks!

Comment 11

[Dão Gottwald [:dao]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/3b60e1052a69

Comment 12

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/3b60e1052a69