While testing a11y-slider we ran into a bug with Firefox and navigating with the prev/next buttons.
When pressing one of the button quickly the slide would first animate the change to the correct slide and then jump back to an earlier one.
Steps to reproduce:
Open a page with a slider and multiple slides in Firefox
Add the styles display:grid and position:relative to a11y-slider container element.
Click on the next button several times (quickly)
See the slide scroll to the correct slide than suddenly jump back to the earlier one.
While we were able circumvent the bug by overriding the position:relative added by the a11y-slider container, there were multiple things that looked strange and perhaps are involved in causing the problem:
_goPrevOrNext() calculates the slide to navigate to based on the current scroll position (via _getActiveAndVisible()). When clicking on a button while the animation is still ongoing the resulting target slide might be wrong.
scrollToSlide() calls _setCSS(targetSlide) asynchronously which seems odd because by the time it is invoked the targetSlide might not actually be the position the user navigated to. The comment or the commit history are not clear of why this call is even needed. Deleting the entire setTimeout() block had no effect in our tests, which means it also didn’t fix the bug.
Some more observations that make things hard to debug (and might be the cause of yet unknown issues) are:
Since this.activeSlide is only ever calculated trough _getActiveAndVisible() it’s very odd to store it as property of the slider object. It’s value is best described by “the active slide at the point in time _getActiveAndVisible() was last called.
There is no respresentation of the user’s mental model of navigating through the slides when using the buttons. “I’m on slide 1, when pressing the next button twice I will end up on slide 3”. Actually this.activeSlide sounds a lot like it would store that state but it doesn’t.
When swiping or resizing the window the position is the user input, so it’s the actual authorative information of where the animation should stop. When using the buttons this has to be turned around to determine the target scroll position based on the user input. The latter doesn’t seem to happen. Also there needs to be a clear switch between these modes.
While testing a11y-slider we ran into a bug with Firefox and navigating with the prev/next buttons. When pressing one of the button quickly the slide would first animate the change to the correct slide and then jump back to an earlier one.
Steps to reproduce:
display:grid
andposition:relative
to a11y-slider container element.While we were able circumvent the bug by overriding the
position:relative
added by the a11y-slider container, there were multiple things that looked strange and perhaps are involved in causing the problem:_goPrevOrNext()
calculates the slide to navigate to based on the current scroll position (via_getActiveAndVisible()
). When clicking on a button while the animation is still ongoing the resultingtarget
slide might be wrong.scrollToSlide()
calls_setCSS(targetSlide)
asynchronously which seems odd because by the time it is invoked thetargetSlide
might not actually be the position the user navigated to. The comment or the commit history are not clear of why this call is even needed. Deleting the entiresetTimeout()
block had no effect in our tests, which means it also didn’t fix the bug.Some more observations that make things hard to debug (and might be the cause of yet unknown issues) are:
this.activeSlide
is only ever calculated trough_getActiveAndVisible()
it’s very odd to store it as property of the slider object. It’s value is best described by “the active slide at the point in time_getActiveAndVisible()
was last called.this.activeSlide
sounds a lot like it would store that state but it doesn’t.