Closed Bug 416801 Opened 15 years ago Closed 14 years ago

View button in History sidebar is stretched vertically

Categories

(Firefox :: Theme, defect)

x86
Windows XP
defect
Not set
trivial

Tracking

()

VERIFIED FIXED
Firefox 3

People

(Reporter: adelfino, Assigned: kliu)

References

Details

(Keywords: polish)

Attachments

(6 files, 3 obsolete files)

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
Version: unspecified → Trunk
Keywords: polish
Can someone please confirm this?
(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.
Summary: In Windows Classic mode, View button in History sidebar is stretched vertically → In Windows, View button in History sidebar is stretched vertically
Summary: In Windows, View button in History sidebar is stretched vertically → View button in History sidebar is stretched vertically in Windows
Summary: View button in History sidebar is stretched vertically in Windows → View button in History sidebar is stretched vertically
Component: General → Theme
QA Contact: general → theme
Confirmed with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008041005 Minefield/3.0pre ID:2008041005
Status: UNCONFIRMED → NEW
Ever confirmed: true
Attached patch patch (obsolete) — Splinter Review
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)
Attachment #315446 - Flags: review?(gavin.sharp)
Attached image Screenshot of what the patch looks like (obsolete) —
This is what the sidebar looks like with the patch from comment 7.
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.
(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...
Attached patch patch v2 (obsolete) — Splinter Review
Attempt to also address the whitespace imbalance
Attachment #315446 - Attachment is obsolete: true
Attachment #315566 - Flags: ui-review?(faaborg)
Attachment #315566 - Flags: review?(gavin.sharp)
Attachment #315446 - Flags: review?(gavin.sharp)
Attached image patch v2, screenshot
Attachment #315448 - Attachment is obsolete: true
Attachment #315566 - Flags: ui-review?(faaborg) → ui-review+
Attachment #315566 - Flags: review?(gavin.sharp)
Attachment #315566 - Flags: review?(beltzner)
Attachment #315566 - Flags: approval1.9?
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?
Attachment #315566 - Flags: review?(bryan)
Attachment #315566 - Flags: review?(beltzner)
Attachment #315566 - Flags: approval1.9?
Assignee: nobody → kliu.bugzilla.3c9f
Status: NEW → ASSIGNED
Attached patch patch v2.1Splinter Review
Minor change to conform with the CSS guidelines at MDC
Attachment #315566 - Attachment is obsolete: true
Attachment #317480 - Flags: review?(gavin.sharp)
Attachment #315566 - Flags: review?(bryan)
Whiteboard: [has patch][has ui-review][needs review gavin]
Attachment #317480 - Flags: review?(gavin.sharp) → review+
Whiteboard: [has patch][has ui-review][needs review gavin] → [has patch][has ui-review]
Attachment #317480 - Flags: approval1.9?
Comment on attachment 317480 [details] [diff] [review]
patch v2.1

a1.9=beltzner
Attachment #317480 - Flags: approval1.9? → approval1.9+
Keywords: checkin-needed
Whiteboard: [has patch][has ui-review] → [needs landing]
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.
mozilla/browser/themes/winstripe/browser/places/places.css 	1.27 
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [needs landing]
Target Milestone: --- → Firefox 3
Looks great with Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9pre) Gecko/2008050606 Minefield/3.0pre ID:2008050606
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.