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

• Status: RESOLVED
• Resolution: FIXED
• Component: Forms

[Darin Adler]

This test shows that making selections on list boxes that are not currently laid out scrolls them incorrectly. I have a fix.

[Darin Adler]

Created attachment 12573 [details]
patch with change log

[Adam Roben (:aroben)]

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 on attachment 12573 [details]
patch with change log

r=me

Comment on attachment 12573 [details]
patch with change log

Hm... I didn't notice Adam's question. Sorry!

[Darin Adler]

(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]

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

[Darin Adler]

Committed revision 19008.

[Darin Adler]

Comment on attachment 12573 [details]
patch with change log

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