Bug 12347 - REGRESSION: list box scrolling broken (fast/forms/listbox-selection.html)
Summary: REGRESSION: list box scrolling broken (fast/forms/listbox-selection.html)
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P1 Normal
Assignee: Darin Adler
URL:
Keywords:
: 12351 (view as bug list)
Depends on:
Blocks:
 
Reported: 2007-01-20 07:50 PST by Darin Adler
Modified: 2007-01-21 08:50 PST (History)
1 user (show)

See Also:

Attachments
patch with change log (5.52 KB, patch)
2007-01-20 08:03 PST, Darin Adler 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Darin Adler 2007-01-20 07:50:46 PST 
This test shows that making selections on list boxes that are not currently laid out scrolls them incorrectly. I have a fix.
Comment 1 Darin Adler 2007-01-20 08:03:56 PST 
Created attachment 12573 [details]
patch with change log
Comment 2 Adam Roben (:aroben) 2007-01-20 15:22:15 PST 
Comment on attachment 12573 [details]
patch with change log

+    if (firstIndex >= 0 && !listIndexIsVisible(select->lastSelectedListIndex()))
         scrollToRevealElementAtListIndex(firstIndex);

   Won't this cause scrolling even if the first selected item is visible?

   Also, given the description of the problem, the bug title seems a little too general.
Comment 3 mitz 2007-01-21 07:52:47 PST 
Comment on attachment 12573 [details]
patch with change log

r=me
Comment 4 mitz 2007-01-21 07:54:58 PST 
Comment on attachment 12573 [details]
patch with change log

Hm... I didn't notice Adam's question. Sorry!
Comment 5 Darin Adler 2007-01-21 08:01:00 PST 
(In reply to comment #2)
> (From update of attachment 12573 [details] [edit])
> +    if (firstIndex >= 0 &&
> !listIndexIsVisible(select->lastSelectedListIndex()))
>          scrollToRevealElementAtListIndex(firstIndex);
> 
>    Won't this cause scrolling even if the first selected item is visible?

No. The scrollToRevealElementAtListIndex function checks that and does nothing in that case. That's the reason I removed the redundant test at the call site.

>    Also, given the description of the problem, the bug title seems a little too
> general.
 
I agree, but couldn't think of a better title.
Comment 6 Darin Adler 2007-01-21 08:34:04 PST 
*** Bug 12351 has been marked as a duplicate of this bug. ***
Comment 7 Darin Adler 2007-01-21 08:38:54 PST 
Committed revision 19008.
Comment 8 Darin Adler 2007-01-21 08:50:06 PST 
Comment on attachment 12573 [details]
patch with change log

Since both Mitz and Adam reviewed this, I landed it. Clearing the review flag.