396565 - Menubar is not tall enough for the text

Status: VERIFIED FIXED

(SeaMonkey :: UI Design, defect)

Description

[Boris Zbarsky [:bzbarsky]]

Starting with the 2007-09-05-02 build, the menubar is not actually tall enough for the text (or more precisely for the accesskey underlines).  Removing the height="0" thing that bug 282188 added fixes the problem.

We really can't ship this, imo.  It looks terrible.

Comment 1

[Boris Zbarsky [:bzbarsky]]

This is a problem on Linux with classic theme only.  The issue is that modern, and classic on Windows, have nonzero padding and transparent borders on "menubar > menu".  Linux classic, however, does not.

I'm not sure why Firefox is not showing this bug.  It's using the same style rules....

Comment 2

[Reed Loden [:reed]]

(In reply to comment #1)
> I'm not sure why Firefox is not showing this bug.  It's using the same style
> rules....

Firefox is showing this bug... see bug 396562.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Not the same bug.  This issue predated that one by several weeks, and has slightly different symptoms.

Comment 4

[Alfred Kayser]

This causes problems in my themes (LittleFox, Walnut, Nautipolis and others) because of the following statement in toolbar.xml:
    <content height="0">
      <xul:hbox flex="1">
        <children/>
      </xul:hbox>
    </content>

The menubar itself has now height 0 and is therefor placed strangely...

There seems to be no reason for this 'height="0"' and it is causing problems all over the place... This really needs to be fixed, as there is no way for theme authors to override this.

Comment 5

[Alfred Kayser]

Attachment: Screenshot of menubar problem in Nautipolis theme

This is what happens in all my themes.
According to DOM Inspector the menubar is actually 10px high, but the containing hbox has height 19px. Can someone explain me why this 'height:0' is needed?

Comment 6

[Robert Kaiser]

I wonder if that changes when we change to the toolkit style menubar-inside-toolbar thing for toolbar customization...

Comment 7

[Alfred Kayser]

This happens in Firefox, so not just Seamonkey, and it also happens on WinXP, so changing components and platforms...

Comment 8

[Boris Zbarsky [:bzbarsky]]

> This happens in Firefox, so not just Seamonkey

No.  The thing I filed this bug on does NOT happen in Firefox as far as I could determine.  There is a separate bug that has been filed on the newer regression that is observable in both browsers.  Again, see comment 2 and comment 3.  
In particular, for this bug the top of the menubar is fine, but the text overflows out the _bottom_.  I suppose I should have posted screenshots.  But in any case, your screenshot is bug 396562.  Please undo your changes and put things back to the way they were when I filed the bug.

Comment 9

[Alfred Kayser]

Back to Seamonkey, including the request for blocking2.0a1.
Note, bug 396562 and this one share the same cause: "height:0"

Comment 10

[Boris Zbarsky [:bzbarsky]]

Gah.  You still didn't fix the assignee....

Comment 11

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

(In reply to comment #0)
> Removing the height="0" thing that bug 282188 added fixes the problem.

Which height="0" thing? I don't see it in any of that bug's patches. The height=0 thing was added in bug 127244. Are you sure your regression range is correct, and that this isn't a dupe of bug 396562?

Comment 12

[Boris Zbarsky [:bzbarsky]]

> Which height="0" thing?

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/suite/common/bindings/toolbar.xml&rev=1.33&mark=226#225

> I don't see it in any of that bug's patches.

Then what landed wasn't one of the patches in the bug.

> The height=0 thing was added in bug 127244.

Apparently it landed with the wrong bug's patch, then.

> Are you sure your regression range is correct

Yes.  Compare the regression range I gave with the diff and date for rev 1.33 of suite/common/bindings/toolbar.xml.

> and that this isn't a dupe of bug 396562?

As I said in comment 8, this is a bug about the state of things between 2007-09-05 and when bug 396562 regressed a few weeks later.  It's possible that fixing bug 396562 will also fix this, of course.

Comment 13

[[e-mail]]

Attachment: Proposed patch

height="0" isn't needed because max-height: 0px; works (was fixed since my original patch to bug 282188).

Comment 14

[[e-mail]]

Fix checked in.

Comment 15

[Alfred Kayser]

Thanks, menubar is now fine again!

Comment 16

[Alfred Kayser]

Attachment: Another problem: menu/urlbar combination in popups doesn't work

There is still some problem with the menubar stuff.
Now in popups, the icons in the menubar/toolbar don't fit.

Comment 17

[Alfred Kayser]

Reopen or new bug?

Comment 18

[Reed Loden [:reed]]

(In reply to comment #16)
> There is still some problem with the menubar stuff.
> Now in popups, the icons in the menubar/toolbar don't fit. 

Bug 397873, probably?