DESCRIPTION: select renders with different font-family than other html form controls. STEPS TO REPRODUCE: View mozilla bug query page http://bugzilla.mozilla.org/query.cgi. Compare the font rendered for select elements to input[type="text"] ACTUAL RESULTS: Select element is rendered in a sans-serif font while the input[type="text"] are rendered in a monospace font. EXPECTED RESULTS: All form elements should render with at least the same font-family, font-size. I reported a similar Bug (#42825). Over the past few builds make this html form bug very obvious. DOES NOT WORK CORRECTLY ON: Win98 build 2000092205 (9-21-00 nightly build)

All pages now layout incorrectly because the form controls are too large. This can be corrected with removing line of code. Here is the diff: Index: nsCSSStyleRule.cpp =================================================================== RCS file: /cvsroot/mozilla/layout/html/style/src/nsCSSStyleRule.cpp,v retrieving revision 3.123 diff -u -r3.123 nsCSSStyleRule.cpp --- nsCSSStyleRule.cpp 2000/09/21 10:21:23 3.123 +++ nsCSSStyleRule.cpp 2000/09/22 18:48:58 @@ -1681,7 +1681,6 @@ case eSystemAttr_Font_Button: case eSystemAttr_Font_List: font->mFont.name.AssignWithConversion("sans-serif"); - font->mFont.size = defaultFont.size; break; } #endif

Marking nsbeta3+, since this is a serious recent regression.

Yea, I noticed that the fonts were way too big on selects too. I see that you are now going back to using the DeviceContext-specified size for the font, and that makes sense, but I'm worried that the patch only affects Win. What about Mac and Linux? Do we want the Select font to be the default-font size on one platform and not another? Pierre, do we really want to apply the default font size to the system fonts for lists and buttons at all?

I removed the font size setting in Linux and it didn't seem to make a diference.

rod, Can you also please make sure that the font-family matches the other form controls. I think all other form controls render with monospace exept for the <select> (sans-serif). I think the select should match the html form elements (use monospace).

As it turns out Pierre was doing the correct thing. You want to use the correct variable width font size but it has to be adjusted by 2 points which is what Nav 4.x does. So instead of removing the line we just need to supbtract 3 points (in twips) from the font size: Index: nsCSSStyleRule.cpp =================================================================== RCS file: /cvsroot/mozilla/layout/html/style/src/nsCSSStyleRule.cpp,v retrieving revision 3.123 diff -u -r3.123 nsCSSStyleRule.cpp --- nsCSSStyleRule.cpp 2000/09/21 10:21:23 3.123 +++ nsCSSStyleRule.cpp 2000/09/22 22:05:23 @@ -1681,7 +1681,7 @@ case eSystemAttr_Font_Button: case eSystemAttr_Font_List: font->mFont.name.AssignWithConversion("sans-serif"); - font->mFont.size = defaultFont.size; + font->mFont.size = defaultFont.size - NSIntPointsToTwips(2); break; } #endif This is a big win, because of Pierre's checkin the form controls now obey the Pref setting (in NavQuirks mode) and this fix makes that final adjustment (a minor oversight by Pierre) so that they have the correct size.

Oops, in the comment before this I meant to always mean 2 points not 3

fixed

What if defaultFont.size is less than 2 points? Then you get a negative size. Can that crash anything elsewhere?

This should also be fixed in the branch. Has it been, or do we need to appeal to PDT?

It looks like the sizing problem has been fixed in build win98 2000092420. However, the select control still renders in a sans-serif font while all other html form controls render with a monospace font. I want to suggest that if you change in the html.css, the font-family for select to "field" instead of "list" or map the keyword "list" to match whatever font-family "field" is matched to, you then will get a consistant font-family rendering along all form controls.

No, look at Nav 4.x, it renders sans-serif for selects and monospace for the other controls. And No this didn't make it into the branch and it needs to.

Rod, I have to ask so I will not waste your/my time on this issue anymore... Do you think it is fine that all form elements are rendered in a monospace font except the select element (ignoring NAV4 since there are so many things NAV4 rendered wrong that are corrected in mozilla)?

PDT marking nsbeta3-, unclear how bad it is now, nominate for rtm if appropriate.

I need this bug for plus for nsbeta3 or RTM. The fix is a one liner for Windows platform only with zero risk: nsCSSStyleRule.cpp line 1678 Old line: font->mFont.size = defaultFont.size; New line: font->mFont.size = PR_MAX(defaultFont.size - NSIntPointsToTwips(2), 0); This fix was checked into the trunk and just missed the branch (by an hour or two). I have spent 3 years trying to get form controls to size correctly and without this fix in the branch EVERY single page with comboboxes or button will not layout correctly because the form control will size to font point sizes too big. I'll add the diff once I have a good branch tree.

PDT: I recommend we get this in for RTM. Very low risk, high value. Fixes layout of a bunch of pages.

PDT: I also recommend that we accept this either for nsbeta3 or, failing that, for rtm. Reasons: (1) HTML forms are ubiquitous on the web, particularly at consumer e-commerce and service sites that are frequented by the intermediate users who are the target of this first product release, so this is critically important backward compatibility. The problem is expected to affect many pages on the web if not fixed. (2) E-commerce sites are quite sensitive about the branding, UE, and look and feel of their pages, so we will likely negatively affect many key partners if we don't fix this. (3) The fix has already been checked in on the trunk and we can use that to make sure it's not causing problems. (4) Rod really had this done in time and checked in on the trunk and just missed the branch by a few hours; let's use good judgment and Do The Right Thing rather than mindlessly enforcing an arbitrary cutoff point by hours without assessing the cost-benefit of accepting the patch.

PDT, if you view the Bugzilla query page with a recent branch build, you will quickly see why it is so important to get this fix into the branch. The fix has been cooking on the trunk for a few days, and the trunk builds are better because of this. Please allow this simple and safe fix to be applied to the branch.

Here is the official diff: Index: nsCSSStyleRule.cpp =================================================================== RCS file: /cvsroot/mozilla/layout/html/style/src/nsCSSStyleRule.cpp,v retrieving revision 3.123 diff -u -r3.123 nsCSSStyleRule.cpp --- nsCSSStyleRule.cpp 2000/09/21 10:21:23 3.123 +++ nsCSSStyleRule.cpp 2000/09/26 20:09:08 @@ -1681,7 +1681,7 @@ case eSystemAttr_Font_Button: case eSystemAttr_Font_List: font->mFont.name.AssignWithConversion("sans-serif"); - font->mFont.size = defaultFont.size; + font->mFont.size = PR_MAX(defaultFont.size - NSIntPointsToTwips (2), 0); break; } #endif

rtm++

Updating QA contact.

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

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

Marking nsbeta3- since we are past nsbeta3.

WOW, This amazes me that this did not get fixed. :-( Now every form will be broken that use select and button elements. Everyone.

Yea James,I don't think Rod tried hard enough to get this one approved (presumably because he is a slacker) <g>

Nope, but I'm guessing Rod wanted this one as bad as anyone else. Can't wait to see what my boss thinks when she sees that these elements are twice as big as the other form controls. Then I have to explain it's not my fault... as always. I think the only thing worse that could happen for me in mozilla is to take out :hover. GD, did that too in the past week! The only :hover that works is in some form elements (selects, buttons, radios, and checkboxes). Thanks Rod, must be your fault again, buddy. :-)

fixed

fixed

Works for Me Platform: PC OS: Windows 98 Mozilla Version: 2000100508 Marking as Verified