diff options
Diffstat (limited to 'app')
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() |