416801 - View button in History sidebar is stretched vertically

Status: VERIFIED FIXED

(Firefox :: Theme, defect)

Description

[Andrés Delfino]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b4pre) Gecko/2008021101 Minefield/3.0b4pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b4pre) Gecko/2008021101 Minefield/3.0b4pre

In Windows Classic mode, View button in History sidebar is stretched vertically. Almost all buttons are 22px tall, but the View button in History sidebar is 27px tall.

Reproducible: Always

Comment 1

[Andrés Delfino]

Attachment: View button compared to Credits button of About window.

Comment 2

[Andrés Delfino]

Can someone please confirm this?

Comment 3

[Steve England [:stevee]]

Attachment: I don't know if this problem is restricted to classic mode.

Comment 4

[Andrés Delfino]

(In reply to comment #3)
> Created an attachment (id=306750) [details]
> I don't know if this problem is restricted to classic mode.
> 

Right, I'm attaching a screen shot with a normal button near it to compare.

Comment 5

[Andrés Delfino]

Attachment: View button compared to Credits button of About window, with Windows XP theme.

Comment 6

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

Confirmed with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008041005 Minefield/3.0pre ID:2008041005

Comment 7

[Kai Liu]

Attachment: patch

The problem is caused by the huge box generated by the dropmarker.  The dropmarker element is defined as having a width and height of 16px, which is what bloated this button's size.  Setting the dropmarker height to auto allows the view button to shrink down to the normal button size.

* In addition to setting the height to auto, I also set the width of the dropmarker to auto.  There is a lot of horizontal white space surrounding the dropmarker, and with the button height returned to normal, this extra horizontal white space becomes much more pronounced and glaring.  I personally think that the spacing looks just right with both the width and height set to auto.  As a bonus, this affords the search box some extra pixels.

* I also set the top and bottom margins of the button itself to 2px.  By default, buttons have an asymmetric 1px/2px vertical margin, which resulted in the view button being out of vertical alignment by 1 pixel in non-classic themes.  By setting the top and bottom margins to the same value, the view button will always be properly aligned with the label and text box.  I set both the top and bottom margins (instead of just the top margin) so that the two would always be equal even if the margins specified in button.css ever get changed (if there is a certain assurance that this won't happen, then the margin-bottom line in this patch could be dropped)

Comment 8

[Kai Liu]

Attachment: Screenshot of what the patch looks like

This is what the sidebar looks like with the patch from comment 7.

Comment 9

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

Isn't the padding on the right side between the dropmarker and the buttons border a bit too wide? It looks like that the dropmarker is pushed as much as possible to the buttons text which makes the look a bit unbalanced.

Comment 10

[Kai Liu]

Attachment: View button on Firefox 2

(In reply to comment #9)
> Isn't the padding on the right side between the dropmarker and the buttons
> border a bit too wide? It looks like that the dropmarker is pushed as much as
> possible to the buttons text which makes the look a bit unbalanced.
> 

Now that you mention it, yes, it does look unbalanced.  But it has always been unbalanced.  I've attached a screenshot of what it looks like on Firefox 2, and it was unbalanced in Firefox 3 before this patch, too.  I guess because it had always been unbalanced, I had become so used to it that it never really caught my eye...

Comment 11

[Kai Liu]

Attachment: patch v2

Attempt to also address the whitespace imbalance

Comment 12

[Kai Liu]

Attachment: patch v2, screenshot

Comment 13

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

Comment on attachment 315566 [details] [diff] [review]
patch v2

I'm not the right person to review this. Ryan, can you take a peek and set approval1.9? if it gets review?

Comment 14

[Kai Liu]

Attachment: patch v2.1

Minor change to conform with the CSS guidelines at MDC

Comment 15

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

Comment on attachment 317480 [details] [diff] [review]
patch v2.1

a1.9=beltzner

Comment 16

[:Gavin Sharp [email: [e-mail]]]

For what it's worth, it would be best to use ".toolbarbutton-menu-dropmarker" rather than "#viewButton > hbox > dropmarker" if you know that it's the only dropmarker that can be styled by this CSS, but probably can't depend on that.

Comment 17

[:Gavin Sharp [email: [e-mail]]]

mozilla/browser/themes/winstripe/browser/places/places.css 	1.27 

Comment 18

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

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