Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crashfix #121

Merged
merged 4 commits into from
Nov 14, 2023
Merged

Crashfix #121

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3ac703a
Fixed crash when table had no columns
spacecowboy Nov 14, 2023
2d70bed
Fixed crash when trying to TTS play a missing file
spacecowboy Nov 14, 2023
801184b
Fixed another crash in table rendering
spacecowboy Nov 14, 2023
2dd1b6d
Fixed crash if trying to notify for too many items
spacecowboy Nov 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions app/src/main/java/com/nononsenseapps/feeder/db/room/FeedItemDao.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<Long>): List<FeedItemWithFeed>
Expand Down
38 changes: 24 additions & 14 deletions app/src/main/java/com/nononsenseapps/feeder/model/RssNotifications.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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)
}
}

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -313,7 +323,7 @@ private fun inboxNotification(context: Context, feedItems: List<FeedItemWithFeed
builder.setContentText(text)
.setContentTitle(title)
.setContentIntent(pendingIntent)
.setGroup(articleNotificationGroup)
.setGroup(ARTICLE_NOTIFICATION_GROUP)
.setGroupAlertBehavior(GROUP_ALERT_SUMMARY)
.setGroupSummary(true)
.setDeleteIntent(getDeleteIntent(context, feedItems))
Expand Down Expand Up @@ -359,7 +369,7 @@ private fun getPendingDeleteIntent(context: Context, feedItem: FeedItemWithFeed)
private fun notificationBuilder(context: Context): NotificationCompat.Builder {
val bm = BitmapFactory.decodeResource(context.resources, R.mipmap.ic_launcher)

return NotificationCompat.Builder(context, channelId)
return NotificationCompat.Builder(context, CHANNEL_ID)
.setSmallIcon(R.drawable.ic_stat_f)
.setLargeIcon(bm)
.setAutoCancel(true)
Expand Down
6 changes: 2 additions & 4 deletions app/src/main/java/com/nononsenseapps/feeder/ui/compose/editfeed/EditFeedScreen.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 = {
Expand Down
5 changes: 2 additions & 3 deletions app/src/main/java/com/nononsenseapps/feeder/ui/compose/feedarticle/ArticleScreen.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
35 changes: 28 additions & 7 deletions app/src/main/java/com/nononsenseapps/feeder/ui/compose/feedarticle/FeedArticleViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
)
}
Expand Down Expand Up @@ -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()
126 changes: 68 additions & 58 deletions app/src/main/java/com/nononsenseapps/feeder/ui/compose/text/HtmlToComposable.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}.max()
try {
element.descendants("tr")
.map { row ->
row.descendants()
.filter {
it.tagName() in setOf("th", "td")
}.count()
}.maxOrNull() ?: 0
} catch (t: Throwable) {
0
}
}
}

Expand Down Expand Up @@ -952,58 +960,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()
}
}
}
}
Expand Down
14 changes: 0 additions & 14 deletions app/src/test/java/com/nononsenseapps/feeder/ui/compose/text/HtmlToComposableKtTest.kt
View file
Open in desktop

This file was deleted.