381915 - [Mac Classic] New tab/Close tab button incorrect positioned in suiterunner

Status: RESOLVED FIXED

(SeaMonkey :: Themes, defect)

Description

[Stefan [:stefanh]]

Comment 1

[Stefan [:stefanh]]

(too fast with hitting the button)

The New Tab and Close Tab buttons are not accurately positioned in suiterunner (Classic theme). It looks like both button has too much margin/padding at the right. See https://bugzilla.mozilla.org/attachment.cgi?id=261571 in bug 376912.

Comment 2

[Stefan [:stefanh]]

jftr: In bug 376912, Karsten pointed out those issues with the buttons:

"- both buttons don't give hover feedback, so you don't know they're buttons
- the close buttons lacks a tooltip"

I don't know if I want a hover (and the xpfe hover:active) effect, since it's not very macish. I've actually thought of filing bugs for removing at least the hover:active movement effect on our toolbar buttons (back, fwd etc). I think we need a new button look, so it's obvious that we're dealing with a close button, or have some very light and sublime hover effect (not the old xpfe effect).

The tooltip issue could be a cocoa issue. I'll dig a bit when I get back from vacation.

That said, I belive those things should be separate bugs.

Comment 3

[Stefan [:stefanh]]

> - the close buttons lacks a tooltip"

This is old - the close tab button lacks a tooltip on branch too :-)

Comment 4

[Stefan [:stefanh]]

Hmm... this seems to work (note to myself):

.tabs-closebutton > .toolbarbutton-text,
.tabs-newbutton > .toolbarbutton-text { margin: 0px !important; }

Comment 5

[Stefan [:stefanh]]

Attachment: Fix position of tabbrowser buttons

Interestingly, label style rules from formatting.css is the cause here. Classic had !important in toolbarbutton.css, but Pinstripe doesn't.

Comment 6

[[e-mail]]

(In reply to comment #5)
>Interestingly, label style rules from formatting.css is the cause here. Classic
>had !important in toolbarbutton.css, but Pinstripe doesn't.
As does gnome/winstripe. Wouldn't it be better to fix toolbarbutton.css?

Comment 7

[Stefan [:stefanh]]

Comment on attachment 268408 [details] [diff] [review]
Fix position of tabbrowser buttons

(In reply to comment #6)
> (In reply to comment #5)
> >Interestingly, label style rules from formatting.css is the cause here. Classic
> >had !important in toolbarbutton.css, but Pinstripe doesn't.
> As does gnome/winstripe. Wouldn't it be better to fix toolbarbutton.css?

I'll take a look.

Comment 8

[Asaf Romano (gone)]

Yes, please (this actually explains a bunch of display:none/toolbarbutton-image workarounds).

Comment 9

[Stefan [:stefanh]]

(In reply to comment #8)
> Yes, please (this actually explains a bunch of display:none/toolbarbutton-image
> workarounds).
> 

Are you sure? From what I can see in DOMi toolbarbutton labels with .toolbarbutton-text (like the Stop button in Minefield) currently have their margins from formatting.css here: http://lxr.mozilla.org/seamonkey/source/toolkit/themes/pinstripe/global/formatting.css#140 (which looks like an odd bug to me). If I use !important on the margin style rules for .toolbarbutton-text I can imagine that all toolbarbutton labels will have 0 margin instead of the current one...

Comment 10

[Stefan [:stefanh]]

Attachment: Demo patch, !important in toolbarbutton.css

OK, here's a demo patch just to show what happen if I patch toolbarbutton.css. Screenshots coming...

Comment 11

[Stefan [:stefanh]]

Attachment: Screenshots non-patched build vs build with attachment #268700 applied

Note the "compact" look with the patch applied.

Comment 12

[Stefan [:stefanh]]

Comment on attachment 268701 [details]
Screenshots non-patched build vs build with attachment #268700 [details] [diff] [review] applied

Sorry, checked the patch box...

Comment 13

[Stefan [:stefanh]]

OK, so I talked to Mano about this. The correct solution here would be to fix toolbarbutton.css and then put the right margin on the toolbarbuttons itself. Then we could also probably remove a lot of display:none hacks. I'm just a bit afraid that this will regress some things.

Comment 14

[Stefan [:stefanh]]

Yes, this will regress lots of things. The problem will be the large toolbarbuttons. I'll only need the margins from formatting.css in toolbar[mode="full"]. And those margins could probably not be the same either since just applying them on the buttons instead of the labels will not make it look the same (naturally). Hmm... 

Comment 15

[Stefan [:stefanh]]

I could of course add the .toolbarbutton-text margins (in toolbar.css) on the large toolbarbuttons when toolbar[mode="full"]. The thing is, I don't see any reasonable/smooth way of doing this if the idea is that everything should look exactly the same as it looks now... If I add margin/padding to the toolbarbutton itself, this will also change the look of mode="icons" and mode="text", since the margin/padding on the toolbarbuttons will stay the same in those modes.

Comment 16

[[e-mail]]

(In reply to comment #15)
>I don't see any reasonable/smooth way of doing this if the idea is
>that everything should look exactly the same as it looks now...
OK, so what (currently) are the computed styles for the margins of the toolbarbutton text for a) full b) text toolbarbuttons?

Perhaps only <toolbarbutton class="toolbarbutton-1"> needs special margins?

Comment 17

[Stefan [:stefanh]]

(In reply to comment #16)
> (In reply to comment #15)
> >I don't see any reasonable/smooth way of doing this if the idea is
> >that everything should look exactly the same as it looks now...
> OK, so what (currently) are the computed styles for the margins of the
> toolbarbutton text for a) full b) text toolbarbuttons?

(Minefield)

a) full mode is 1px 5px 2px 6px (from formatting.css)
b) text mode is mixed: type="menubutton" have the same as full mode, but normal ones has 4px margin (from toolbar.css).
> 
> Perhaps only <toolbarbutton class="toolbarbutton-1"> needs special margins?
> 

Yes, it seems so. I just wonder where I should put that stuff... suite has button.css in communicator, but toolkit doesn't really use .toolbarbutton-1 for anything else than adding images.

Comment 18

[Stefan [:stefanh]]

JFTR: I've been looking at this, but I'm still looking at what would be the best strategy here. I haven't really had the time to sit down and think about it either. I'll take a more detailed look at this after the weekend.

To summarize:

The best way is probably to try and avoid any margin on .toolbarbutton-text unless we have a very special case. Also, adding padding/margin to .toolbarbutton-1 is probably the way to go here. The fact that some toolbars (Firefox BM) doesn't use toolbarbutton-1 buttons complicates things, though - but I guess that could be fixed by adding the class (unless it regresses other OS, of course). Or the relevant BM .css file could be changed.

Note also that this won't change SeaMonkey, since SeaMonkey has it's own style rules for .toolbarbutton-1 in classic/communicator/button.css

One question here is where to put the style rules for the different apps that this affects (Thunderbird, Firefox, maybe Calendar, etc). I suppose I could add the rules in the different app-specific files, even though it feels a bit like they could go in one file somewhere.

Comment 19

[Stefan [:stefanh]]

I'll try to get to this once I've got my OS/Hardware upgrade...

Comment 20

[Stefan [:stefanh]]

Actually, mstange fixed this in bug 467091.