Bug 12347

Summary:

REGRESSION: list box scrolling broken (fast/forms/listbox-selection.html)

Product:

WebKit

Reporter:

Darin Adler <darin>

Component:

Forms

Assignee:

Darin Adler <darin>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

ddkilzer

Priority:

Version:

420+

Hardware:

Mac

OS:

OS X 10.4

Attachments:

Description	Flags
patch with change log	none

Darin Adler

Reported 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.

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 attachment proposed patch, testcase, etc.

Darin Adler

Comment 1 2007-01-20 08:03:56 PST

Created attachment 12573 [details] patch with change log

Adam Roben (:aroben)

Comment 2 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.

mitz

Comment 3 2007-01-21 07:52:47 PST

Comment on attachment 12573 [details] patch with change log r=me

mitz

Comment 4 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!

Darin Adler

Comment 5 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.

Darin Adler

Comment 6 2007-01-21 08:34:04 PST

*** Bug 12351 has been marked as a duplicate of this bug. ***

Darin Adler

Comment 7 2007-01-21 08:38:54 PST

Committed revision 19008.

Darin Adler

Comment 8 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.

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