522050 - select menus appear huge on bugzilla

Status: VERIFIED FIXED

(Core :: Layout: Form Controls, defect)

Description

[Robert Sayre]

about twice as big as they should be

Comment 1

[Robert Sayre]

Attachment: screenshot

Comment 3

[Boris Zbarsky [:bzbarsky]]

This is a regression from bug 515777.  forms.css used to be preprocessed before that change (see the PREPROCESS_FORMS_CSS stuff in layout/style/Makefile.in), so the "%if OSARCH==OS2" code block at the bottom used to get stripped out.  Now it's left in, which triggers two CSS errors and incorrect styling on select, optgroup, and reset/submit/button inputs.

It also looks like we're still preprocessing the forms.css and sticking it in dist/ but just not using that version...

Comment 4

[(dormant account)]

Attachment: preprocess forms.css

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 406035 [details] [diff] [review]
preprocess forms.css

r=bzbarsky if that stray printf goes away

Comment 6

[(dormant account)]

Attachment: patch for checkin

carrying over r+

Comment 7

[Robert Sayre]

testing should have caught this immediately. what are we missing? Every form element with dimensions taken in mochitest?

Comment 8

[Boris Zbarsky [:bzbarsky]]

We could have a mochitest that checks the computed font-family, assuming that's a system font for <select>...  maybe.  Is the above the case?

Comment 9

[Robert Sayre]

(In reply to comment #8)
> We could have a mochitest that checks the computed font-family, assuming that's
> a system font for <select>...  maybe.  Is the above the case?

Couldn't we test the dimensions of the elements? this regression seemed to break the layout of lots of sites. shouldn't we catch it?

Comment 10

[Boris Zbarsky [:bzbarsky]]

> Couldn't we test the dimensions of the elements?

Those would depend on the exact OS settings for system fonts...

And yes, if you happen to set slightly bigger OS default fonts some sites break.  Lots of sucky sites out there.  But in that situation, breaking them is the expected behavior for us.

Comment 11

[Robert Sayre]

(In reply to comment #10)
> > Couldn't we test the dimensions of the elements?
> 
> Those would depend on the exact OS settings for system fonts...
> 

Seems to me that those should be the exact same for our test systems, and easily skipped a if a preliminary check finds that the system default is not what we expect.

Comment 12

[Boris Zbarsky [:bzbarsky]]

They'd differ by OS and OS version on our test systems.  If we want to write a test that has all that information baked into it _and_ make sure it doesn't run on developers' machines, I guess we could check sizes.  But the approach from comment 8 would work better, I think, at least in catching the wrong font-family.

Comment 13

[Robert Sayre]

(In reply to comment #12)
> They'd differ by OS and OS version on our test systems.  

We could still write, say, an OS 10.5 Mac test and catch this for many years.

> If we want to write a
> test that has all that information baked into it _and_ make sure it doesn't run
> on developers' machines, I guess we could check sizes.  But the approach from
> comment 8 would work better, I think, at least in catching the wrong
> font-family.

I'll defer to you on catching this particular bug. However, it seems to me that changing form control sizes by 2x will break page layouts, and we should catch have tests that detect changes in form control sizes.

Comment 14

[Robert Sayre]

> (In reply to comment #12)
> we should catch have tests that detect changes in form control sizes.

Sorry, I mean that our tests should catch unintended changes in form control size.

Comment 15

[mdew]

can we check this patch in?

Comment 16

[Boris Zbarsky [:bzbarsky]]

Pushed http://hg.mozilla.org/mozilla-central/rev/7a11da3c6a06

Comment 17

[Boris Zbarsky [:bzbarsky]]

> I mean that our tests should catch unintended changes in form control size.

I do think this would be nice to do if we can do it sanely.  David, thoughts?

Comment 18

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Verified fixed with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.3a1pre) Gecko/20091015 Minefield/3.7a1pre ID:20091015030945

Comment 19

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

(In reply to comment #17)
> > I mean that our tests should catch unintended changes in form control size.
> 
> I do think this would be nice to do if we can do it sanely.  David, thoughts?

Not particularly.  Perhaps the "right" way is to have a native-code program that looks up the system sizes and generates an appropriate mochitest, but that's a good bit of work.

Testing font sizes seems like a reasonable idea, though.