242967 - mail button is malformed

Status: RESOLVED WORKSFORME

(Firefox :: Toolbars and Customization, defect)

Description

[miahz]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.6) Gecko/20040206 Firefox/0.8
Build Identifier: 

the structure of the mail button is needlessly complex.  it contains two nested
boxes, when none are needed, and also contains orientation attributes that break
themes. the patch for bug 237959 was completely unecessary, and actually didn't
fix the problem - it just changed the problem.  button orientation is handled by
the theme.  using the standard menu-type toolbarbutton will greatly simplify
things and fix these problems.

at first, looking thru browser.xul, it's unclear where these extra boxes come
from.  but with a little digging, it would appear the mail-button uses the
"menu-orient" binding instead of the standard "menu".
http://lxr.mozilla.org/mozilla/source/toolkit/content/widgets/toolbarbutton.xml#33

current structure:
toolbarbutton orient="vertical"
  hbox
    box
      image/
      label/
    /box
    dropmarker/
  /hbox
/toolbarbutton

proposed structure:
toolbarbutton
  image/
  label/
  dropmarker/
/toolbarbutton

unless there's some yet-to-be-used functionality planned for the mail button
which necessitates this structure, it should be changed to the regular "menu" type.

precedent exists in the DOM Inspector panel buttons, the Web Developer
extension's toolbar buttons, and even the regular old toolbar bookmark folders.


Reproducible: Always
Steps to Reproduce:




this bug can also be a tracker for all mail button issues:
bug 237959 - mail button label orientation
bug 237900 - mail button needs icon

see also the forum discussion at the attached url.

Comment 1

[Stephen Clavering]

With your proposed structure (which is the same as the standard type=menu
binding) it would be impossible to have the button text below the icon, but with
the dropmarker on the right.

If themes are to be allowed to set toolbar button orientation then a modified
version of the menu-orient binding where the anonymous <box/> has a class
attribute (so its orientation can be specified in CSS), rather than inheriting
the orient attribute, should be used.

Comment 2

[miahz]

bingo - good point.  i overlooked that, as my theme has horizontally aligned
labels, so it naturally worked out.  so yeah, disregard the type="menu" stuff
from the top.

but regardless, the orientation needs to be dealt with and perhaps the extra box
as well.  so how about something more similar to a type="menu-button" with an
actual nested button, but without the stack:

toolbarbutton[orient="horizontal"]
  toolbarbutton[orient not inherited, but defined by the theme]
    image/
    label/
  /toolbarbutton
  dropmarker/
/toolbarbutton

and as clav said, the nested button needs some way - perhaps a unique class - to
deal with the orient inheritance.  what would happen if inherits="align" was
removed from the nested button?  would it then pick up its orientation from the
theme rules?

Comment 3

[Simon Paquet [:sipaq]]

*** Bug 259999 has been marked as a duplicate of this bug. ***

Comment 4

[Peter van der Woude [:Peter6]]

can you check if this is still the case in FF1.0PR or RC1 ?

Comment 5

[miahz]

Yep.  Hasn't changed.  Comments above still apply.

Comment 6

[Peter van der Woude [:Peter6]]

ok, setting ->NEW

Comment 7

[Phil Ringnalda (:philor)]

No more mail button, no more problem.