summaryrefslogtreecommitdiff
path: root/app/src/main/java
diff options
context:
space:
mode:
authorPeter Stone <thepeterstone@gmail.com>2026-07-23 01:59:14 +0000
committerPeter Stone <thepeterstone@gmail.com>2026-07-23 01:59:14 +0000
commit3914ba17608884a6fa8614b9058752e0c1c0d94f (patch)
tree2525f44cf04116416935d82cb43e748267b18679 /app/src/main/java
parentd7c05609692153670dc136082da4e0566d7838a6 (diff)
Fix crash on blank-guid articles; simplify browser back; backlog notes
- FeedParser: a feed with a literal empty <guid>/<id> element (confirmed live: a Mother Jones entry) produced an ArticleEntity with id = "" -- the fallback chain only checked for null, not blank, so an empty-but- present tag passed straight through. An empty-string primary key broke Navigation Compose's route matching and crashed on tap ("tapping this article crashes the app"). Fixed both RSS and Atom paths to treat blank the same as null before falling back to a link-based id. Schema v5 migration deletes any row already on-device with the bad id; it'll come back with a real, hash-based id on the next fetch of that feed. Verified live: zero blank-id rows after the migration and a fresh fetch, no crash. - BrowserScreen: back (both the system gesture and the in-app arrow) now always closes the screen directly, rather than stepping through in-page WebView history first. Reported live as not making sense for multi-page browsing (search results -> article -> another link, etc.) -- you'd have to tap back that many times just to leave. There's no separate in-page-back affordance now, matching how every other screen in this app behaves. - Tried moving swipe-to-next-article detection off Compose's gesture system entirely (a passive native OnTouchListener on the WebView, never consuming) on the theory that Compose's gesture arbitration was the source of reported scroll jerkiness. Reverted after confirming live that swipe stopped working at all with that approach -- a worse regression than unproven jerkiness. Back to the known-working Compose-based detector; the jerkiness complaint itself is still open. - BACKLOG.md: added bug-report-button, smaller-title-text, and bottom-action-strip-with-auto-hide notes.
Diffstat (limited to 'app/src/main/java')
-rw-r--r--app/src/main/java/org/terst/feedreader/data/AppDatabase.kt17
-rw-r--r--app/src/main/java/org/terst/feedreader/data/FeedParser.kt10
-rw-r--r--app/src/main/java/org/terst/feedreader/ui/BrowserScreen.kt18
-rw-r--r--app/src/main/java/org/terst/feedreader/ui/ReaderScreen.kt21
4 files changed, 39 insertions, 27 deletions
diff --git a/app/src/main/java/org/terst/feedreader/data/AppDatabase.kt b/app/src/main/java/org/terst/feedreader/data/AppDatabase.kt
index 441556b..66722fd 100644
--- a/app/src/main/java/org/terst/feedreader/data/AppDatabase.kt
+++ b/app/src/main/java/org/terst/feedreader/data/AppDatabase.kt
@@ -9,7 +9,7 @@ import androidx.room.migration.Migration
@Database(
entities = [FeedEntity::class, ArticleEntity::class, FilterRuleEntity::class],
- version = 4,
+ version = 5,
exportSchema = false
)
abstract class AppDatabase : RoomDatabase() {
@@ -77,12 +77,25 @@ abstract class AppDatabase : RoomDatabase() {
}
}
+ // v5: a feed with a literal empty <guid>/<id> element (confirmed live: a Mother
+ // Jones entry) produced an ArticleEntity with id = "" -- FeedParser fell back to
+ // itemLink only when the guid was null, not when it was blank. An empty-string
+ // primary key broke Navigation Compose's route matching and crashed on tap. The
+ // parser is fixed for future fetches; this cleans out any row already on-device
+ // with the bad id so it doesn't keep crashing. It'll come back with a real,
+ // hash-based id on the next fetch of that feed.
+ private val MIGRATION_4_5 = object : Migration(4, 5) {
+ override fun migrate(db: SupportSQLiteDatabase) {
+ db.execSQL("DELETE FROM articles WHERE trim(id) = ''")
+ }
+ }
+
fun get(context: Context): AppDatabase = instance ?: synchronized(this) {
instance ?: Room.databaseBuilder(
context.applicationContext,
AppDatabase::class.java,
"feedreader.db"
- ).addMigrations(MIGRATION_1_2, MIGRATION_2_3, MIGRATION_3_4).build().also { instance = it }
+ ).addMigrations(MIGRATION_1_2, MIGRATION_2_3, MIGRATION_3_4, MIGRATION_4_5).build().also { instance = it }
}
}
}
diff --git a/app/src/main/java/org/terst/feedreader/data/FeedParser.kt b/app/src/main/java/org/terst/feedreader/data/FeedParser.kt
index 629afe6..8b37e55 100644
--- a/app/src/main/java/org/terst/feedreader/data/FeedParser.kt
+++ b/app/src/main/java/org/terst/feedreader/data/FeedParser.kt
@@ -120,7 +120,12 @@ object FeedParser {
inItem = false
val content = itemContent ?: itemDescription
items += ParsedItem(
- guid = itemGuid ?: itemLink.ifBlank { null },
+ // .ifBlank, not just ?: -- a feed with a literal empty <guid></guid>
+ // (confirmed live: a Mother Jones entry) gives itemGuid = "" here,
+ // which is non-null and so passed straight through a plain `?:`,
+ // landing as the article's primary key. An empty-string id broke
+ // Navigation Compose's route matching and crashed on tap.
+ guid = itemGuid?.ifBlank { null } ?: itemLink.ifBlank { null },
title = itemTitle,
link = itemLink,
author = itemAuthor,
@@ -195,7 +200,8 @@ object FeedParser {
inEntry = false
val content = entryContent ?: entrySummary
items += ParsedItem(
- guid = entryId ?: entryLink.ifBlank { null },
+ // Same blank-vs-null fix as the RSS path above.
+ guid = entryId?.ifBlank { null } ?: entryLink.ifBlank { null },
title = entryTitle,
link = entryLink,
author = entryAuthor,
diff --git a/app/src/main/java/org/terst/feedreader/ui/BrowserScreen.kt b/app/src/main/java/org/terst/feedreader/ui/BrowserScreen.kt
index 6202016..6eb5b71 100644
--- a/app/src/main/java/org/terst/feedreader/ui/BrowserScreen.kt
+++ b/app/src/main/java/org/terst/feedreader/ui/BrowserScreen.kt
@@ -50,21 +50,21 @@ fun BrowserScreen(startUrl: String, onBack: () -> Unit) {
var title by remember(startUrl) { mutableStateOf<String?>(null) }
var webViewRef by remember(startUrl) { mutableStateOf<WebView?>(null) }
- BackHandler {
- val wv = webViewRef
- if (wv != null && wv.canGoBack()) wv.goBack() else onBack()
- }
+ // Deliberately NOT "goBack() if the WebView has history, else exit" -- that means
+ // however many pages deep a search/login session went (search results -> article ->
+ // another link -> ...), leaving requires tapping back that many times first. Reported
+ // live as not making sense for this kind of multi-page browsing. Back always just
+ // closes the screen now, both the system gesture and the in-app arrow; there's no
+ // separate in-page-back affordance, matching how every other screen in this app behaves.
+ BackHandler(onBack = onBack)
Scaffold(
topBar = {
TopAppBar(
title = { Text(title ?: "Browser", maxLines = 1, overflow = TextOverflow.Ellipsis) },
navigationIcon = {
- IconButton(onClick = {
- val wv = webViewRef
- if (wv != null && wv.canGoBack()) wv.goBack() else onBack()
- }) {
- Icon(Icons.Default.ArrowBack, contentDescription = "Back")
+ IconButton(onClick = onBack) {
+ Icon(Icons.Default.ArrowBack, contentDescription = "Close browser")
}
},
actions = {
diff --git a/app/src/main/java/org/terst/feedreader/ui/ReaderScreen.kt b/app/src/main/java/org/terst/feedreader/ui/ReaderScreen.kt
index 806926c..82cb792 100644
--- a/app/src/main/java/org/terst/feedreader/ui/ReaderScreen.kt
+++ b/app/src/main/java/org/terst/feedreader/ui/ReaderScreen.kt
@@ -105,20 +105,13 @@ fun ReaderScreen(
val scope = rememberCoroutineScope()
val swipeThresholdPx = with(LocalDensity.current) { 80.dp.toPx() }
- // HorizontalPager's own full-surface drag detection was grabbing ordinary vertical
- // scrolling inside the embedded WebView (a native View, which doesn't arbitrate with
- // Compose's gesture system the way a pure-Compose scrollable child would) as an
- // attempted horizontal page-turn on anything less than a perfectly vertical swipe.
- // detectHorizontalDragGestures has its own orientation-aware touch-slop detection, so
- // it already declines to engage for vertical-dominant movement -- confirmed live: a
- // diagonal mid-screen scroll stays vertical, doesn't page-turn. (A generic,
- // non-oriented detectDragGestures was tried for a stricter dx/dy ratio check and broke
- // scrolling entirely: its slop detector claims the first bit of movement in ANY
- // direction isotropically, before it knows which way the gesture is really going --
- // reported live as "the article view doesn't scroll at all anymore.") Also: page-turn
- // commit is based on total drag distance, not where the gesture started, since an
- // edge-anchored version collided with Android's own edge-swipe-back gesture (confirmed
- // live: an edge swipe triggered system back, not the pager).
+ // Tried moving swipe detection to a passive native OnTouchListener on the WebView
+ // (always returns false, never consumes) to get Compose's gesture arbitration
+ // completely out of the touch path, on the theory that it was the source of reported
+ // scroll jerkiness. Reverted: swipe-to-next-article stopped working at all with that
+ // approach (confirmed live), which is a worse regression than unproven jerkiness.
+ // Back to the known-working Compose detector -- orientation-aware touch-slop
+ // detection, so it doesn't grab vertical-dominant drags, confirmed live repeatedly.
Box(
modifier = Modifier
.fillMaxSize()