User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20061107 Minefield/3.0a1 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20061107 Minefield/3.0a1 The text inside form buttons is not centered as it should be, but aligned farther to the right. Take a look at any button here on Bugzilla, or anywhere else. Reproducible: Always

Hrmph. This used to work; the tests at http://lxr.mozilla.org/seamonkey/source/layout/html/tests/formctls/base/button_overconstrained.html http://lxr.mozilla.org/seamonkey/source/layout/html/tests/formctls/base/button_overconstrained_strict.html http://lxr.mozilla.org/seamonkey/source/layout/html/tests/formctls/base/input_button_overconstrained_strict.html http://lxr.mozilla.org/seamonkey/source/layout/html/tests/formctls/base/input_button_overconstrained.html used to pass...

So this is a ComputeSize regression. nsHTMLButtonControlFrame::ReflowButtonContents munges the availSize of the child reflow state, but the child area frame no longer uses the availSize for its sizing -- it now uses the mComputedWidth.

This was fixed by switching ComputeSize to use aAvailableWidth.

Comment on attachment 247372 [details] [diff] [review] Fix minusing since patch is no longer needed

Added tests for this to reftest.

So test 1 is currently failing on Mac, and it seems like it ought to for any platforms without native-theme buttons. The rule on forms.css line 448 gives buttons 6px of horizontal padding. Now, what I don't understand is why the failure manifests itself only as a difference in subpixel positioning of the "M" glyph. (It looks like Mac does subpixel positioning of text.)

The rule in forms.css is more or less ignored for too-small button widths due to the code at http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/layout/forms/nsHTMLButtonControlFrame.cpp&rev=3.216&mark=355-372,378#354 As for subpixel positioning, we should probably round extraleft to the nearest pixel.

David, does that make your tests pass?

Why do you expect this to help?

Well, the only way I can think of for the text to not end up on a pixel boundary is if extraleft is an odd number of pixels and we end up with extraleft being a non-integral pixel offset....

And no, it does not make the test pass.

Comment on attachment 253974 [details] [diff] [review] Round to the nearest pixel OK. Which way is the test image text offset from the ref?

So, the differences between test and reference according to frame dumps are: ButtonControl (input): width=293 (in both test and reference) Area (input): test width=143 reference width=180 test overflow width=168; no overflow in reference Text: width=168 (in both test and reference) test x=0 reference x=6

The Area frame is at x=75 in both test and reference.

Oh, and that's all *with* your patch above.

Oh. Right. The problem is that in the reference the area is wider than the text, so the text-align:center kicks in and (180-168)/2 == 6 is exactly how far to the right it gets moved. Does setting "text-align: left" on the reference button fix the issue?

Yes, putting text-align: left on the input in 359903-1-ref.html makes the test pass. (But under what conditions should things really be centered? It seems like currently both are off-center, but by different amounts.)

(That said, adding text-align:left to both the reference *and* the test and then filing another bug for the centering might be the best path forward.)

Basically, the text is centered within the Area (whatever "text-align: center" does, including whatever it does when the text is wider than the Area). The problems start if the Area is wider than the content-width of the button. We attempt to center it within the button anyway by overlapping it with the left padding, up to the point where the left edge of the Area is up against the left padding edge. So if the Area's width is wider than the padding-box width of the button the text will not really end up centered in the button overall...

I'm seeing this failure in my Linux build too.

> That said, adding text-align:left to both the reference *and* the test Checked in this change.

...which makes 359903-1 pass and makes 359903-2 fail.

Ugh. Let me look into that.

Er, yeah. That change to -2 was just wrong; I backed it out. I should figure out why it was passing here...