Closed
Bug 348698
Opened 18 years ago
Closed 18 years ago
Menu bar no longer expands to accomodate icons when toolbars are customized
Categories
(Firefox :: Toolbars and Customization, defect, P1)
Tracking
()
RESOLVED
FIXED
Firefox 2
People
(Reporter: Kensie, Assigned: Gavin)
References
Details
(Keywords: fixed1.8.1, regression, Whiteboard: [Fx2 theme change])
Attachments
(4 files)
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
patch
|
mconnor
:
review+
mconnor
:
approval1.8.1+
|
Details | Diff | Splinter Review |
(deleted),
image/png
|
Details |
In 1.5.0.x if you choose to move icons (large, I believe) to the menu bar (where File Edit etc. are) it makes the menu bar as tall as the toolbar that generally has the back, forward, refresh etc. icons.
Under the new theme, the bar does not expand and the icons reach the top and bottom edges of the bar. Can see this on Windows, don't have Linux to test on.
Reporter | ||
Comment 1•18 years ago
|
||
note- this also depends on what font size you're using, i.e. if your menu fonts are large enough you won't see that the menu bar is too small (and therefor doesn't expand) as in your case it won't be.
Flags: blocking-firefox2?
Comment 2•18 years ago
|
||
Updated•18 years ago
|
Flags: blocking-firefox2? → blocking-firefox2+
Whiteboard: [Fx2 theme change]
Target Milestone: --- → Firefox 2
Comment 3•18 years ago
|
||
I don't see this on Linux (Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1b1) Gecko/20060815 BonEcho/2.0b1).
Comment 4•18 years ago
|
||
*** Bug 349156 has been marked as a duplicate of this bug. ***
Comment 5•18 years ago
|
||
Trying to imitate the appearance of the browser in the mock-up, if I change padding to 3px for top and 2px for bottom, also the other toolbars (navigation toolbar, extra toolbar) will look better. This appears to be forgotten in the code (or it is left out for some reason).
.toolbarbutton-1, .toolbarbutton-menubutton-button {
padding-top: 3px !important;
padding-bottom: 2px !important;
}
Comment 6•18 years ago
|
||
Gavin, please take a look at the old padding/margin values and see how we compare. I'm not sure we want to go back completely, but we can stand a bit bigger clickable areas vertically.
Assignee: nobody → gavin.sharp
Comment 7•18 years ago
|
||
*** Bug 350526 has been marked as a duplicate of this bug. ***
Assignee | ||
Updated•18 years ago
|
Status: NEW → ASSIGNED
Priority: -- → P1
Assignee | ||
Comment 8•18 years ago
|
||
This patch removes the overriding of the standard toolbarbutton padding across all toolbarbuttons, and adds two rules for the back and forward buttons specifically. This results in more padding around all the toolbarbuttons than there currently is. I picked 5px to match the previous theme, but perhaps 3 or 4 pixels would be preferred - I'll attach a screenshot of all the options for comparison.
Myk tested this on linux, and it has no negative effects since the native themeing overrides these rules there anyways. I've tested this with Windows XP Luna and Classic. I also tested the back/forward button changes in RTL mode.
Attachment #235967 -
Flags: review?(mconnor)
Assignee | ||
Comment 9•18 years ago
|
||
Assignee | ||
Updated•18 years ago
|
Whiteboard: [Fx2 theme change] → [patch-r?][Fx2 theme change]
Reporter | ||
Comment 10•18 years ago
|
||
is it just me or do the toolbars have padding that add on to the button padding? Notice if you drag the url bar to the current menubar the menubar expands a little for it. It also seems in Gavin's screenshots that the toolbar with the url and search bars is slightly taller than the menubar with the added icon.
Reporter | ||
Comment 11•18 years ago
|
||
never mind, it's an optical illusion as the toolbar has a bevel on top whereas the menubar has the dark blue on top.
Comment 12•18 years ago
|
||
Gavin,
Don't forget to test adding links to the toolbar. If I move a link from the bookmark toolbar to the navigation toolbar, the navigation toolbar "grows" vertically (at least this occurs under classic view. Make sure the min-height takes that into account.
~B
Comment 13•18 years ago
|
||
BTW, IMO 3px looks just about right! Thanks for the time you're putting into this...
~B
Comment 14•18 years ago
|
||
Comment on attachment 235967 [details] [diff] [review]
patch
r+a=me on this theme patch. went over the options with beltzner, and we're going to go with 5px for now. thanks for cleaning up the margin/padding thing so the targets are bigger again.
Attachment #235967 -
Flags: review?(mconnor)
Attachment #235967 -
Flags: review+
Attachment #235967 -
Flags: approval1.8.1+
Assignee | ||
Comment 15•18 years ago
|
||
Checked in on the branch.
mozilla/browser/themes/winstripe/browser/browser.css 1.17.2.45
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Keywords: fixed1.8.1
Resolution: --- → FIXED
Whiteboard: [patch-r?][Fx2 theme change] → [Fx2 theme change]
Comment 16•18 years ago
|
||
(In reply to comment #14)
> (From update of attachment 235967 [details] [diff] [review] [edit])
> r+a=me on this theme patch. went over the options with beltzner, and we're
> going to go with 5px for now.
Could we possibly go with 3px as 5px seems offly "off", especially in Classic? I believe there is a bug coming up for this.
~B
Comment 17•18 years ago
|
||
(In reply to comment #16)
> (In reply to comment #14)
> > (From update of attachment 235967 [details] [diff] [review] [edit] [edit])
> > r+a=me on this theme patch. went over the options with beltzner, and we're
> > going to go with 5px for now.
>
> Could we possibly go with 3px as 5px seems offly "off", especially in Classic?
> I believe there is a bug coming up for this.
>
> ~B
>
Bug 350736 (Since 0830 build, the Toolbars look very large and ugly)
Comment 18•18 years ago
|
||
*** Bug 351019 has been marked as a duplicate of this bug. ***
You need to log in
before you can comment on or make changes to this bug.
Description
•