[Android] Fix IndexOutOfBoundsException on media delete fail

Geoffrey Métais geoffrey.metais at gmail.com
Tue Jan 9 16:31:42 CET 2018


Yes exactly,

And reinserting it at a wrong position could mess up the current sorting.
Or if item is added at the end of the list, user is likely to not see it
showing back at all.

I don't have any good solution for this particular case.

At least, user is now warned by a snack that deletion failed. So user won't
be surprised to see it appear later.

For successful reinsertion, we may use diffutil for safety.

Le mar. 9 janv. 2018 à 16:06, Romain Vimont <rom at rom1v.com> a écrit :

> On Tue, Jan 09, 2018 at 03:36:54PM +0100, Geoffrey Métais wrote:
> > vlc-android | branch: master | Geoffrey Métais <
> geoffrey.metais at gmail.com> | Tue Jan  9 09:51:49 2018 +0100|
> [5cc334ce561866ee91cab70f533732c845b78829] | committer: Geoffrey Métais
> >
> > Fix IndexOutOfBoundsException on media delete fail
> >
> > >
> https://code.videolan.org/videolan/vlc-android/commit/5cc334ce561866ee91cab70f533732c845b78829
> > ---
> >
> >  vlc-android/src/org/videolan/vlc/gui/audio/AudioBrowserAdapter.java | 1
> +
> >  vlc-android/src/org/videolan/vlc/gui/video/VideoListAdapter.java    | 1
> +
> >  2 files changed, 2 insertions(+)
> >
> > diff --git
> a/vlc-android/src/org/videolan/vlc/gui/audio/AudioBrowserAdapter.java
> b/vlc-android/src/org/videolan/vlc/gui/audio/AudioBrowserAdapter.java
> > index c0df3a9a1..a2930c820 100644
> > --- a/vlc-android/src/org/videolan/vlc/gui/audio/AudioBrowserAdapter.java
> > +++ b/vlc-android/src/org/videolan/vlc/gui/audio/AudioBrowserAdapter.java
> > @@ -333,6 +333,7 @@ public class AudioBrowserAdapter extends
> SortableAdapter<MediaLibraryItem, Audio
> >
> >      public void addItem(final int position, final MediaLibraryItem
> item) {
> >          final List<MediaLibraryItem> referenceList = peekLast();
> > +        if (position < 0 || position >= referenceList.size()) return;
>
> IMO, providing a valid position is the caller responsability. Here, the
> error is silently ignored.
>
> This method is always called from a Runnable passed to
> UiTools.snackerWithCancel(), relying on the position of the item when it
> was deleted. But the list may have changed in the meantime, so adding
> the item back should not rely on the position.
>
> In the worst case, the position is out-of-range (with this patch it does
> not crash anymore, but the item is not added back); but even when it is
> in-range, there is no guarantee that the position is correct.
>
> >          final List<MediaLibraryItem> dataList = new
> ArrayList<>(referenceList);
> >          dataList.add(position,item);
> >          update(dataList);
> > diff --git
> a/vlc-android/src/org/videolan/vlc/gui/video/VideoListAdapter.java
> b/vlc-android/src/org/videolan/vlc/gui/video/VideoListAdapter.java
> > index 2f5863fcf..4ad8f091b 100644
> > --- a/vlc-android/src/org/videolan/vlc/gui/video/VideoListAdapter.java
> > +++ b/vlc-android/src/org/videolan/vlc/gui/video/VideoListAdapter.java
> > @@ -148,6 +148,7 @@ public class VideoListAdapter extends
> SortableAdapter<MediaWrapper, VideoListAda
> >      @MainThread
> >      public void add(MediaWrapper item, int position) {
> >          final List<MediaWrapper> list = new ArrayList<>(peekLast());
> > +        if (position < 0 || position >= list.size()) return;
> >          list.add(position, item);
> >          update(list);
> >      }
> >
> > _______________________________________________
> > Android mailing list
> > Android at videolan.org
> > https://mailman.videolan.org/listinfo/android
> _______________________________________________
> Android mailing list
> Android at videolan.org
> https://mailman.videolan.org/listinfo/android
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/android/attachments/20180109/ae68fa2c/attachment.html>


More information about the Android mailing list