285367 - <select> with size="" renders as a listbox

Status: NEW

(Core :: Layout: Form Controls, defect, P3)

Description

[Wevah (Nate Weaver)]

<select size=""> is rendered as a text field instead of a popup menu, though
clicking on it still pops up the proper menu.

This can be fixed by adding |select[size=""]| as a selector to the rulesets for
|select[size="0"], select[size="1"]| in forms.css.

Comment 1

[Wevah (Nate Weaver)]

Attachment: Testcase.

Comment 2

[Mike Pinkerton (not reading bugmail)]

is this a regression?

Comment 3

[Wevah (Nate Weaver)]

I'm actually not sure. The |size| attribute is supposed to be non-empty and an
integer if used, and so it's not very common to see this (I've only run across
it on one site that I can remember).

Comment 4

[Samuel Sidler (old account; do not CC)]

Is this fixable by 1.0? Or rather, should this be fixed by then?

(Or even 0.9?...)

Comment 5

[Wevah (Nate Weaver)]

Yes. Want a patch?

;)

Comment 6

[Wevah (Nate Weaver)]

Attachment: Patch for forms.css.

Patch for the fix I mentioned in comment 0. This also changes the non-Cocoa
widget declaration, but I think that is the right thing to do (and shouldn't
break anything anyway).

Comment 7

[Wevah (Nate Weaver)]

I just checked Firefox; it doesn't suffer from this bug, so if the first part of
the patch (the non-Cocoa widget bit) should be taken out, it can be.

Comment 8

[Wevah (Nate Weaver)]

Comment on attachment 178865 [details] [diff] [review]
Patch for forms.css.

Checked again; leaving it in doesn't break anything. Asking for review (or just
super, considering the tiny size of the patch).

Comment 9

[Wevah (Nate Weaver)]

Upon further inspection, it looks like firefox (and un-themed Gecko?) totally
disregards the -moz-appearance for selects, rendering them as popups if the
resulting size is 1, and as listboxes otherwise.

The patch still fixes selects with size="" for Camino, though it's probably not
optimal. :/

Comment 10

[Wevah (Nate Weaver)]

This might be able to be fixed through forms.css using :not(), actually...

Comment 11

[Wevah (Nate Weaver)]

(In reply to comment #10)
> This might be able to be fixed through forms.css using :not(), actually...

Eh, should probably be fixed in code anyway. <select>s with size="" render at a
funny height.

Comment 12

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Can this be rolled in to the tweaking in bug 298111?

Comment 13

[Wevah (Nate Weaver)]

(In reply to comment #12)
> Can this be rolled in to the tweaking in bug 298111?

I think FF deals with this in code; I don't know if all cases can be overcome in
CSS alone.

(I should probably give up this bug, too.)

Comment 14

[Simon Fraser [no longer active]]

ping!

Comment 15

[Simon Fraser [no longer active]]

Attachment: Testcase with select with bogus size.

Comment 16

[Simon Fraser [no longer active]]

The problem here seems to be that we when using nsNativeTheme, we choose which
appearance to draw the widget in based on the "mAppearance" member of the
nsStyleDisplay struct, which comes out of CSS. But when using gecko widgets,
there is logic in nsCSSFrameConstructor that figures out how to draw the select
based on the "size" attribute.

So either we replicate the logic in
nsCSSFrameConstructor::ConstructSelectFrame() via CSS, or this needs a more
thorough fix in layout.

Comment 17

[Boris Zbarsky [:bzbarsky]]

The problem is that in CSS (at least the part we implement) you can't select on
"attribute value is a number".  Which is what I believe we'd need here.

Comment 18

[Samuel Sidler (old account; do not CC)]

(In reply to comment #17)
> The problem is that in CSS (at least the part we implement) you can't select on
> "attribute value is a number".  Which is what I believe we'd need here.

So this isn't fixable for us without changing code, correct? Is there a core bug corresponding to fixing this or should this be moved to widgets:mac and fixed on the trunk?

(i.e. is there anything we can do to fix this by 1.0 on the Gecko 1.8 branch?)

Comment 19

[Boris Zbarsky [:bzbarsky]]

> So this isn't fixable for us without changing code, correct?

I suspect that is correct.

Comment 20

[Samuel Sidler (old account; do not CC)]

(In reply to comment #19)
> > So this isn't fixable for us without changing code, correct?
> 
> I suspect that is correct.
> 

Because this will require a code change which I doubt we'll get on the 1.8 branch (possibly on 1.8.1 though), I'm retargeting this bug for 1.1.

Comment 21

[Wevah (Nate Weaver)]

This shouldn't be assigned to me anymore; it's a bigger change than I thought at first.

Comment 22

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Comment on attachment 178865 [details] [diff] [review]
Patch for forms.css.

obsoleting this patch since it needs a code fix, too

Comment 23

[Stuart Morgan]

Moving to Cocoa Widgets, since this happens in post-CSS-removal Camino

Comment 24

[Josh Aas]

I can reproduce with current Linux trunk, marking as a cross-platform layout bug.

Comment 25

[Josh Aas]

I don't think they are rendering as text fields though - they are rendering as funny-looking listboxes with only one option, no scrollbar. That is still incorrect, they should be the same as size="1" selects.

Comment 26

[Boris Zbarsky [:bzbarsky]]

Note that theming impls that care could fix this by making ThemeSupportsWidget return false in cases like this...  It'd look a little weird, but not as weird as it does now.

If someone wants to fix it, the options as I see them are to either have another CSS selector for this or to move the appearance rule into C++ wholesale (e.g. in nsHTMLStylesheet).

David, do you hve a preference between those two or other ideas?

Comment 27

[Josh Aas]

If I put "-moz-appearance: none;" on all the controls this bug does go away. However, a different problem seems to still exist. Why are the selects with bogus size values taller than the select with 'size="1"'?

Comment 28

[Josh Aas]

Attachment: testcase with -moz-appearance: none;

Comment 29

[Boris Zbarsky [:bzbarsky]]

Because the forms.css rules give listboxes 1px of top and bottom padding (which you've complained about in another bug) and then remove that padding for comboboxes.  So if the node doesn't match the combobox rule, the padding will still be there.

Given that, sounds like a special selector is what we might want here...