403634 - <toolbar align="center"> et al not working as expected

Status: RESOLVED FIXED

(Toolkit :: UI Widgets, defect, P4)

Description

[Dão Gottwald [:dao]]

possible fix:

>   <binding id="toolbar" extends="chrome://global/content/bindings/toolbar.xml#toolbar-base">
>     <content>
>-      <xul:hbox flex="1">
>+      <xul:hbox flex="1" class="box-inherit" xbl:inherits="align,dir,pack,orient">
>         <children/>
>       </xul:hbox>
>     </content>

Comment 1

[[e-mail]]

I'm not sure class="box-inherit" will work.

Also, historically toolbars have only inherited align, orient and pack.

Comment 2

[Neil Deakin]

can you describe what the bug is here?

Comment 3

[[e-mail]]

IIRC the easiest way is to compare the Error Console before/after bug 397873.

Comment 4

[Dão Gottwald [:dao]]

The bug is that align="center" (for instance) set on the toolbar element used to center toolbar items vertically, while they are now stretched as if the attribute wasn't present. This is happening in the Places Organizer if you search for something.

(In reply to comment #1)
> I'm not sure class="box-inherit" will work.

While XBL is need to inherit the attributes, the box-inherit class is needed to inherit CSS properties like -moz-box-align.

> Also, historically toolbars have only inherited align, orient and pack.

This isn't about what a toolbar should inherit, it's about the hbox that was added in bug 397873.

Comment 5

[Dão Gottwald [:dao]]

(In reply to comment #4)
> This is happening in the Places Organizer if you search for something.

(In reply to comment #3)
> IIRC the easiest way is to compare the Error Console before/after bug 397873.

Yeah, the Error Console too.

Comment 6

[[e-mail]]

(In reply to comment #4)
>(In reply to comment #1)
>>I'm not sure class="box-inherit" will work.
>While XBL is need to inherit the attributes, the box-inherit class is needed to
>inherit CSS properties like -moz-box-align.
That would break bug 127244, which relies on the orient of the hbox to be horizontal while that of the toolbar is vertical while it is hidden.

>>Also, historically toolbars have only inherited align, orient and pack.
>This isn't about what a toolbar should inherit
I meant to say inherited onto their hbox. (I keep over-summarising. Sorry.)

Comment 7

[Dão Gottwald [:dao]]

(In reply to comment #6)
> >While XBL is need to inherit the attributes, the box-inherit class is needed to
> >inherit CSS properties like -moz-box-align.
> That would break bug 127244, which relies on the orient of the hbox to be
> horizontal while that of the toolbar is vertical while it is hidden.

Well, I don't think not inheriting properties like -moz-box-align is an option. It sounds like either -moz-box-orient should be set explicitly on the inner box or the other properties should be inherited explicitly without box-inherit.

> >>Also, historically toolbars have only inherited align, orient and pack.
> >This isn't about what a toolbar should inherit
> I meant to say inherited onto their hbox. (I keep over-summarising. Sorry.)

But wasn't the hbox just added? What's historical in that context?

Comment 8

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

I'm thinking we should back out the patch for bug 127244. I really don't think the benefit is worth all the headaches we've had to go through to avoid breaking things.

Comment 9

[[e-mail]]

(In reply to comment #7)
>But wasn't the hbox just added? What's historical in that context?
Before August 2002.

(In reply to comment #8)
>I'm thinking we should back out the patch for bug 127244. I really don't think
>the benefit is worth all the headaches we've had to go through to avoid
>breaking things.
OK, maybe I can revisit this after Fx3.

Comment 10

[[e-mail]]

Attachment: Back out

Comment 11

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

Comment on attachment 288675 [details] [diff] [review]
Back out

Also fixes bug 402333, and I would have expected it to fix bug 403365, but that bug seems to have always been broken (unless it's a recent regression - I can reproduce it with a branch build).

Comment 12

[[e-mail]]

Fix checked in.