348516 - Auto margins don't work on <option>s inside <select>

Status: RESOLVED FIXED

(Core :: Layout: Form Controls, defect)

Description

[Uri Bernstein]

When the widths of a <select> box and an <option> contained in it are specified explicitly, it should be possible to control the alignment of the <selcet> by setting "auto" horizontal margins on it (left for right-aligned, left and right for center-align). This does not work - the <option> block is always left-aligned.

This is blocking bug 203353, because the same mechanism used for dealing with auto margins is also used to align the options to the right by default, when the <select> is RTL.

Testcase coming up.

Comment 1

[Uri Bernstein]

Attachment: testcase

The cyan options in both <select> controls should be aligned like the divs inside the reference div on the bottom.

Comment 2

[Uri Bernstein]

Here's what I found:

nsHTMLReflowState::CalculateBlockSideMargins(), which should deal with the auto margins, exits immediately because NS_UNCONSTRAINEDSIZE == aComputedWidth.

aComputedWidth is "unconstrained" because "Inline non-replaced elements do not have computed widths or heights" - http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/layout/generic/nsHTMLReflowState.cpp&rev=1.238&mark=1966-1967#1966

The frame in question here is a nsSelectsAreaFrame. The frame type for it is set to NS_CSS_FRAME_TYPE_INLINE, because its display type is NS_STYLE_DISPLAY_INLINE.

I'm not sure why this frame should be of display NS_STYLE_DISPLAY_INLINE, and I couldn't even find where this is decided.

I'd appreciate any help I can get on this, so CC-ing the layout team. If you're not interested in this, please feel free to un-CC yourself.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Fixing things like this is why the reflow branch exists.  And in fact, that testcase passes on the reflow branch.  ;)

Comment 4

[Uri Bernstein]

Fixed by the reflow branch landing.

Verified with:
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20061208 Minefield/3.0a1 ID:20061208064

Comment 5

[Boris Zbarsky [:bzbarsky]]

Need regression test.

Comment 6

[arno renevier]

Attachment: reftest

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

The reftest looks fine, although it might be good to have an additional != test to make sure that margins are doing something at all rather than completely breaking (by comparing the reference to an equivalent without margins).

Do you need me to check this in for you?  I can add the additional test.

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I checked in the reftest, along with a few additional tests to make sure that margins don't break entirely.

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 259184 [details] [diff] [review]
reftest

Oops, didn't notice you weren't cc:ed on the bug.  Anyway, this test is fine; I checked it in, along with a few additional tests that I added (see above).

Comment 10

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

And thanks for the test.