Closed Bug 957933 Opened 10 years ago Closed 10 years ago

Dispatch an event to let tests know when arrowscrollbox smooth scroll animation finishes

Categories

(Toolkit :: UI Widgets, defect)

defect
Not set
minor

Tracking

()

RESOLVED WONTFIX

People

(Reporter: mbrubeck, Assigned: mbrubeck)

References

Details

Attachments

(1 file)

Attached patch patchSplinter Review
Bug 952297 added test code that triggers smooth scrolling in an <arrowscrollbox> and needs to wait until the smooth scroll finishes.  Currently it uses a delay that matches the animation duration, but it would be more robust if it could listen for some sort of event to signal the end of the scrolling animation.
Attachment #8357605 - Flags: review?(dao)
Comment on attachment 8357605 [details] [diff] [review]
patch

Why is this event specific to smooth scrolling? Seems like code could start depending on it and break when smooth scrolling is disabled. Also note that _stopSmoothScroll is called in various spots and doesn't necessarily mean that the whole scroll operation got completed. It can also mean one scroll operation is canceled due to a new one starting. All in all, I'm not sure that's a worthwhile and sane API.
Attachment #8357605 - Flags: review?(dao) → review-
Do you think there's a reasonable new API we *should* add here, or should we just WONTFIX this bug?  I'm happy either way; we could probably find other workarounds for the affected test code, like having it directly call a non-smooth scroll method.

To solve the more general case, it looks like we could add an event that fires when either _smoothScrollByPixels or _scrollByPixels finishes.  In the smooth scrolling case, we'd fire the event only if we pass the (pos == 1) check in _scrollAnim.sample, to avoid the problem with "canceled" animations.
Flags: needinfo?(dao)
(In reply to Matt Brubeck (:mbrubeck) from comment #2)
> To solve the more general case, it looks like we could add an event that
> fires when either _smoothScrollByPixels or _scrollByPixels finishes.  In the
> smooth scrolling case, we'd fire the event only if we pass the (pos == 1)
> check in _scrollAnim.sample, to avoid the problem with "canceled" animations.

This sounds like it would work, but if this is only motivated by the test and there are no other obvious use cases, I think we can WONTFIX this.
Flags: needinfo?(dao)
Okay, thanks.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: