347470 - New theme icons have too much intervening horizontal space on Linux

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Peter Kasting]

On Linux, the new theme icons have too much horizontal space between them.  This is possibly caused by the same cause as bug 347447, where the icons all have padding around them for the old "draw button border on hover" effect that apparently wasn't taken into account in the new theme design.  (I don't think this effect is consistent with the rest of my OS, so it would be OK to remove it.)

Screenshot to follow shortly.

Comment 1

[Peter Kasting]

Attachment: Large icons, new (top) vs. old (bottom)

Comment 2

[Peter Kasting]

Attachment: Small icons, new (top) vs. old (bottom)

Comment 3

[Declan Naughton]

remove the location bar and the buttons get smaller (vertically). The location bar it too big. There is still much-too-much horizontal spacing between the buttons however.

Comment 4

[Declan Naughton]

Attachment: path for browser.css inside classic.jar, just so ye have an idea what I did. I can't find where to make the changes in the source tree :/

just removes margins.

Comment 5

[Declan Naughton]

Attachment: screenshot with margins removed of small icons

once we fix the size issue with the location bar they'll look even smaller vertically - see the other screenshot

Comment 6

[Declan Naughton]

Attachment: screenshot without the location bar on the toolbar

there's no location bar on the toolbar this time, so the buttons are smaller (only vertically).

Comment 7

[Declan Naughton]

(In reply to comment #3)
> remove the location bar and the buttons get smaller (vertically). The location
> bar it too big. There is still much-too-much horizontal spacing between the
> buttons however.


For that I filed bug 348002.


For this, I unzipped my classic.jar and messed about in browser.css - removing the margins on the buttons when the icons are small. Now there's /no/ spacing between any of the buttons, and it doesn't look too bad. See the screenshot.

Anyone know where my clasic.jar -> browser.css would've been generated from in the source tree?

I think this should be OS -> All btw.

Oh ****, I just noticed you weren't just talking about small icons...

Comment 8

[Declan Naughton]

FWIW, in Firefox 1.5 there are no margins with small and large icons on.

Comment 9

[Declan Naughton]

Attachment: removes the margins on the small icons

Comment 10

[Declan Naughton]

Attachment: removes the margins from the icons

Comment 11

[Declan Naughton]

(In reply to comment #7)
> I think this should be OS -> All btw.

Actually Windows doesn't have the margins.

Comment 12

[Reed Loden [:reed]]

Comment on attachment 233017 [details] [diff] [review]
removes the margins from the icons

Er, you need to request review for a patch, not grant review. :P

Just set the review flag to '?' and optionally put a reviewer.

Comment 13

[Mike Connor [:mconnor]]

Can someone test this for me on Windows to ensure that it doesn't impact Windows appearance?

Comment 14

[Mike Connor [:mconnor]]

Comment on attachment 233017 [details] [diff] [review]
removes the margins from the icons

this patch kills all spacing on Windows, not so ok.

Comment 15

[Declan Naughton]

We have a problem...

I vote for reverting to the old theme :P

Comment 16

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

This should get worked out for beta in some fashion, even if it's a temporary workaround/hack of some sort. We can't break the Linux users this badly for something where we want to get as widespread testing as beta2.

Comment 17

[Peter Kasting]

Well, we needed the margins because we removed the "button" sort of look on win32, and they filled in the space, right?

The main problem on Linux is that the "search" button (the search bar right endcap) is the only button drawn without this "button" look.  All the others still  get that effect onhover.

I think if all the other buttons were made to draw like that search button does, this problem would just vanish.

Alternatively, if the "button" look is restored on win32, the margins will go away, and this problem should just fix itself.

Comment 18

[Michael Wu [:mwu]]

Attachment: Simplify css for toolbar buttons, special case gtk2

Comment 19

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

(In reply to comment #17)
> Alternatively, if the "button" look is restored on win32, the margins will go
> away, and this problem should just fix itself.

Not quite true, depending on what you mean by "button" look. The hover effect is fixed by bug 347416, but the padding isn't (see bug 347416 comment 34).

Comment 20

[Michael Wu [:mwu]]

Attachment: Simplify css for toolbar buttons, special case gtk2, v2

Comment 21

[Mike Connor [:mconnor]]

Comment on attachment 233930 [details] [diff] [review]
Simplify css for toolbar buttons, special case gtk2, v2

r+a=me, thanks Michael!

Comment 22

[Michael Wu [:mwu]]

Attachment: Simplify css for toolbar buttons, special case gtk2, v3

Comment 23

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

mozilla/browser/themes/winstripe/browser/jar.mn 	1.13.2.17
mozilla/browser/themes/winstripe/browser/browser.css 	1.17.2.34
mozilla/browser/themes/winstripe/browser/searchbar.css 	1.3.8.15