From b54efd6206a009019dcdf2defed6bcf5c35505a5 Mon Sep 17 00:00:00 2001 From: Andrew Date: Tue, 14 May 2024 13:13:38 +0930 Subject: [PATCH 1/5] Clamp initial page of MediaPreview --- .../securesms/MediaPreviewActivity.java | 30 ++++++++++--------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/MediaPreviewActivity.java b/app/src/main/java/org/thoughtcrime/securesms/MediaPreviewActivity.java index b74638fec7..4c3c45ab53 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/MediaPreviewActivity.java +++ b/app/src/main/java/org/thoughtcrime/securesms/MediaPreviewActivity.java @@ -21,6 +21,7 @@ import android.annotation.TargetApi; import android.content.Context; import android.content.Intent; import android.database.Cursor; +import android.database.CursorIndexOutOfBoundsException; import android.net.Uri; import android.os.AsyncTask; import android.os.Build; @@ -526,23 +527,24 @@ public class MediaPreviewActivity extends PassphraseRequiredActionBarActivity im @Override public void onLoadFinished(@NonNull Loader> loader, @Nullable Pair data) { - if (data != null) { - CursorPagerAdapter adapter = new CursorPagerAdapter(this, GlideApp.with(this), getWindow(), data.first, data.second, leftIsRecent); - mediaPager.setAdapter(adapter); - adapter.setActive(true); + if (data == null) return; - viewModel.setCursor(this, data.first, leftIsRecent); + CursorPagerAdapter adapter = new CursorPagerAdapter(this, GlideApp.with(this), getWindow(), data.first, data.second, leftIsRecent); + mediaPager.setAdapter(adapter); + adapter.setActive(true); - if (restartItem >= 0 || data.second >= 0) { - int item = restartItem >= 0 ? restartItem : data.second; - mediaPager.setCurrentItem(item); + viewModel.setCursor(this, data.first, leftIsRecent); - if (item == 0) { - viewPagerListener.onPageSelected(0); - } - } else { - Log.w(TAG, "one of restartItem "+restartItem+" and data.second "+data.second+" would cause OOB exception"); - } + int item = restartItem >= 0 && restartItem < adapter.getCount() ? restartItem : Math.max(Math.min(data.second, adapter.getCount() - 1), 0); + + try { + mediaPager.setCurrentItem(item); + } catch (CursorIndexOutOfBoundsException e) { + throw new RuntimeException("restartItem = " + restartItem + ", data.second = " + data.second + " leftIsRecent = " + leftIsRecent, e); + } + + if (item == 0) { + viewPagerListener.onPageSelected(0); } } From be0b8007513a49cfc0dae7e3cd58997b83f68a83 Mon Sep 17 00:00:00 2001 From: Andrew Date: Tue, 14 May 2024 13:36:12 +0930 Subject: [PATCH 2/5] Move adapter to field --- .../securesms/MediaPreviewActivity.java | 59 +++++++------------ 1 file changed, 22 insertions(+), 37 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/MediaPreviewActivity.java b/app/src/main/java/org/thoughtcrime/securesms/MediaPreviewActivity.java index 4c3c45ab53..5f9ca6fecc 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/MediaPreviewActivity.java +++ b/app/src/main/java/org/thoughtcrime/securesms/MediaPreviewActivity.java @@ -146,6 +146,7 @@ public class MediaPreviewActivity extends PassphraseRequiredActionBarActivity im View.SYSTEM_UI_FLAG_HIDE_NAVIGATION); } }; + private MediaItemAdapter adapter; public static Intent getPreviewIntent(Context context, MediaPreviewArgs args) { return getPreviewIntent(context, args.getSlide(), args.getMmsRecord(), args.getThread()); @@ -379,7 +380,8 @@ public class MediaPreviewActivity extends PassphraseRequiredActionBarActivity im if (conversationRecipient != null) { getSupportLoaderManager().restartLoader(0, null, this); } else { - mediaPager.setAdapter(new SingleItemPagerAdapter(this, GlideApp.with(this), getWindow(), initialMediaUri, initialMediaType, initialMediaSize)); + adapter = new SingleItemPagerAdapter(this, GlideApp.with(this), getWindow(), initialMediaUri, initialMediaType, initialMediaSize); + mediaPager.setAdapter(adapter); if (initialCaption != null) { detailsContainer.setVisibility(View.VISIBLE); @@ -507,13 +509,8 @@ public class MediaPreviewActivity extends PassphraseRequiredActionBarActivity im } private @Nullable MediaItem getCurrentMediaItem() { - MediaItemAdapter adapter = (MediaItemAdapter)mediaPager.getAdapter(); - - if (adapter != null) { - return adapter.getMediaItemFor(mediaPager.getCurrentItem()); - } else { - return null; - } + if (adapter == null) return null; + return adapter.getMediaItemFor(mediaPager.getCurrentItem()); } public static boolean isContentTypeSupported(final String contentType) { @@ -529,9 +526,8 @@ public class MediaPreviewActivity extends PassphraseRequiredActionBarActivity im public void onLoadFinished(@NonNull Loader> loader, @Nullable Pair data) { if (data == null) return; - CursorPagerAdapter adapter = new CursorPagerAdapter(this, GlideApp.with(this), getWindow(), data.first, data.second, leftIsRecent); + adapter = new CursorPagerAdapter(this, GlideApp.with(this), getWindow(), data.first, data.second, leftIsRecent); mediaPager.setAdapter(adapter); - adapter.setActive(true); viewModel.setCursor(this, data.first, leftIsRecent); @@ -562,26 +558,22 @@ public class MediaPreviewActivity extends PassphraseRequiredActionBarActivity im if (currentPage != -1 && currentPage != position) onPageUnselected(currentPage); currentPage = position; - MediaItemAdapter adapter = (MediaItemAdapter)mediaPager.getAdapter(); + if (adapter == null) return; - if (adapter != null) { - MediaItem item = adapter.getMediaItemFor(position); - if (item.recipient != null) item.recipient.addListener(MediaPreviewActivity.this); - viewModel.setActiveAlbumRailItem(MediaPreviewActivity.this, position); - updateActionBar(); - } + MediaItem item = adapter.getMediaItemFor(position); + if (item.recipient != null) item.recipient.addListener(MediaPreviewActivity.this); + viewModel.setActiveAlbumRailItem(MediaPreviewActivity.this, position); + updateActionBar(); } public void onPageUnselected(int position) { - MediaItemAdapter adapter = (MediaItemAdapter)mediaPager.getAdapter(); + if (adapter == null) return; - if (adapter != null) { - MediaItem item = adapter.getMediaItemFor(position); - if (item.recipient != null) item.recipient.removeListener(MediaPreviewActivity.this); + MediaItem item = adapter.getMediaItemFor(position); + if (item.recipient != null) item.recipient.removeListener(MediaPreviewActivity.this); - adapter.pause(position); - } + adapter.pause(position); } @Override @@ -595,7 +587,7 @@ public class MediaPreviewActivity extends PassphraseRequiredActionBarActivity im } } - private static class SingleItemPagerAdapter extends PagerAdapter implements MediaItemAdapter { + private static class SingleItemPagerAdapter extends MediaItemAdapter { private final GlideRequests glideRequests; private final Window window; @@ -667,7 +659,7 @@ public class MediaPreviewActivity extends PassphraseRequiredActionBarActivity im } } - private static class CursorPagerAdapter extends PagerAdapter implements MediaItemAdapter { + private static class CursorPagerAdapter extends MediaItemAdapter { private final WeakHashMap mediaViews = new WeakHashMap<>(); @@ -677,7 +669,6 @@ public class MediaPreviewActivity extends PassphraseRequiredActionBarActivity im private final Cursor cursor; private final boolean leftIsRecent; - private boolean active; private int autoPlayPosition; CursorPagerAdapter(@NonNull Context context, @NonNull GlideRequests glideRequests, @@ -692,15 +683,9 @@ public class MediaPreviewActivity extends PassphraseRequiredActionBarActivity im this.leftIsRecent = leftIsRecent; } - public void setActive(boolean active) { - this.active = active; - notifyDataSetChanged(); - } - @Override public int getCount() { - if (!active) return 0; - else return cursor.getCount(); + return cursor.getCount(); } @Override @@ -802,9 +787,9 @@ public class MediaPreviewActivity extends PassphraseRequiredActionBarActivity im } } - interface MediaItemAdapter { - MediaItem getMediaItemFor(int position); - void pause(int position); - @Nullable View getPlaybackControls(int position); + abstract static class MediaItemAdapter extends PagerAdapter { + abstract MediaItem getMediaItemFor(int position); + abstract void pause(int position); + @Nullable abstract View getPlaybackControls(int position); } } From cee06bf7ee3a31ed631413ef815befaa81d3aec2 Mon Sep 17 00:00:00 2001 From: Andrew Date: Tue, 14 May 2024 13:43:25 +0930 Subject: [PATCH 3/5] Remove invalid ViewPagerListener before updating pager data --- .../org/thoughtcrime/securesms/MediaPreviewActivity.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/MediaPreviewActivity.java b/app/src/main/java/org/thoughtcrime/securesms/MediaPreviewActivity.java index 5f9ca6fecc..ed3af774b5 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/MediaPreviewActivity.java +++ b/app/src/main/java/org/thoughtcrime/securesms/MediaPreviewActivity.java @@ -287,9 +287,6 @@ public class MediaPreviewActivity extends PassphraseRequiredActionBarActivity im mediaPager = findViewById(R.id.media_pager); mediaPager.setOffscreenPageLimit(1); - viewPagerListener = new ViewPagerListener(); - mediaPager.addOnPageChangeListener(viewPagerListener); - albumRail = findViewById(R.id.media_preview_album_rail); albumRailAdapter = new MediaRailAdapter(GlideApp.with(this), this, false); @@ -526,6 +523,8 @@ public class MediaPreviewActivity extends PassphraseRequiredActionBarActivity im public void onLoadFinished(@NonNull Loader> loader, @Nullable Pair data) { if (data == null) return; + mediaPager.removeOnPageChangeListener(viewPagerListener); + adapter = new CursorPagerAdapter(this, GlideApp.with(this), getWindow(), data.first, data.second, leftIsRecent); mediaPager.setAdapter(adapter); @@ -533,6 +532,9 @@ public class MediaPreviewActivity extends PassphraseRequiredActionBarActivity im int item = restartItem >= 0 && restartItem < adapter.getCount() ? restartItem : Math.max(Math.min(data.second, adapter.getCount() - 1), 0); + viewPagerListener = new ViewPagerListener(); + mediaPager.addOnPageChangeListener(viewPagerListener); + try { mediaPager.setCurrentItem(item); } catch (CursorIndexOutOfBoundsException e) { From 41dde125303d29e01504423e127bae11b0cbca59 Mon Sep 17 00:00:00 2001 From: Andrew Date: Tue, 14 May 2024 14:35:21 +0930 Subject: [PATCH 4/5] Clamp MediaPreview#getCursorPosition --- .../thoughtcrime/securesms/MediaPreviewActivity.java | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/MediaPreviewActivity.java b/app/src/main/java/org/thoughtcrime/securesms/MediaPreviewActivity.java index ed3af774b5..7e4f3a4e01 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/MediaPreviewActivity.java +++ b/app/src/main/java/org/thoughtcrime/securesms/MediaPreviewActivity.java @@ -219,13 +219,6 @@ public class MediaPreviewActivity extends PassphraseRequiredActionBarActivity im Permissions.onRequestPermissionsResult(this, requestCode, permissions, grantResults); } - @TargetApi(VERSION_CODES.JELLY_BEAN) - private void setFullscreenIfPossible() { - if (VERSION.SDK_INT >= VERSION_CODES.JELLY_BEAN) { - getWindow().getDecorView().setSystemUiVisibility(View.SYSTEM_UI_FLAG_FULLSCREEN); - } - } - @Override public void onModified(Recipient recipient) { Util.runOnMain(this::updateActionBar); @@ -760,8 +753,8 @@ public class MediaPreviewActivity extends PassphraseRequiredActionBarActivity im } private int getCursorPosition(int position) { - if (leftIsRecent) return position; - else return cursor.getCount() - 1 - position; + int unclamped = leftIsRecent ? position : cursor.getCount() - 1 - position; + return Math.max(Math.min(unclamped, cursor.getCount() - 1), 0); } } From 9f150391888af2cc2299fc9bc73b3448911cf91a Mon Sep 17 00:00:00 2001 From: Andrew Date: Tue, 14 May 2024 15:25:45 +0930 Subject: [PATCH 5/5] Add more info to unselect exception --- .../org/thoughtcrime/securesms/MediaPreviewActivity.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/MediaPreviewActivity.java b/app/src/main/java/org/thoughtcrime/securesms/MediaPreviewActivity.java index 7e4f3a4e01..2e67becbfd 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/MediaPreviewActivity.java +++ b/app/src/main/java/org/thoughtcrime/securesms/MediaPreviewActivity.java @@ -565,8 +565,12 @@ public class MediaPreviewActivity extends PassphraseRequiredActionBarActivity im public void onPageUnselected(int position) { if (adapter == null) return; - MediaItem item = adapter.getMediaItemFor(position); - if (item.recipient != null) item.recipient.removeListener(MediaPreviewActivity.this); + try { + MediaItem item = adapter.getMediaItemFor(position); + if (item.recipient != null) item.recipient.removeListener(MediaPreviewActivity.this); + } catch (CursorIndexOutOfBoundsException e) { + throw new RuntimeException("position = " + position + " leftIsRecent = " + leftIsRecent, e); + } adapter.pause(position); }