Closed Bug 232540 Opened 21 years ago Closed 20 years ago

select-box with scroller set to a css-width smaller than the content of one option jump on mouseover

Categories

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

Product:
Core
Component:
Layout: Form Controls
Type:
defect

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: joachim.wendenburg, Assigned: roc)

References

(
URL
)

Details

(Keywords: fixed1.7, regression, testcase)

Attachments

(4 files)

The reporter's code, as an attachment
778 bytes, text/html
Details
modified testcase
857 bytes, text/html
Details
fix
1.32 KB, patch
dbaron
: review+
dbaron
: superreview+
asa
: approval1.7+
Details | Diff | Splinter Review
fix for the reflow problem
1.86 KB, patch
dbaron
: review+
dbaron
: superreview+
Details | Diff | Splinter Review 
User-Agent:       
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.6) Gecko/20040113

hi,

the select-box gets a css-width via a class. There are so many otions, that a
scrollbar appears.
At least one option content is more wide then the width defined in the css.

Moving the mouse over the options they will "jump" some steps as soon as the
mouse passes a option more wide then the css-width. This behavier is quite
confusing. See the testscript also posted here.

regards  Joachim

<html>
<head>
<style type="text/css">
 .sel {
  width: 60px; }
</style>
</head>
<body marginheight="0" marginwidth="0">
<form>
<select class="sel">
 <option>...</option>
 <option>test</option>
 <option>test</option>
 <option>test test test test test test test test test test</option>
 <option>test</option>
 <option>test</option>
 <option>test</option>
 <option>test</option>
 <option>test</option>
 <option>test</option>
 <option>test</option>
 <option>test</option>
 <option>test</option>
 <option>test</option>
 <option>test</option>
 <option>test</option>
 <option>test</option>
 <option>test</option>
 <option>test</option>
 <option>test</option>
 <option>test</option>
 <option>test</option>
 <option>test</option>
 <option>test</option>
</select>
</form>
</body>
</html>

Reproducible: Always
Steps to Reproduce:
Just check the Testscript im Mozilla 1.6 an move your mouse over the options
view url for a testcase
URL: <html><head><style type="text/css"> ....
Keywords: testcase
Assignee: dveditz+bmo → nobody
Component: Form Manager → Layout: Form Controls
QA Contact: core.layout.form-controls
I see this on LInux 2004021708
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows 2000 → All
I believe the missing missing width is one scrollbar width wide
*** Bug 235162 has been marked as a duplicate of this bug. ***
Thought I'd add the reporter's original test code as an attachment, to save
time. Especially since the URL field looks garbled to me.
Hardware: PC → All
Summary: select-box with scroller set to a css-width smaller then the content of one option jump on mouseover → select-box with scroller set to a css-width smaller than the content of one option jump on mouseover
Additional info: this bug was introduced between the 20031111 and 20031112
builds (at least in Firebird).
*** Bug 236232 has been marked as a duplicate of this bug. ***
(In reply to comment #5)
> Created an attachment (id=142737)
> The reporter's code, as an attachment
> 
> Thought I'd add the reporter's original test code as an attachment, to save
> time. Especially since the URL field looks garbled to me.

No actually this is a data: URLs, very handy (and it works).

Bonsai URL for checkins:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=11%2F11%2F2003&maxdate=11%2F12%2F2003&cvsroot=%2Fcvsroot

Wayne: Do you also have the hour of the builds (normally in titlebar there
stands something like 2004030205 where 05 is the hour) 20031111 and 20031112? 
Sorry, I didn't know about the data: thing, it just looked garbled to me :)

Firefox/Firebird don't show the build on the title bar. And the about info only
lists the day, not the hour. The builds were both stored in folders ending in
'02', but that doesn't seem to correlate with the hour in Seamonkey builds, so I
assume it's not the hour for Firebird builds either.

I tested it out on Seamonkey builds of the same time. The last non-buggy build
was 2003111109, the first buggy build was 2003111210.
*** Bug 241278 has been marked as a duplicate of this bug. ***
You can add OPTION{overflow:hidden;} as a workaround
If there is no time for real fix (for 1.7), should we use that workaround
in forms.css?
Flags: blocking1.7?
adding 'overflow:hidden' to all OPTIONs would create significant slowdown on
some pages, I think.
Attached file modified testcase — 
This is a bit modified testcase. It is trying to show where the actual problem
is.
I'll look at doing a real fix for this.
Assignee: nobody → roc
Keywords: regression
The fix for bug 191699 is the most likely culprit.
Attached patch fix — — Splinter Review 
Okay. This turned out to be a fairly straighforward problem. When we check to
see whether we need to scroll the selected option into view, we shouldn't just
call rect.Contains(fRect); that might be false just because fRect overflows
horizontally. Instead we should manually check for intersection only in the
vertical dimension.
Comment on attachment 147519 [details] [diff] [review]
fix

OK, this one's easy and basically solves the bug.

But it is actually only part of the problem. The underlying change that broke
this is that the drop-down listbox is not making itself quite wide enough to
fit the long option. I should track that down and fix it too.
Attachment #147519 - Flags: superreview?(dbaron)
Attachment #147519 - Flags: review?(dbaron)
Here's what's happening with the dropdown width: we compute the dropdown width
with unconstrained width and height to get the maximum width of any option. And
that's the width we will always use for the dropdown UNLESS the computed width
of the selection box itself is wider than the dropdown, in which case we make
the dropdown as wide as the selection box.

Now, for an intrinsically sized SELECT, the selection box is sized to be the
width of the widest option plus a dropdown arrow which is the same width as a
vertical scrollbar plus some slop. Then the dropdown is smaller than the select
box and grows to be as wide as the select box, so is wide enough for its widest
option plus the vertical scrollbar.

When the select box width is shrunk by CSS styling, that second case does not
kick in. We stick with the original computed width which had no vertical or
horizontal constraint, and thus no vertical scrollbar. But of course the
dropdown DOES have vertical overflow, thus a vertical scroll, which thus
overlaps the widest option.
This patch fixes the width problem. Listboxes reflow once unconstrained to
measure the options and then again to actually size the listbox --- once the
vertical height is known, which depends on the option heights. Previously the
second reflow used the listbox's computed width if that was constrained,
otherwise we imposed the desired width from the first, unconstrained reflow.
This patch changes the latter behaviour so that if we're reflowing with
unconstrained width then we let the second reflow use an unconstrained width
too. This allows the listbox to get wider if a vertical scrollbar is now
needed. It fixes this bug and doesn't introduce any obvious regressions.
Comment on attachment 147597 [details] [diff] [review]
fix for the reflow problem

Sorry David, but I think you and me are the only ones who nominally understand
this blighted code :-)
Attachment #147597 - Flags: superreview?(dbaron)
Attachment #147597 - Flags: review?(dbaron)
Both of these patches are improvements and should be checked in (IMHO). The
first patch is much less risky (and thus suitable for 1.7) than the second patch.
Comment on attachment 147519 [details] [diff] [review]
fix

This seems simple and safe.
Attachment #147519 - Flags: superreview?(dbaron)
Attachment #147519 - Flags: superreview+
Attachment #147519 - Flags: review?(dbaron)
Attachment #147519 - Flags: review+
Comment on attachment 147597 [details] [diff] [review]
fix for the reflow problem

I want to look at this patch more later -- but one obvious thing is that you
could move the constructor of |secondPassState| as well.  Also, you don't need
to set mComputedWidth if you want to make this change, since secondPassState is
just copy-constructed from aReflowState.

Also, is the |visibleWidth != scrolledAreaWidth| test the right test now?
OK, the first patch is checked in.
Comment on attachment 147519 [details] [diff] [review]
fix

This patch is branch material. A very simple, safe fix to an annoying UI
glitch, which also appears to be a regression.
Attachment #147519 - Flags: approval1.7?
Comment on attachment 147519 [details] [diff] [review]
fix

a=asa (on behalf of drivers) for checkin to 1.7
Attachment #147519 - Flags: approval1.7? → approval1.7+
OK, I just checked the first patch into the branch.
*** Bug 239029 has been marked as a duplicate of this bug. ***
Works great in 20040508 Firefox/0.8.0+, Mac OS X. Thanks Robert :)
Flags: blocking1.7? → blocking1.7-
Keywords: fixed1.7
*** Bug 245205 has been marked as a duplicate of this bug. ***
*** Bug 243834 has been marked as a duplicate of this bug. ***
Comment on attachment 147597 [details] [diff] [review]
fix for the reflow problem

r+sr=dbaron, but move the constructor of secondPassState inside the if as well.
Attachment #147597 - Flags: superreview?(dbaron)
Attachment #147597 - Flags: superreview+
Attachment #147597 - Flags: review?(dbaron)
Attachment #147597 - Flags: review+
checked in.
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: