Checkboxes are being squashed horizontally. It's very odd looking. Doesn't happen to all checkboxes, just a lot of them. See https://backstage.site5.com for example. Attached a screenshot as well.

This is caused by width:auto. dbaron, is there a way to specify the width that should be used when it's set to auto? Or should we just restore the minimum size? This is a regression from bug 394892 which removed the minimum size for checkboxes and radiobuttons.

The issue is the width:auto styling, which overrides the width setting from forms.css. See also bug 373852. I should note that "-moz-appearance:none" works correctly, so this sounds like a Mac widgetry issue. Testcases: data:text/html,<input type="checkbox" style="width: auto; -moz-appearance: none"> data:text/html,<input type="checkbox" style="width: auto">

Wouldn't leaving the mimimum widget size but allowing it to be overridden do the right thing?

I'll try that.

Allowing the override doesn't allow them to be made smaller than the minimum size. It only allows them to be made larger.

Hmm. I would have thought that setting a width smaller than that would work. Perhaps we need a GetPrefSize API for native theme...

Another testcase that I showed Markus and Josh on IRC: 1. Load http://support.mozilla.com/en-US/kb/Back+and+forward+or+other+toolbar+buttons+are+missing 2. Click on the first "Yes" button at the bottom of the article (while logged out)

(In reply to comment #8) Note that this testcase also stretches the radios in Firefox 2 (on Mac), just not on Firefox 3.0 because we accidentally changed something.

So I think there are two things we need to do here: 1. Increase the widget border so that width: auto; results in a correct width. 2. Keep the aspect radio when scaling and add space around the buttons if the requested size isn't square.

Note that the added space might need to be negative if we're looking for IE compat here.

This is what IE7 on Windows does to the testcase in attachment 341352 [details]. Thanks to Boris for the screenshot. Note that the scaling behavior is not symmetrical wrt width / height. To be specific, the "negative space" case is only hit if (width > height && height < smallSize && width >= smallSize)... I'm not sure if we want to emulate that part of IE compatibility.

Having our controls take up more space than IE's is a good way to break sites, for what it's worth...

Let me clarify. I'm proposing this solution: renderedWidth = renderedHeight = min(width, height) I don't think our controls can take up more space than IE's that way; in some cases, they will be smaller, though.

Ah, ok. I can live with that.

When I set width: auto on the radio / checkbox, it becomes 9px wide. Boris, do you have any idea what causes this? I couldn't find it. Ideally, width: auto should result in a width of 13px. I could just add 2px of padding in GetWidgetPadding, but I'd rather find the real cause. Setting 2px padding also shifts the button's baseline - which looks really good.

> Boris, do you have any idea what causes this? See bug 373381 and bug 373852 maybe?

Oh, that's it. Thanks.

This patch does four things: 1. It sets the button's widget border to 2px to match the border in forms.css. This fixes width/height: auto because nsFormControlFrame:: GetIntrinsicHeight assumes a border width of 2px, too. 2. It makes sure that the drawRect is always square. 3. It changes the "snap tolerance" from 1px to 2px because that looks better and because IE snaps, too, so there's no compatibility concern. 4. It removes a stupid assumption in the snapping algorithms that's no longer true due to 3.

of attachment 341352 [details]

Comment on attachment 351052 [details] [diff] [review] v1 Looks reasonable to me, but I'd like dbaron to sr

Comment on attachment 351052 [details] [diff] [review] v1 I'd like to review this before checkin.

Comment on attachment 351052 [details] [diff] [review] v1 >+ case NS_THEME_CHECKBOX: >+ case NS_THEME_RADIO: >+ { >+ // This needs to be set to 2px because that's what forms.css uses. >+ // Otherwise we'll get wrong sizes when setting width / height: auto. >+ aResult->SizeTo(2, 2, 2, 2); >+ break; >+ } >+ Could you please point specifically to nsFormControlFrame::GetIntrinsicWidth and nsFormControlFrame::GetIntrinsicHeight as the reason you need this? With that, sr=dbaron

I'm worried that your patch's higher snap tolerance will make things like this worse. This is from Wells Fargo Bank. Your patch won't break page layout due to control width/height, but it may make rendering inconsistent between Gecko platforms. Windows and Linux probably render this example much better because they don't do much render snapping if any.

Push buttons don't snap. DrawPushButton directly calls DrawCellWithScaling and not DrawCellWithSnapping.

pushed: http://hg.mozilla.org/mozilla-central/rev/e6ef326c3cf0

Verified with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20081212 Minefield/3.2a1pre ID:20081212020331 with the given URL and the steps from comment 8. Markus, will the latest patch apply cleanly on the 1.9.1 branch? If yes, could you ask for approval?

Comment on attachment 351916 [details] [diff] [review] v2 a191=beltzner; be sure to watch out for instances of the wells-fargo thing as Josh points out.

Landed on 1.9.1: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/86b3cb75f3df

(In reply to comment #30) > (From update of attachment 351916 [details] [diff] [review]) > a191=beltzner; be sure to watch out for instances of the wells-fargo thing as > Josh points out. Beltzner or Josh, how can I check this thingy?

This bug can't cause the problem Josh pointed out in comment 26 due to the reason I gave in comment 27.

Verified with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b3pre) Gecko/20090103 Shiretoko/3.1b3pre ID:20090103020545

This is my first run to create a reftest. Would it be ok to get it into the testsuite?

Sorry, but I have to reopen this bug. This fix doesn't solve the problem completely. If a new height and width is applied to input fields they are still stretched. I'll attach an enhanced reftest and screenshot of the current behavior.

This reftest also tests radio buttons and takes a given height into account. See attachment 355242 [details] for the current behavior.

Comment on attachment 355243 [details] [diff] [review] Reftest for both types and height/width It doesn't seem like this test would be valid for all platforms. Maybe use JavaScript to make both test and reference blank on non-Mac? Also, the reftest.list should list the reference rather than listing the same file twice.

When the width is set, it needs to be respected. So the attachment in comment 38 is showing the right behavior as far as I can tell. This bug was about sizing issues with auto size. Re-resolving, and restoring the flags that should be here.

Oh, and the reftest looks incorrect to me, of course.

(In reply to comment #41) > When the width is set, it needs to be respected. So the attachment in comment > 38 is showing the right behavior as far as I can tell. This bug was about > sizing issues with auto size. Re-resolving, and restoring the flags that > should be here. With that said, you mean that checkboxes and radiobuttons can be drawn in user-defined sizes? If the answer is yes or no, we have differences between OS X and Windows. Resizing these types of input fields on Windows isn't possible. Shall I file a new bug on that?

> you mean that checkboxes and radiobuttons can be drawn in user-defined sizes? Yes. > Resizing these types of input fields on Windows isn't possible. And we have a bug on that.

(In reply to comment #44) > > you mean that checkboxes and radiobuttons can be drawn in user-defined sizes? > > Yes. Ok, so I'll update the reftest accordingly. Thanks for pointing this out. > > Resizing these types of input fields on Windows isn't possible. > > And we have a bug on that. I believe you mean bug 446511. Marking as verified again and sorry for the confusion.

The reftest uses "width: auto" now, as said by Boris. I've tested it on OS X and Windows. It works fine on both platforms. Should it really not be used for other Platforms as Mac?

Comment on attachment 355318 [details] [diff] [review] Reftest v2 Looks good.

Removed useless style tag in reference file. Please wait with check-in until I've checked if the test will pass on Linux too.

Comment on attachment 355465 [details] [diff] [review] Reftest v2.1 >+++ b/layout/reftests/forms/checkbox-radio-stretched.html >@@ -0,0 +1,26 @@ >+input { >+ width: 300px; >+} What's this rule for? Both inputs have class="stretched", so the width will always be overwritten to auto. <input type="checkbox/radio" style="width: auto"> would be shorter and achieve the same. Still, thanks for taking the time to write a test!

(In reply to comment #49) > What's this rule for? Both inputs have class="stretched", so the width will > always be overwritten to auto. > <input type="checkbox/radio" style="width: auto"> would be shorter and achieve > the same. We could leave that out. That's true. But it makes the bug less visible. With the additional rule checkboxes and radiobuttons where stretched to this given width. So I think we should stay with that.

(In reply to comment #50) > With > the additional rule checkboxes and radiobuttons where stretched to this given > width. Are you sure? I don't see how that could be possible.

Markus, you are right. That's really a left-over from the older reftest where I have miss-understood the issue. It can really be removed. New ref test attached. I've also setup an Ubuntu machine to test it on Linux. Everything works fine for Mac/Win/Lin. Boris, can I bother you again? Just to make sure everything is fine.

pushed the reftest: http://hg.mozilla.org/mozilla-central/rev/1283369519f3

on 1.9.1, too: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/e62b5e93fbbf