Closed
Bug 381915
Opened 18 years ago
Closed 15 years ago
[Mac Classic] New tab/Close tab button incorrect positioned in suiterunner
Categories
(SeaMonkey :: Themes, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
seamonkey2.0a2
People
(Reporter: stefanh, Assigned: mstange)
References
Details
Attachments
(3 files)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
image/gif
|
Details |
Reporter | ||
Comment 1•18 years ago
|
||
(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.
Reporter | ||
Comment 2•18 years ago
|
||
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.
Reporter | ||
Comment 3•18 years ago
|
||
> - the close buttons lacks a tooltip"
This is old - the close tab button lacks a tooltip on branch too :-)
Reporter | ||
Comment 4•18 years ago
|
||
Hmm... this seems to work (note to myself): .tabs-closebutton > .toolbarbutton-text, .tabs-newbutton > .toolbarbutton-text { margin: 0px !important; }
Reporter | ||
Comment 5•18 years ago
|
||
Interestingly, label style rules from formatting.css is the cause here. Classic had !important in toolbarbutton.css, but Pinstripe doesn't.
Assignee: general → stefanh
Status: NEW → ASSIGNED
Attachment #268408 -
Flags: superreview?(neil)
Attachment #268408 -
Flags: review?(mnyromyr)
Comment 6•18 years ago
|
||
(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?
Reporter | ||
Comment 7•18 years ago
|
||
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.
Attachment #268408 -
Flags: superreview?(neil)
Attachment #268408 -
Flags: review?(mnyromyr)
Comment 8•18 years ago
|
||
Yes, please (this actually explains a bunch of display:none/toolbarbutton-image workarounds).
Reporter | ||
Comment 9•18 years ago
|
||
(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...
Reporter | ||
Comment 10•18 years ago
|
||
OK, here's a demo patch just to show what happen if I patch toolbarbutton.css. Screenshots coming...
Reporter | ||
Comment 12•18 years ago
|
||
Comment on attachment 268701 [details] Screenshots non-patched build vs build with attachment #268700 [details] [diff] [review] applied Sorry, checked the patch box...
Attachment #268701 -
Attachment is patch: false
Attachment #268701 -
Attachment mime type: text/plain → image/gif
Reporter | ||
Comment 13•18 years ago
|
||
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.
Reporter | ||
Comment 14•18 years ago
|
||
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...
Reporter | ||
Comment 15•18 years ago
|
||
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•18 years ago
|
||
(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?
Reporter | ||
Comment 17•18 years ago
|
||
(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.
Reporter | ||
Comment 18•17 years ago
|
||
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.
Reporter | ||
Comment 19•17 years ago
|
||
I'll try to get to this once I've got my OS/Hardware upgrade...
Status: ASSIGNED → NEW
Reporter | ||
Updated•16 years ago
|
Assignee: stefanh → nobody
Component: General → Themes
QA Contact: general → themes
Reporter | ||
Comment 20•15 years ago
|
||
Actually, mstange fixed this in bug 467091.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.0a2
Reporter | ||
Updated•15 years ago
|
Assignee: nobody → mstange
You need to log in
before you can comment on or make changes to this bug.
Description
•