Fixed IndexOutOfBoundsException when swiping penultimate+last item of th...

[Gerlac]

...e list

Hi Roman,

First of all, thanks for your library.

I've discovered a bug in the library that is quite easy to reproduce: if you swipe the penultimate item of the list and just right after you start swiping the last element of the list (keep it pressed) before the performDismiss() method of the penultimate item gets called, the application throws an IndexOutOfBoundsException.

If I am not wrong, that happens because when you swipe the penultimate item, mAdapter.notifyDataSetChanged() gets called in the onDismiss() callback, the view gets refreshed, and MotionEvent.ACTION_UP gets called using the mDownPosition saved when MotionEvent.ACTION_DOWN. If performDismiss() gets called (depending on the X of the touch event), you might end up trying to remove an element with a stale position from the list. In case it is the last item of the list, that will lead to an IndexOutOfBoundsException.

I know the bug depends on the current implementation of the callback method onDismiss() (depends if you try to remove the item from the adapter). But I think that's one of the most common implementations.

I've just restored mDownPosition = ListView.INVALID_POSITION; just after calling mCallbacks.onDismiss(mListView, dismissPositions); in order to restore mDownPosition so I can avoid dismissing a view with a stale position when this happens.

This is just a quick fix to avoid the crash, I might be missing some other stuff. Please let me know if there's a better way to solve the issue.

Regards,
Gerlac

[cermakcz]

Hi Gerlac, I just stumbled upon something similar (might be the same actually). I fixed it with this:

case MotionEvent.ACTION_DOWN: {
  if (mPaused || mPendingDismisses.size() > 0) {
    return false;
  }

and it seems to be working. I just don't allow any swipes when there're some dismisses in progress. I think it's OK to disallow the swipe, because it's almost impossible to do it when the views are animating up. What do you think?

[romannurik]

@Gerlac I can't reproduce this with the sample app... are the repro steps: (1) swipe Item 19 (2) before animation finishes, start swiping Item 20 (3) when animation finishes keep swiping Item 20 (4) observe crash?

[Gerlac]

@cermakcz It looks to me that your solution would also solve the issue, but as you said, it would not allow any more swipe if there are some dismisses in progress. That might not produce the expected behaviour from a UX point of view, but it would solve the issue.

[Gerlac]

@romannurik It looks you were able to reproduce it. If you need more information about it just let me know.