From e83798a7c4ce66290a9be8dcc2f0d155dad6a59f Mon Sep 17 00:00:00 2001 From: Ahmed El-Helw Date: Sun, 28 Jan 2024 00:45:24 +0400 Subject: [PATCH] Fixes for a plethora of crashes --- .../labs/androidquran/QuranDataActivity.kt | 9 ++- .../pageselect/PageSelectPresenter.kt | 31 +++++++-- .../labs/androidquran/service/AudioService.kt | 63 ++++++++++++++----- .../labs/androidquran/ui/PagerActivity.java | 8 ++- .../androidquran/view/AudioStatusBar.java | 5 +- .../androidquran/view/CurrentQariBridge.kt | 14 ++--- .../view/HighlightingImageView.java | 26 ++++---- .../mobile/bookmark/model/BookmarkModel.kt | 9 +-- 8 files changed, 117 insertions(+), 48 deletions(-) diff --git a/app/src/main/java/com/quran/labs/androidquran/QuranDataActivity.kt b/app/src/main/java/com/quran/labs/androidquran/QuranDataActivity.kt index a0ff0b8952..cda14007d4 100644 --- a/app/src/main/java/com/quran/labs/androidquran/QuranDataActivity.kt +++ b/app/src/main/java/com/quran/labs/androidquran/QuranDataActivity.kt @@ -631,9 +631,16 @@ class QuranDataActivity : Activity(), SimpleDownloadListener, OnRequestPermissio if (downloadReceiver != null && downloadReceiver!!.didReceiveBroadcast() && !force) { return } + val dataStatus = quranDataStatus + if (dataStatus == null) { + // we lost the cached data status, so just check again + checkPages() + return + } + var url: String - url = if (dataStatus!!.needPortrait() && !dataStatus.needLandscape()) { + url = if (dataStatus.needPortrait() && !dataStatus.needLandscape()) { // phone (and tablet when upgrading on some devices, ex n10) quranFileUtils.zipFileUrl } else if (dataStatus.needLandscape() && !dataStatus.needPortrait()) { diff --git a/app/src/main/java/com/quran/labs/androidquran/pageselect/PageSelectPresenter.kt b/app/src/main/java/com/quran/labs/androidquran/pageselect/PageSelectPresenter.kt index 19e8b43e8d..f0c8c6ee3f 100644 --- a/app/src/main/java/com/quran/labs/androidquran/pageselect/PageSelectPresenter.kt +++ b/app/src/main/java/com/quran/labs/androidquran/pageselect/PageSelectPresenter.kt @@ -101,14 +101,33 @@ constructor( val updatedBookmarks = bookmarksDao.bookmarks() .map { val page = it.page - val (pageSura, pageAyah) = suraAyahFromPage(page) - val sura = it.sura ?: pageSura - val ayah = it.ayah ?: pageAyah + if (page - 1 >= sourcePageSuraStart.size) { + if (it.isPageBookmark()) { + // this bookmark is on a page that doesn't exist in the old page type + if (destination.suraForPageArray.size > page) { + // but it does exist on the new type, so it's ok, let's not re-map + it + } else { + // we can't map it, so let's just put it as the max page number to avoid bad data + it.copy(page = sourcePageAyahStart.size - 1) + } + } else { + // ayah bookmark, so let's just map it + val sura = requireNotNull(it.sura) + val ayah = requireNotNull(it.ayah) + val mappedPage = destinationQuranInfo.getPageFromSuraAyah(sura, ayah) + it.copy(page = mappedPage) + } + } else { + val (pageSura, pageAyah) = suraAyahFromPage(page) + val sura = it.sura ?: pageSura + val ayah = it.ayah ?: pageAyah - val mappedPage = destinationQuranInfo.getPageFromSuraAyah(sura, ayah) + val mappedPage = destinationQuranInfo.getPageFromSuraAyah(sura, ayah) - // we only copy the page because sura and ayah are the same. - it.copy(page = mappedPage) + // we only copy the page because sura and ayah are the same. + it.copy(page = mappedPage) + } } if (updatedBookmarks.isNotEmpty()) { diff --git a/app/src/main/java/com/quran/labs/androidquran/service/AudioService.kt b/app/src/main/java/com/quran/labs/androidquran/service/AudioService.kt index fbf812b05b..b7586b2d0e 100644 --- a/app/src/main/java/com/quran/labs/androidquran/service/AudioService.kt +++ b/app/src/main/java/com/quran/labs/androidquran/service/AudioService.kt @@ -221,6 +221,13 @@ class AudioService : Service(), OnCompletionListener, OnPreparedListener, localPlayer.setOnCompletionListener(this) localPlayer.setOnErrorListener(this) localPlayer.setOnSeekCompleteListener(this) + + val audioAttributes = AudioAttributes.Builder() + .setContentType(AudioAttributes.CONTENT_TYPE_MUSIC) + .setUsage(AudioAttributes.USAGE_MEDIA) + .build() + localPlayer.setAudioAttributes(audioAttributes) + mediaSession.isActive = true localPlayer } else { @@ -979,12 +986,6 @@ class AudioService : Service(), OnCompletionListener, OnPreparedListener, state = State.Preparing updateAudioPlaybackStatus() - val audioAttributes = AudioAttributes.Builder() - .setContentType(AudioAttributes.CONTENT_TYPE_MUSIC) - .setUsage(AudioAttributes.USAGE_MEDIA) - .build() - localPlayer.setAudioAttributes(audioAttributes) - // starts preparing the media player in the background. When it's // done, it will call our OnPreparedListener (that is, the // onPrepared() method on this class, since we set the listener @@ -1018,15 +1019,47 @@ class AudioService : Service(), OnCompletionListener, OnPreparedListener, } val builder = PlaybackStateCompat.Builder() builder.setState(state, position, 1.0f) - builder.setActions( - PlaybackStateCompat.ACTION_PLAY or - PlaybackStateCompat.ACTION_STOP or - PlaybackStateCompat.ACTION_REWIND or - PlaybackStateCompat.ACTION_FAST_FORWARD or - PlaybackStateCompat.ACTION_PAUSE or - PlaybackStateCompat.ACTION_SKIP_TO_PREVIOUS or - PlaybackStateCompat.ACTION_SKIP_TO_NEXT - ) + + val actions = when (state) { + PlaybackStateCompat.STATE_PLAYING -> { + PlaybackStateCompat.ACTION_PAUSE or + PlaybackStateCompat.ACTION_STOP or + PlaybackStateCompat.ACTION_REWIND or + PlaybackStateCompat.ACTION_FAST_FORWARD or + PlaybackStateCompat.ACTION_SKIP_TO_PREVIOUS or + PlaybackStateCompat.ACTION_SKIP_TO_NEXT + } + PlaybackStateCompat.STATE_PAUSED -> { + PlaybackStateCompat.ACTION_PLAY or + PlaybackStateCompat.ACTION_STOP or + PlaybackStateCompat.ACTION_REWIND or + PlaybackStateCompat.ACTION_FAST_FORWARD or + PlaybackStateCompat.ACTION_SKIP_TO_PREVIOUS or + PlaybackStateCompat.ACTION_SKIP_TO_NEXT + } + PlaybackStateCompat.STATE_STOPPED -> { + PlaybackStateCompat.ACTION_PLAY + } + PlaybackStateCompat.STATE_CONNECTING -> { + PlaybackStateCompat.ACTION_STOP + } + PlaybackStateCompat.STATE_REWINDING -> { + PlaybackStateCompat.ACTION_STOP or + PlaybackStateCompat.ACTION_REWIND or + PlaybackStateCompat.ACTION_FAST_FORWARD or + PlaybackStateCompat.ACTION_SKIP_TO_PREVIOUS or + PlaybackStateCompat.ACTION_SKIP_TO_NEXT + } + PlaybackStateCompat.STATE_SKIPPING_TO_NEXT -> { + PlaybackStateCompat.ACTION_STOP or + PlaybackStateCompat.ACTION_SKIP_TO_PREVIOUS or + PlaybackStateCompat.ACTION_SKIP_TO_NEXT or + PlaybackStateCompat.ACTION_FAST_FORWARD or + PlaybackStateCompat.ACTION_REWIND + } + else -> { PlaybackStateCompat.ACTION_STOP } + } + builder.setActions(actions) mediaSession.setPlaybackState(builder.build()) } diff --git a/app/src/main/java/com/quran/labs/androidquran/ui/PagerActivity.java b/app/src/main/java/com/quran/labs/androidquran/ui/PagerActivity.java index b9681bbd9a..fa9f70df5c 100644 --- a/app/src/main/java/com/quran/labs/androidquran/ui/PagerActivity.java +++ b/app/src/main/java/com/quran/labs/androidquran/ui/PagerActivity.java @@ -759,8 +759,12 @@ public void onResume() { foregroundDisposable.add(Completable.timer(500, TimeUnit.MILLISECONDS) .observeOn(AndroidSchedulers.mainThread()) .subscribe(() -> { - startService( - audioUtils.getAudioIntent(PagerActivity.this, AudioService.ACTION_CONNECT)); + try { + startService( + audioUtils.getAudioIntent(PagerActivity.this, AudioService.ACTION_CONNECT)); + } catch (IllegalStateException ise) { + // we're likely in the background, so ignore. + } shouldReconnect = false; })); } diff --git a/app/src/main/java/com/quran/labs/androidquran/view/AudioStatusBar.java b/app/src/main/java/com/quran/labs/androidquran/view/AudioStatusBar.java index 3b939eaf02..f83ebd4a70 100644 --- a/app/src/main/java/com/quran/labs/androidquran/view/AudioStatusBar.java +++ b/app/src/main/java/com/quran/labs/androidquran/view/AudioStatusBar.java @@ -261,8 +261,9 @@ private void showStoppedMode() { private void updateButton() { final TextView currentQariView = qariView; - if (currentQariView != null && currentQari != null) { - currentQariView.setText(currentQari.getNameResource()); + final Qari qari = currentQari; + if (currentQariView != null && qari != null) { + currentQariView.setText(qari.getNameResource()); } } diff --git a/app/src/main/java/com/quran/labs/androidquran/view/CurrentQariBridge.kt b/app/src/main/java/com/quran/labs/androidquran/view/CurrentQariBridge.kt index 3d19db653d..332eab05d9 100644 --- a/app/src/main/java/com/quran/labs/androidquran/view/CurrentQariBridge.kt +++ b/app/src/main/java/com/quran/labs/androidquran/view/CurrentQariBridge.kt @@ -10,20 +10,20 @@ import kotlinx.coroutines.launch import javax.inject.Inject import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.flow.flowOn +import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.observeOn +import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.withContext class CurrentQariBridge @Inject constructor(private val currentQariManager: CurrentQariManager) { private val scope: CoroutineScope = CoroutineScope(SupervisorJob()) fun listenToQaris(lambda: ((Qari) -> Unit)) { - scope.launch { - withContext(Dispatchers.Main) { - currentQariManager - .flow() - .collect { lambda(it) } - } - } + currentQariManager + .flow() + .onEach { lambda(it) } + .flowOn(Dispatchers.Main) + .launchIn(scope) } fun unsubscribeAll() { diff --git a/app/src/main/java/com/quran/labs/androidquran/view/HighlightingImageView.java b/app/src/main/java/com/quran/labs/androidquran/view/HighlightingImageView.java index 6f48e83e65..33ce5a2bf4 100644 --- a/app/src/main/java/com/quran/labs/androidquran/view/HighlightingImageView.java +++ b/app/src/main/java/com/quran/labs/androidquran/view/HighlightingImageView.java @@ -1,5 +1,11 @@ package com.quran.labs.androidquran.view; +import static com.quran.data.model.highlight.HighlightType.Mode.BACKGROUND; +import static com.quran.data.model.highlight.HighlightType.Mode.COLOR; +import static com.quran.data.model.highlight.HighlightType.Mode.HIDE; +import static com.quran.data.model.highlight.HighlightType.Mode.HIGHLIGHT; +import static com.quran.data.model.highlight.HighlightType.Mode.UNDERLINE; + import android.animation.Animator; import android.animation.ValueAnimator; import android.content.Context; @@ -15,6 +21,11 @@ import android.graphics.drawable.Drawable; import android.util.AttributeSet; +import androidx.annotation.NonNull; +import androidx.appcompat.widget.AppCompatImageView; +import androidx.core.content.ContextCompat; +import androidx.core.view.DisplayCutoutCompat; + import com.quran.data.model.highlight.HighlightType; import com.quran.labs.androidquran.R; import com.quran.labs.androidquran.data.Constants; @@ -37,19 +48,8 @@ import java.util.SortedMap; import java.util.TreeMap; -import androidx.annotation.NonNull; -import androidx.appcompat.widget.AppCompatImageView; -import androidx.core.content.ContextCompat; -import androidx.core.view.DisplayCutoutCompat; import dev.chrisbanes.insetter.Insetter; -import static com.quran.data.model.highlight.HighlightType.Mode.BACKGROUND; -import static com.quran.data.model.highlight.HighlightType.Mode.COLOR; -import static com.quran.data.model.highlight.HighlightType.Mode.HIDE; -import static com.quran.data.model.highlight.HighlightType.Mode.HIGHLIGHT; -import static com.quran.data.model.highlight.HighlightType.Mode.UNDERLINE; -import java.lang.Math; - public class HighlightingImageView extends AppCompatImageView { // for debugging / visualizing glyph bounds: // when enabled, will draw bounds around each glyph to visualize the glyph bounds @@ -276,6 +276,10 @@ private void highlightFloatableAyah(Set highlights, AyahHighlight final TransitionAyahHighlight transitionHighlight = new TransitionAyahHighlight(sourceHighlight, destinationHighlight); + if (startingBounds == null) { + startingBounds = new ArrayList<>(); + } + // yes we make copies, because normalizing the bounds will change them List sourceBounds = new ArrayList<>(startingBounds); diff --git a/common/bookmark/src/main/java/com/quran/mobile/bookmark/model/BookmarkModel.kt b/common/bookmark/src/main/java/com/quran/mobile/bookmark/model/BookmarkModel.kt index 4c31a56c9a..6729cc8e37 100644 --- a/common/bookmark/src/main/java/com/quran/mobile/bookmark/model/BookmarkModel.kt +++ b/common/bookmark/src/main/java/com/quran/mobile/bookmark/model/BookmarkModel.kt @@ -2,7 +2,6 @@ package com.quran.mobile.bookmark.model import app.cash.sqldelight.coroutines.asFlow import app.cash.sqldelight.coroutines.mapToList -import app.cash.sqldelight.coroutines.mapToOneOrNull import com.quran.data.model.SuraAyah import com.quran.data.model.bookmark.Bookmark import com.quran.labs.androidquran.BookmarksDatabase @@ -21,10 +20,12 @@ class BookmarkModel @Inject constructor(bookmarksDatabase: BookmarksDatabase) { .mapToList(Dispatchers.IO) suspend fun isSuraAyahBookmarked(suraAyah: SuraAyah): Pair { - val bookmarkId = bookmarkQueries.getBookmarkIdForSuraAyah(suraAyah.sura, suraAyah.ayah) + val bookmarkIds = bookmarkQueries.getBookmarkIdForSuraAyah(suraAyah.sura, suraAyah.ayah) .asFlow() - .mapToOneOrNull(Dispatchers.IO) + // was .mapToOneOrNull, but some people have multiple bookmarks for the same ayah + // should try to figure out why at some point or otherwise de-duplicate them + .mapToList(Dispatchers.IO) .first() - return suraAyah to (bookmarkId != null) + return suraAyah to bookmarkIds.isNotEmpty() } }