992447 - 'overflow-clip-box:content-box' on a combobox clips the drop-down button

Status: RESOLVED FIXED

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

Description

[Mats Palmgren (inactive)]

STR
0. set the pref layout.css.overflow-clip-box.enabled to true
1. load the URL

ACTUAL RESULTS
The drop-down button is clipped.

EXPECTED RESULTS
The drop-down button should not be affected by 'overflow-clip-box'
in any way.  For the area that displays the value it's less clear
what the desired results should be -- currently it's not affected
at all (behaves as 'overflow-clip-box:content-box' by default)
but maybe we should support setting 'overflow-clip-box:padding-box'
if the author so desires (only for the padding around the value
though, i.e. not overflowing into the drop-down button)

Anyway, let's scope this bug to be about the clipping of the
drop-down button only.

Comment 1

[Mats Palmgren (inactive)]

The reason 'overflow-clip-box' is applied is that <select> has
'overflow:-moz-hidden-unscrollable' in the UA sheet:
http://hg.mozilla.org/mozilla-central/annotate/5fa70bd90a8b/layout/style/forms.css#l209
(added in bug 300030 (reflow-refactor))

So it would be easy for authors to workaround this bug by setting 'overflow:visible'.
That might cause problems in some edge cases though, like if you specify a 'width' that
doesn't fit the horizontal padding / button.

Comment 2

[Alex Henrie]

When this bug is fixed, we may be able to remove the if statement added to layout/generic/TextOverflow.cpp by https://hg.mozilla.org/mozilla-central/rev/b5c7349ece19

Comment 3

[Mats Palmgren (inactive)]

Attachment: wallpaper+reftest

I think we should just wallpaper this for now.  It's probably a lot of work
to implement 'overflow-clip-box' correctly for <select> and it seems not
very useful for this element.

https://tbpl.mozilla.org/?tree=Try&rev=ce513056b62f
https://tbpl.mozilla.org/?tree=Try&rev=407719b9b8ed

Comment 4

[Mats Palmgren (inactive)]

I wonder if the right fix is to have a zero-sized padding area for the
nsComboboxControlFrame itself, and have the padding applied to its
anonymous inner frame instead, like so:

########################################
#ppppppppppppppppppppppbbbbbbbbbbbbbbbb#
#ppppppppppppppppppppppbbbbbbbbbbbbbbbb#
#pppp            ppppppbbbbbbbbbbbbbbbb#
#pppp            ppppppbbbbbbbbbbbbbbbb#
#pppp            ppppppbbbbbbbbbbbbbbbb#
#pppp            ppppppbbbbbbbbbbbbbbbb#
#ppppppppppppppppppppppbbbbbbbbbbbbbbbb#
#ppppppppppppppppppppppbbbbbbbbbbbbbbbb#
########################################

Where p=padding on the inner anon block, b=button, #=outer border,
and the content area for outermost frame is everything inside #
(i.e. zero padding).  That should fix 'overflow-clip-box' properly
if we put 'overflow-clip-box:inherit' (in the UA sheet) on the inner
frame and 'overflow-clip-box:content-box' on the outer, which would
give you the blank area above as the clip area by default.  If the
author specifies 'padding-box', the inner content can overflow into
the 'p' area but not beyond it.

(This design should also work for <select> lists where 'b' is
instead the scrollbar.)

Alex, what do you think?

Comment 5

[Mats Palmgren (inactive)]

(In reply to Alex Henrie from comment #2)
> When this bug is fixed, we may be able to remove the if statement added to
> layout/generic/TextOverflow.cpp by
> https://hg.mozilla.org/mozilla-central/rev/b5c7349ece19

Maybe.  We have to watch out for edge cases though, for example when the 'width'
is too small to fit the horizontal padding / button.

data:text/html,<select size=1 style="padding:10px;width:40px;border:solid;outline:3px dotted blue;overflow:visible"><option>xxxxxxxxxx

Perhaps we remove the button/scrollbar for those edge cases anyway?

Comment 6

[Alex Henrie]

(In reply to Mats Palmgren (:mats) from comment #4)
> Alex, what do you think?

That's exactly what I was trying to describe in https://bugzilla.mozilla.org/show_bug.cgi?id=990655#c13 - I think this is the only real solution.

Comment 7

[Mats Palmgren (inactive)]

OK, good.  We should probably make the change in two steps though; 1, implement the
layout of the children as described; 2, (possibly) make nsComboboxControlFrame
not inherit nsBlockFrame (nsContainerFrame perhaps).
I haven't really looked in to 2 though, so I don't know how feasible it is.

Comment 8

[Mats Palmgren (inactive)]

Comment on attachment 8402147 [details] [diff] [review]
wallpaper+reftest

Green on Try.

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5cea3e42cb44

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/5cea3e42cb44

Comment 11

[Mats Palmgren (inactive)]

(filed bug 994839 to follow-up on comment 4)