From 3ac703a832ca3aafbab70e2e0b34b13eb5fac3eb Mon Sep 17 00:00:00 2001 From: Jonas Kalderstam Date: Tue, 14 Nov 2023 15:17:54 +0100 Subject: [PATCH 1/4] Fixed crash when table had no columns --- .../ui/compose/editfeed/EditFeedScreen.kt | 6 +- .../ui/compose/feedarticle/ArticleScreen.kt | 5 +- .../ui/compose/text/HtmlToComposable.kt | 104 +++++++++--------- .../ui/compose/text/HtmlToComposableKtTest.kt | 14 --- 4 files changed, 57 insertions(+), 72 deletions(-) delete mode 100644 app/src/test/java/com/nononsenseapps/feeder/ui/compose/text/HtmlToComposableKtTest.kt diff --git a/app/src/main/java/com/nononsenseapps/feeder/ui/compose/editfeed/EditFeedScreen.kt b/app/src/main/java/com/nononsenseapps/feeder/ui/compose/editfeed/EditFeedScreen.kt index 27ff429e7d..663f31600a 100644 --- a/app/src/main/java/com/nononsenseapps/feeder/ui/compose/editfeed/EditFeedScreen.kt +++ b/app/src/main/java/com/nononsenseapps/feeder/ui/compose/editfeed/EditFeedScreen.kt @@ -46,7 +46,6 @@ import androidx.compose.ui.ExperimentalComposeUiApi import androidx.compose.ui.Modifier import androidx.compose.ui.focus.FocusDirection import androidx.compose.ui.focus.FocusRequester -import androidx.compose.ui.focus.FocusRequester.Companion.createRefs import androidx.compose.ui.focus.focusProperties import androidx.compose.ui.focus.focusRequester import androidx.compose.ui.focus.onFocusChanged @@ -254,9 +253,8 @@ fun EditFeedView( ) { val dimens = LocalDimens.current - val (leftFocusRequester, rightFocusRequester) = remember { - createRefs() - } + val leftFocusRequester = remember { FocusRequester() } + val rightFocusRequester = remember { FocusRequester() } OkCancelWithContent( onOk = { diff --git a/app/src/main/java/com/nononsenseapps/feeder/ui/compose/feedarticle/ArticleScreen.kt b/app/src/main/java/com/nononsenseapps/feeder/ui/compose/feedarticle/ArticleScreen.kt index da11904180..cdc142460b 100644 --- a/app/src/main/java/com/nononsenseapps/feeder/ui/compose/feedarticle/ArticleScreen.kt +++ b/app/src/main/java/com/nononsenseapps/feeder/ui/compose/feedarticle/ArticleScreen.kt @@ -188,9 +188,8 @@ fun ArticleScreen( bottomBarVisibleState.targetState = viewState.isBottomBarVisible } - val (focusArticle, focusTopBar) = remember { - FocusRequester.createRefs() - } + val focusArticle = remember { FocusRequester() } + val focusTopBar = remember { FocusRequester() } Scaffold( modifier = modifier diff --git a/app/src/main/java/com/nononsenseapps/feeder/ui/compose/text/HtmlToComposable.kt b/app/src/main/java/com/nononsenseapps/feeder/ui/compose/text/HtmlToComposable.kt index eaec908796..49ba162ed1 100644 --- a/app/src/main/java/com/nononsenseapps/feeder/ui/compose/text/HtmlToComposable.kt +++ b/app/src/main/java/com/nononsenseapps/feeder/ui/compose/text/HtmlToComposable.kt @@ -888,7 +888,7 @@ private fun EagerComposer.tableColFirst( .filter { it.tagName() in setOf("th", "td") }.count() - }.max() + }.maxOrNull() ?: 0 } } @@ -952,58 +952,60 @@ private fun EagerComposer.tableColFirst( } key(rowCount, colCount, rowData, baseUrl, onLinkClick) { - LazyRow( - horizontalArrangement = Arrangement.spacedBy(32.dp), - modifier = Modifier - .horizontalScroll(rememberScrollState()) - .width(dimens.maxReaderWidth), - ) { - items( - count = colCount, - ) { colIndex -> - Column( - verticalArrangement = Arrangement.spacedBy(4.dp), - modifier = Modifier, - ) { - for (rowIndex in 0 until rowCount) { - val (section, rowElement) = rowData.getOrNull(rowIndex) ?: break - var emptyCell = false - Surface( - tonalElevation = when (section) { - "thead" -> 3.dp - "tbody" -> 0.dp - "tfoot" -> 1.dp - else -> 0.dp - }, - ) { - rowElement.children() - .filter { it.tagName() in setOf("th", "td") } - .elementAtOrNullWithSpans(colIndex) - ?.let { colElement -> - withParagraph { - withStyle( - if (colElement.tagName() == "th") { - SpanStyle(fontWeight = FontWeight.Bold) - } else { - null - }, - ) { - appendTextChildren( - colElement.childNodes(), - baseUrl = baseUrl, - onLinkClick = onLinkClick, - keyHolder = keyHolder, - ) + if (rowCount > 0 && colCount > 0) { + LazyRow( + horizontalArrangement = Arrangement.spacedBy(32.dp), + modifier = Modifier + .horizontalScroll(rememberScrollState()) + .width(dimens.maxReaderWidth), + ) { + items( + count = colCount, + ) { colIndex -> + Column( + verticalArrangement = Arrangement.spacedBy(4.dp), + modifier = Modifier, + ) { + for (rowIndex in 0 until rowCount) { + val (section, rowElement) = rowData.getOrNull(rowIndex) ?: break + var emptyCell = false + Surface( + tonalElevation = when (section) { + "thead" -> 3.dp + "tbody" -> 0.dp + "tfoot" -> 1.dp + else -> 0.dp + }, + ) { + rowElement.children() + .filter { it.tagName() in setOf("th", "td") } + .elementAtOrNullWithSpans(colIndex) + ?.let { colElement -> + withParagraph { + withStyle( + if (colElement.tagName() == "th") { + SpanStyle(fontWeight = FontWeight.Bold) + } else { + null + }, + ) { + appendTextChildren( + colElement.childNodes(), + baseUrl = baseUrl, + onLinkClick = onLinkClick, + keyHolder = keyHolder, + ) + } } } - } - emptyCell = !render() - } - if (emptyCell) { - // An empty cell looks better if it has some height - but don't want - // the surface because having one space wide surface is weird - append(' ') - render() + emptyCell = !render() + } + if (emptyCell) { + // An empty cell looks better if it has some height - but don't want + // the surface because having one space wide surface is weird + append(' ') + render() + } } } } diff --git a/app/src/test/java/com/nononsenseapps/feeder/ui/compose/text/HtmlToComposableKtTest.kt b/app/src/test/java/com/nononsenseapps/feeder/ui/compose/text/HtmlToComposableKtTest.kt deleted file mode 100644 index 83e4a66a7d..0000000000 --- a/app/src/test/java/com/nononsenseapps/feeder/ui/compose/text/HtmlToComposableKtTest.kt +++ /dev/null @@ -1,14 +0,0 @@ -package com.nononsenseapps.feeder.ui.compose.text - -import kotlin.test.assertEquals -import org.junit.Test - -class HtmlToComposableKtTest { - @Test - fun htmlIsStrippedFromAlt() { - assertEquals( - "Hello there", - stripHtml("Hello there") - ) - } -} From 2d70bedab30c0e999b738ab9577c1d2ebca42db7 Mon Sep 17 00:00:00 2001 From: Jonas Kalderstam Date: Tue, 14 Nov 2023 15:34:42 +0100 Subject: [PATCH 2/4] Fixed crash when trying to TTS play a missing file --- .../feedarticle/FeedArticleViewModel.kt | 35 +++++++++++++++---- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/com/nononsenseapps/feeder/ui/compose/feedarticle/FeedArticleViewModel.kt b/app/src/main/java/com/nononsenseapps/feeder/ui/compose/feedarticle/FeedArticleViewModel.kt index cc123828f4..c966c91dba 100644 --- a/app/src/main/java/com/nononsenseapps/feeder/ui/compose/feedarticle/FeedArticleViewModel.kt +++ b/app/src/main/java/com/nononsenseapps/feeder/ui/compose/feedarticle/FeedArticleViewModel.kt @@ -38,7 +38,9 @@ import com.nononsenseapps.feeder.ui.compose.feed.FeedListItem import com.nononsenseapps.feeder.ui.compose.feed.FeedOrTag import com.nononsenseapps.feeder.ui.compose.navdrawer.DrawerItemWithUnreadCount import com.nononsenseapps.feeder.ui.compose.text.htmlToAnnotatedString +import com.nononsenseapps.feeder.util.Either import com.nononsenseapps.feeder.util.FilePathProvider +import java.io.FileNotFoundException import java.time.Instant import java.time.ZonedDateTime import java.util.Locale @@ -396,7 +398,14 @@ class FeedArticleViewModel( fun ttsPlay() { viewModelScope.launch(Dispatchers.IO) { val fullText = when (viewState.value.textToDisplay) { - TextToDisplay.DEFAULT -> { + TextToDisplay.DEFAULT -> Either.catching( + onCatch = { + when (it) { + is FileNotFoundException -> TTSFileNotFound + else -> TTSUnknownError + } + }, + ) { blobInputStream(viewState.value.articleId, filePathProvider.articleDir).use { htmlToAnnotatedString( inputStream = it, @@ -405,7 +414,14 @@ class FeedArticleViewModel( } } - TextToDisplay.FULLTEXT -> { + TextToDisplay.FULLTEXT -> Either.catching( + onCatch = { + when (it) { + is FileNotFoundException -> TTSFileNotFound + else -> TTSUnknownError + } + }, + ) { blobFullInputStream( viewState.value.articleId, filePathProvider.fullArticleDir, @@ -422,14 +438,13 @@ class FeedArticleViewModel( TextToDisplay.FAILED_MISSING_BODY, TextToDisplay.FAILED_MISSING_LINK, TextToDisplay.FAILED_NOT_HTML, - -> null + -> Either.Left(TTSUnknownError) } - if (fullText == null) { - // TODO show error some message - } else { + // TODO show error some message + fullText.onRight { ttsStateHolder.tts( - textArray = fullText, + textArray = it, useDetectLanguage = viewState.value.useDetectLanguage, ) } @@ -583,3 +598,9 @@ data class FeedArticleScreenViewState( override val filter: FeedListFilter = emptyFeedListFilter, val isArticleOpen: Boolean = false, ) : FeedScreenViewState, ArticleScreenViewState + +sealed class TSSError + +object TTSFileNotFound : TSSError() + +object TTSUnknownError : TSSError() From 801184bf804aad7bc492e99532b4f7c6d82d45f2 Mon Sep 17 00:00:00 2001 From: Jonas Kalderstam Date: Tue, 14 Nov 2023 15:38:17 +0100 Subject: [PATCH 3/4] Fixed another crash in table rendering --- .../ui/compose/text/HtmlToComposable.kt | 24 ++++++++++++------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/app/src/main/java/com/nononsenseapps/feeder/ui/compose/text/HtmlToComposable.kt b/app/src/main/java/com/nononsenseapps/feeder/ui/compose/text/HtmlToComposable.kt index 49ba162ed1..47081887a5 100644 --- a/app/src/main/java/com/nononsenseapps/feeder/ui/compose/text/HtmlToComposable.kt +++ b/app/src/main/java/com/nononsenseapps/feeder/ui/compose/text/HtmlToComposable.kt @@ -877,18 +877,26 @@ private fun EagerComposer.tableColFirst( ) { val rowCount by remember { derivedStateOf { - element.descendants("tr").count() + try { + element.descendants("tr").count() + } catch (t: Throwable) { + 0 + } } } val colCount by remember { derivedStateOf { - element.descendants("tr") - .map { row -> - row.descendants() - .filter { - it.tagName() in setOf("th", "td") - }.count() - }.maxOrNull() ?: 0 + try { + element.descendants("tr") + .map { row -> + row.descendants() + .filter { + it.tagName() in setOf("th", "td") + }.count() + }.maxOrNull() ?: 0 + } catch (t: Throwable) { + 0 + } } } From 2dd1b6dd27562a63219f8ffdadf506768f4d7685 Mon Sep 17 00:00:00 2001 From: Jonas Kalderstam Date: Tue, 14 Nov 2023 15:43:50 +0100 Subject: [PATCH 4/4] Fixed crash if trying to notify for too many items --- .../feeder/db/room/FeedItemDao.kt | 2 + .../feeder/model/RssNotifications.kt | 38 ++++++++++++------- 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/app/src/main/java/com/nononsenseapps/feeder/db/room/FeedItemDao.kt b/app/src/main/java/com/nononsenseapps/feeder/db/room/FeedItemDao.kt index 9585b196e9..e925cc85b5 100644 --- a/app/src/main/java/com/nononsenseapps/feeder/db/room/FeedItemDao.kt +++ b/app/src/main/java/com/nononsenseapps/feeder/db/room/FeedItemDao.kt @@ -250,6 +250,8 @@ interface FeedItemDao { LEFT JOIN feeds ON feed_items.feed_id = feeds.id WHERE feed_id IN (:feedIds) AND notified IS 0 AND read_time is null AND NOT EXISTS (SELECT 1 FROM blocklist WHERE lower(feed_items.plain_title) GLOB blocklist.glob_pattern) + ORDER BY $feedItemsListOrderByDesc + LIMIT 20 """, ) suspend fun loadItemsToNotify(feedIds: List): List diff --git a/app/src/main/java/com/nononsenseapps/feeder/model/RssNotifications.kt b/app/src/main/java/com/nononsenseapps/feeder/model/RssNotifications.kt index cf5aeaf99a..b9243e9107 100644 --- a/app/src/main/java/com/nononsenseapps/feeder/model/RssNotifications.kt +++ b/app/src/main/java/com/nononsenseapps/feeder/model/RssNotifications.kt @@ -14,7 +14,9 @@ import android.content.pm.PackageManager import android.graphics.BitmapFactory import android.net.Uri import android.os.Build +import android.os.TransactionTooLargeException import android.provider.Browser.EXTRA_CREATE_NEW_TAB +import android.util.Log import androidx.annotation.RequiresApi import androidx.core.app.NotificationCompat import androidx.core.app.NotificationCompat.GROUP_ALERT_SUMMARY @@ -45,8 +47,10 @@ import org.kodein.di.android.closestDI import org.kodein.di.instance const val summaryNotificationId = 2_147_483_646 -private const val channelId = "feederNotifications" -private const val articleNotificationGroup = "com.nononsenseapps.feeder.ARTICLE" +private const val CHANNEL_ID = "feederNotifications" +private const val ARTICLE_NOTIFICATION_GROUP = "com.nononsenseapps.feeder.ARTICLE" + +private const val LOG_TAG = "FEEDER_NOTIFY" suspend fun notify( appContext: Context, @@ -68,18 +72,24 @@ suspend fun notify( val nm: NotificationManagerCompat by di.instance() + // If too many it can cause a crash, so it's limited to 20 val feedItems = getItemsToNotify(di) - if (feedItems.isNotEmpty()) { - if (!updateSummaryOnly) { - feedItems.map { - it.id.toInt() to singleNotification(appContext, it) - }.forEach { (id, notification) -> - nm.notify(id, notification) + try { + if (feedItems.isNotEmpty()) { + if (!updateSummaryOnly) { + feedItems.map { + it.id.toInt() to singleNotification(appContext, it) + }.forEach { (id, notification) -> + nm.notify(id, notification) + } } + // Shown on API Level < 24 + nm.notify(summaryNotificationId, inboxNotification(appContext, feedItems)) } - // Shown on API Level < 24 - nm.notify(summaryNotificationId, inboxNotification(appContext, feedItems)) + } catch (e: TransactionTooLargeException) { + // This can happen if there are too many notifications + Log.e(LOG_TAG, "Too many notifications", e) } } @@ -119,7 +129,7 @@ private fun createNotificationChannel(context: Context) { val notificationManager: NotificationManager = context.getSystemService(NOTIFICATION_SERVICE) as NotificationManager - val channel = NotificationChannel(channelId, name, NotificationManager.IMPORTANCE_LOW) + val channel = NotificationChannel(CHANNEL_ID, name, NotificationManager.IMPORTANCE_LOW) channel.description = description notificationManager.createNotificationChannel(channel) @@ -156,7 +166,7 @@ private suspend fun singleNotification(context: Context, item: FeedItemWithFeed) builder.setContentText(text) .setContentTitle(title) - .setGroup(articleNotificationGroup) + .setGroup(ARTICLE_NOTIFICATION_GROUP) .setGroupAlertBehavior(GROUP_ALERT_SUMMARY) .setDeleteIntent(getPendingDeleteIntent(context, item)) .setNumber(1) @@ -313,7 +323,7 @@ private fun inboxNotification(context: Context, feedItems: List