Last Comment Bug 232540 - select-box with scroller set to a css-width smaller than the content of one option jump on mouseover
: select-box with scroller set to a css-width smaller than the content of one o...
Status: RESOLVED FIXED
: fixed1.7, regression, testcase
Product: Core
Classification: Components
Component: Layout: Form Controls (show other bugs)
: Trunk
: All All
: P2 normal with 1 vote (vote)
: ---
Assigned To: Robert O'Callahan (:roc) (Exited; email my personal email if necessary)
:
Mentors:
data:text/html,<html><head><style typ...
: 235162 236232 239029 241278 243834 245205 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2004-01-29 06:17 PST by joachim.wendenburg
Modified: 2004-11-07 05:01 PST (History)
9 users (show)
asa: blocking1.7-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
The reporter's code, as an attachment (778 bytes, text/html)
2004-03-02 03:11 PST, Wayne Woods
no flags Details
modified testcase (857 bytes, text/html)
2004-04-28 08:29 PDT, Olli Pettay [:smaug]
no flags Details
fix (1.32 KB, patch)
2004-05-02 20:15 PDT, Robert O'Callahan (:roc) (Exited; email my personal email if necessary)
dbaron: review+
dbaron: superreview+
asa: approval1.7+
Details | Diff | Review
fix for the reflow problem (1.86 KB, patch)
2004-05-03 22:22 PDT, Robert O'Callahan (:roc) (Exited; email my personal email if necessary)
dbaron: review+
dbaron: superreview+
Details | Diff | Review

Description joachim.wendenburg 2004-01-29 06:17:45 PST
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
Comment 1 Frank Wein [:mcsmurf] 2004-01-29 07:55:03 PST
view url for a testcase
Comment 2 Jesiah S 2004-02-18 13:29:58 PST
I see this on LInux 2004021708
Comment 3 David Anderson 2004-02-20 13:57:39 PST
I believe the missing missing width is one scrollbar width wide
Comment 4 Wayne Woods 2004-03-02 02:40:37 PST
*** Bug 235162 has been marked as a duplicate of this bug. ***
Comment 5 Wayne Woods 2004-03-02 03:11:23 PST
Created attachment 142737 [details]
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.
Comment 6 Wayne Woods 2004-03-02 17:44:56 PST
Additional info: this bug was introduced between the 20031111 and 20031112
builds (at least in Firebird).
Comment 7 Olli Pettay [:smaug] 2004-03-03 02:23:33 PST
*** Bug 236232 has been marked as a duplicate of this bug. ***
Comment 8 Frank Wein [:mcsmurf] 2004-03-03 05:08:50 PST
(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? 
Comment 9 Wayne Woods 2004-03-03 15:51:26 PST
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.
Comment 10 Ali Ebrahim 2004-04-21 20:09:21 PDT
*** Bug 241278 has been marked as a duplicate of this bug. ***
Comment 11 Jesiah S 2004-04-27 11:56:34 PDT
You can add OPTION{overflow:hidden;} as a workaround
Comment 12 Olli Pettay [:smaug] 2004-04-28 06:52:34 PDT
If there is no time for real fix (for 1.7), should we use that workaround
in forms.css?
Comment 13 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2004-04-28 08:28:08 PDT
adding 'overflow:hidden' to all OPTIONs would create significant slowdown on
some pages, I think.
Comment 14 Olli Pettay [:smaug] 2004-04-28 08:29:09 PDT
Created attachment 147235 [details]
modified testcase

This is a bit modified testcase. It is trying to show where the actual problem
is.
Comment 15 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2004-04-28 08:29:45 PDT
I'll look at doing a real fix for this.
Comment 16 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2004-04-29 18:03:22 PDT
The fix for bug 191699 is the most likely culprit.
Comment 17 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2004-05-02 20:15:55 PDT
Created attachment 147519 [details] [diff] [review]
fix

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 18 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2004-05-02 20:19:15 PDT
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.
Comment 19 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2004-05-03 21:53:33 PDT
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.
Comment 20 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2004-05-03 22:22:51 PDT
Created attachment 147597 [details] [diff] [review]
fix for the reflow problem

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 21 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2004-05-03 22:25:18 PDT
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 :-)
Comment 22 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2004-05-03 22:26:15 PDT
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 23 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2004-05-04 17:24:26 PDT
Comment on attachment 147519 [details] [diff] [review]
fix

This seems simple and safe.
Comment 24 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2004-05-04 17:37:11 PDT
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?
Comment 25 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2004-05-04 19:50:12 PDT
OK, the first patch is checked in.
Comment 26 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2004-05-04 19:51:27 PDT
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.
Comment 27 Asa Dotzler [:asa] 2004-05-05 04:09:04 PDT
Comment on attachment 147519 [details] [diff] [review]
fix

a=asa (on behalf of drivers) for checkin to 1.7
Comment 28 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2004-05-06 19:16:06 PDT
OK, I just checked the first patch into the branch.
Comment 29 Jesiah S 2004-05-07 09:41:02 PDT
*** Bug 239029 has been marked as a duplicate of this bug. ***
Comment 30 Wayne Woods 2004-05-08 07:51:10 PDT
Works great in 20040508 Firefox/0.8.0+, Mac OS X. Thanks Robert :)
Comment 31 Tuukka Tolvanen (sp3000) 2004-06-16 08:39:25 PDT
*** Bug 245205 has been marked as a duplicate of this bug. ***
Comment 32 Jesiah S 2004-06-23 12:27:29 PDT
*** Bug 243834 has been marked as a duplicate of this bug. ***
Comment 33 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2004-07-11 18:20:46 PDT
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.
Comment 34 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2004-07-23 16:46:24 PDT
checked in.

Note You need to log in before you can comment on or make changes to this bug.