335453 - can't get to form selection from dropdown list

Status: RESOLVED DUPLICATE of bug 314939

(Minimo Graveyard :: General, defect)

Description

[chris hofmann]

I tried going though the check out on amazon using latest minimo build.  everything went ok until I hit a form with a list of years for the expiration date on my ccard.  my year of expiration was off the end of the list of the years that could be displayed (.

I imagine it would be the same for any long dropdown list on any page.  I'll try to drum up a test case..

Comment 1

[chris hofmann]

looks like firefox has a vertical scroll bar on the list box for this situation

Comment 2

[Doug Turner (:dougt)]

The default number of rows visable in a drop down is 20.  Any more rows, then the select will have a scrollbar.  

in mozilla/layout/forms/nsListControlFrame.cpp, line 99:

const nscoord kMaxDropDownRows          = 20; // This matches the setting for 4.x browsers

This is the interesting starting point.  If i set this to 8, you get what you want -- a select that doesn't have options floating off the screen.  If I were to guess, the "adaptive algorithm" on line 989 is suspect.

Comment 3

[Boris Zbarsky [:bzbarsky]]

We never even get to the adaptive algorithm, do we?  We only hit that code if

  886     if (visibleHeight > (mNumDisplayRows * heightOfARow)) {

(as in, if our height is bigger than kMaxDropDownRows * heightOfARow, considering the previous line is

  885     mNumDisplayRows = kMaxDropDownRows;

If we're serious about the adaptive stuff, we should remove that check, probably...

Comment 4

[Doug Turner (:dougt)]

In firefox we do hit it.  for a simple select text case i made up, i see us entering this the block under the if with the following:

visibleHeight  5040
heightOfARow 240

Comment 5

[Boris Zbarsky [:bzbarsky]]

240 * 20 = 4800.  How many rows did your <select> have?  My point was that if it has fewer than 20 rows, we won't enter that code.

Comment 6

[Doug Turner (:dougt)]

I had kMaxDropDownRows = 8.  This result seams to be what I want on a qvga screen.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Right.  My point is we only use the adaptive algorithm once we're over kMaxDropDownRows right now.  Maybe we should change that.

Comment 8

[Doug Turner (:dougt)]

roc, bz, in the short term, would like to see a preference for the maximum 
number of items in a select before we add a scrollbar?

Comment 9

[Boris Zbarsky [:bzbarsky]]

Are we talking trunk or 1.8 branch?  On trunk, I'm going to somewhat heavily modify this code.

Comment 10

[Doug Turner (:dougt)]

Bz, i mostly care about the 1.8 Branch -- this is where I am shipping from for 
the next year (at least).  However, i would like this problem to go away also 
on the trunk.

For the branch, would you advise simply using a pref to control the 
kMaxDropDownRow?

Comment 11

[Boris Zbarsky [:bzbarsky]]

Probably...  You could nix the whole thing and just always do window-based sizing, but that would need some perf testing.

Comment 12

[Doug Turner (:dougt)]

bz, care to put up a strawman patch for testing? :-)

Comment 13

[Boris Zbarsky [:bzbarsky]]

Not likely to happen until July on this end.  :(

Comment 14

[Boris Zbarsky [:bzbarsky]]

See bug 314939

Comment 15

[Doug Turner (:dougt)]

bz, this is a dup of 314939, right?

Comment 16

[Boris Zbarsky [:bzbarsky]]

More or less, I think.  That bug is going off into behavior-change boonies that I'm not sure this bug wants to be saddled with...