338340 - Back and forward buttons too small in "Icons and Text" mode

Status: RESOLVED FIXED

(Firefox :: Toolbars and Customization, defect)

Description

[James C]

User-Agent:       Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8.1a2) Gecko/20060517 BonEcho/2.0a2
Build Identifier: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8.1a2) Gecko/20060517 BonEcho/2.0a2

The fix for bug 336887 removed the min-width for buttons with the class .toolbarbutton-menubutton-button when toolbars are set to show icons and text. This results in no min-width being set for the back or forward button making them smaller than other buttons.

Reproducible: Always

Steps to Reproduce:
1.Right-click toolbar and select "Customize..."
2.Set to show Icons and Text
3.Hover over buttons to make size clear

Actual Results:  
Back and forward buttons are noticeably smaller than other buttons.

Expected Results:  
All buttons should be at least 57px wide.

Comment 1

[Ria Klaassen (not reading all bugmail)]

Personally I think the proportions are better now, because of the little triangles that make the buttons already wider visually. But I'm not a Winstripe user :)

Comment 2

[Charli Li (winsxs)]

(In reply to comment #1)
> But I'm not a Winstripe user

I use winstripe sometimes, and this WFM on Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a1) Gecko/20060517 Minefield/3.0a1 - Build ID: 2006051706
---
James, could you please try to reproduce this on the latest stable (1.5.0.3), trunk nightly (3.0a1), or the Mozilla1.8.0 nightly (1.5.0.4)?

Comment 3

[Charli Li (winsxs)]

(In reply to comment #2)
> James, could you please try to reproduce this on the latest stable (1.5.0.3),
> trunk nightly (3.0a1), or the Mozilla1.8.0 nightly (1.5.0.4)?
> 

Oops, sorry about the spam.  I forgot that 3.0 is not compatible with Win98

Comment 4

[Ria Klaassen (not reading all bugmail)]

Charli, the patch of Bug 336887 is not checked in on 1.5.0.3 so it is superfluous to ask the reporter to test it on the release. 

Comment 5

[James C]

As has been pointed out, the patch has not been checked in on the 1.8.0 branch and I can't test on 3.0. Just to clarify, the entire button (including dropdown markers) will be at least 57px but the main part of the button, i.e. the bit you click on to perform the main function, is smaller than a normal button.

Comment 6

[Ria Klaassen (not reading all bugmail)]

Hm, I don't see the min-width removed:
http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&file=browser.css&branch=&root=/cvsroot&subdir=mozilla/browser/themes/winstripe/browser&command=DIFF_FRAMESET&rev1=1.34&rev2=1.35
Although it's sure that this patch has caused this. When I revert it the buttons are wider again.
Martijn. I'm sure you know what's going on :)

Comment 7

[James C]

The patch causes the min-width to only be applied to buttons with the class toolbarbutton-1 (e.g. stop, reload and the entire back/forward button including dropmarker). The back/forward button have the class toolbarbutton-menubutton-button so the min-width is no longer applied to them.

Comment 8

[Ria Klaassen (not reading all bugmail)]

OK, you're right. Very curious if there's another solution possible.

Comment 9

[James C]

I would have thought that the best solution would be to revert that patch and add something to overrule it only for the search button.

Comment 10

[Martijn Wargers (dead)]

Attachment: patch

Sorry, I could have known the css rule I removed was useful for something. But I didn't see this regression (I can see it now, although I must say it's not a very noticeable problem). I also did look at bonsai ( http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/browser/themes/winstripe/browser/browser.css&rev=1.35 ) to see what the css rule was good for, but bonsai wasn't really helpful.

This patch basically reverts that patch and adds a css rule in searchbar.css.
I need to use !important, since searchbar.css is an xbl stylesheet and those have a lower precedence as other stylesheets (see bug 334398).

An alternative would be to not allow styling anonymous content inside the searchbar, other than searchbar.css, but I'm not sure what kind of (negative) effects that would have.

Comment 11

[Asaf Romano (gone)]

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

r=mano

Comment 12

[Darin Fisher]

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

a=darin on behalf of drivers for the MOZILLA_1_8_BRANCH.  Please add the fixed1.8.1 keyword once this change is committed.

Comment 13

[Martijn Wargers (dead)]

Checked into trunk and 1.8.1 branch.
The checkin comment on trunk went a little wrong, sorry about that.