diff options
Diffstat (limited to 'app/src/main/java/org/terst/feedreader/ui')
5 files changed, 210 insertions, 16 deletions
diff --git a/app/src/main/java/org/terst/feedreader/ui/AddFeedScreen.kt b/app/src/main/java/org/terst/feedreader/ui/AddFeedScreen.kt index 1e8814c..9f817ab 100644 --- a/app/src/main/java/org/terst/feedreader/ui/AddFeedScreen.kt +++ b/app/src/main/java/org/terst/feedreader/ui/AddFeedScreen.kt @@ -33,11 +33,13 @@ import kotlinx.coroutines.launch import kotlinx.coroutines.withContext import okhttp3.Request import org.terst.feedreader.data.AppDatabase +import org.terst.feedreader.data.FeedDiscovery import org.terst.feedreader.data.FeedEntity import org.terst.feedreader.data.FeedParser import org.terst.feedreader.data.Net import org.terst.feedreader.opml.Opml import org.terst.feedreader.work.FeedFetchWorker +import java.io.ByteArrayInputStream @OptIn(ExperimentalMaterial3Api::class) @Composable @@ -168,25 +170,60 @@ fun AddFeedScreen(onDone: () -> Unit) { private fun preferHttps(url: String): String = if (url.startsWith("http://", ignoreCase = true)) "https://" + url.removePrefix("http://") else url +/** + * Fetches [url] and returns its bytes, or null when the server refused. + * + * Read into memory rather than parsed from the live stream because the bytes may need looking + * at twice: once as a feed, and -- when that is not what the URL turned out to be -- again as a + * web page advertising where its feed actually lives. A response body is a one-shot stream, so + * the second look is only possible if the first did not consume it. + */ +private class Fetched(val bytes: ByteArray?, val error: String?) + +private fun fetchBytes(url: String): Fetched = + Net.client.newCall(Request.Builder().url(url).build()).execute().use { response -> + when { + !response.isSuccessful -> Fetched(null, "Couldn't fetch feed: HTTP ${response.code}") + else -> response.body?.let { Fetched(it.bytes(), null) } ?: Fetched(null, "Empty response") + } + } + private suspend fun addFeedByUrl(context: android.content.Context, db: AppDatabase, feedUrl: String): String = withContext(Dispatchers.IO) { - val url = preferHttps(feedUrl) + val typedUrl = preferHttps(feedUrl) try { - val response = Net.client.newCall(Request.Builder().url(url).build()).execute() - response.use { - if (!it.isSuccessful) return@withContext "Couldn't fetch feed: HTTP ${it.code}" - val body = it.body ?: return@withContext "Empty response" - val parsed = FeedParser.parse(body.byteStream()) - // Same REPLACE-wipes-other-fields concern as the OPML import path. - val existing = db.feedDao().getById(url) - val entity = existing?.copy( - title = parsed.title.ifBlank { url }, - siteUrl = parsed.siteUrl ?: existing.siteUrl - ) ?: FeedEntity(id = url, title = parsed.title.ifBlank { url }, siteUrl = parsed.siteUrl) - db.feedDao().upsert(entity) + val fetched = fetchBytes(typedUrl) + val bytes = fetched.bytes ?: return@withContext fetched.error ?: "Empty response" + + // What the user had to hand is usually the site, not the feed -- a site advertises + // its feed in its head, it does not print the URL anywhere a reader can copy. So a + // failed parse is not the answer yet: look for what the page says its feed is and + // try that, once. See FeedDiscovery. + var url = typedUrl + var parsed = runCatching { FeedParser.parse(ByteArrayInputStream(bytes)) }.getOrNull() + if (parsed == null) { + val discovered = FeedDiscovery.findFeedUrl(String(bytes, Charsets.UTF_8), typedUrl) + ?: return@withContext "No feed found at that address" + val feed = fetchBytes(discovered) + val feedBytes = feed.bytes ?: return@withContext feed.error ?: "Empty response" + // One hop only: whatever the page pointed at has to be a feed. Following a + // second page would be a redirect chain nobody asked for. + parsed = FeedParser.parse(ByteArrayInputStream(feedBytes)) + url = discovered } + + // Same REPLACE-wipes-other-fields concern as the OPML import path. + val existing = db.feedDao().getById(url) + val entity = existing?.copy( + title = parsed.title.ifBlank { url }, + siteUrl = parsed.siteUrl ?: existing.siteUrl + ) ?: FeedEntity(id = url, title = parsed.title.ifBlank { url }, siteUrl = parsed.siteUrl) + db.feedDao().upsert(entity) + FeedFetchWorker.fetchNow(context) - "Added" + // Says which feed was added when it is not the URL that was typed, so a site URL + // quietly becoming a different feed URL is visible rather than surprising later. + if (url == typedUrl) "Added" else "Added ${parsed.title.ifBlank { url }}" } catch (e: Exception) { "Couldn't add feed: ${e.message}" } diff --git a/app/src/main/java/org/terst/feedreader/ui/LinkReaderScreen.kt b/app/src/main/java/org/terst/feedreader/ui/LinkReaderScreen.kt index 72f6c1f..9f9f74a 100644 --- a/app/src/main/java/org/terst/feedreader/ui/LinkReaderScreen.kt +++ b/app/src/main/java/org/terst/feedreader/ui/LinkReaderScreen.kt @@ -258,6 +258,7 @@ fun LinkReaderScreen(url: String, onBack: () -> Unit, onOpenLink: (String) -> Un ReaderWebViewCallbacks( onOpenLink = onOpenLink, onLongPressLink = { longPressedLink = it }, + onOpenYouTubeExternally = { context.startActivity(Intent(Intent.ACTION_VIEW, Uri.parse(it))) }, onYouTubeVideo = { youtubeVideoId = it }, onImageTap = { zoomedImageUrl = it }, currentFontSizePx = { latestFontSizePx }, 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 9952dc5..0d326ea 100644 --- a/app/src/main/java/org/terst/feedreader/ui/ReaderScreen.kt +++ b/app/src/main/java/org/terst/feedreader/ui/ReaderScreen.kt @@ -599,6 +599,7 @@ private fun ReaderContent( ReaderWebViewCallbacks( onOpenLink = onOpenLink, onLongPressLink = { longPressedLink = it }, + onOpenYouTubeExternally = { context.startActivity(Intent(Intent.ACTION_VIEW, Uri.parse(it))) }, onYouTubeVideo = { youtubeVideoId = it }, onImageTap = { zoomedImageUrl = it }, onScrollChanged = { scrollY -> ReaderScrollPositions.set(articleId, scrollY) }, diff --git a/app/src/main/java/org/terst/feedreader/ui/ReaderWebView.kt b/app/src/main/java/org/terst/feedreader/ui/ReaderWebView.kt index f1400d3..78020a9 100644 --- a/app/src/main/java/org/terst/feedreader/ui/ReaderWebView.kt +++ b/app/src/main/java/org/terst/feedreader/ui/ReaderWebView.kt @@ -74,6 +74,11 @@ private fun extractYouTubeVideoId(uri: Uri): String? { * WebView's own default long-press behavior (a native "open/copy link" popup) -- * callers show their own save/share/open action menu instead. * + * onOpenYouTubeExternally fires for a LONG-press on a YouTube link, with the watch URL, instead + * of the ordinary onLongPressLink actions menu -- callers hand it to the system, which is the + * YouTube app on a phone that has one. A tap plays the video in the reader; a long-press is how + * you say you wanted it somewhere else. + * * onYouTubeVideo fires for a tap on a YouTube watch link (what ReaderExtractor turns * an embedded video into a thumbnail link to) with just the video ID, instead of the * old behavior of handing off to the YouTube app/browser -- callers show an in-app @@ -124,6 +129,7 @@ private fun extractYouTubeVideoId(uri: Uri): String? { data class ReaderWebViewCallbacks( val onOpenLink: (String) -> Unit, val onLongPressLink: (String) -> Unit = {}, + val onOpenYouTubeExternally: (String) -> Unit = {}, val onYouTubeVideo: (String) -> Unit = {}, val onImageTap: (String) -> Unit = {}, val onScrollChanged: (Int) -> Unit = {}, @@ -245,7 +251,19 @@ fun newReaderWebView(context: Context, callbacks: ReaderWebViewCallbacks): Reade // Every article image now sits inside a zoom link, so a long-press on one // reports that link's internal scheme -- strip it back to the image's own URL // so the actions menu shows, and acts on, something real. - callbacks.onLongPressLink(withoutImageZoomScheme(url)) + val target = withoutImageZoomScheme(url) + // A long-press on a video reports the thumbnail's URL rather than the link's -- + // see youTubeTargetForLongPress, which resolves it back to the video. Straight + // out to whatever handles YouTube on the phone, usually the app: a tap already + // plays the video in the reader, so the only thing a long-press can usefully + // mean is "not in here, out there", and picking that off a menu would be a menu + // with one useful item. + val video = youTubeTargetForLongPress(target) + if (video != null) { + callbacks.onOpenYouTubeExternally(video) + } else { + callbacks.onLongPressLink(target) + } true } else { false @@ -380,7 +398,17 @@ fun YouTubePlayerDialog(videoId: String, onDismiss: () -> Unit) { } } webChromeClient = WebChromeClient() - loadUrl("https://www.youtube-nocookie.com/embed/$videoId?autoplay=1&playsinline=1") + // A host page with the embed in an iframe, not the embed itself as the + // top-level document. YouTube answers a top-level embed with "Video player + // configuration error -- Error 153", which is the reported "YouTube play is + // misconfigured"; framed, the same URL plays. See YouTubeLinks. + loadDataWithBaseURL( + YOUTUBE_EMBED_BASE_URL, + youTubeEmbedPageHtml(videoId), + "text/html", + "utf-8", + null + ) } }, onRelease = { it.destroy() } diff --git a/app/src/main/java/org/terst/feedreader/ui/YouTubeLinks.kt b/app/src/main/java/org/terst/feedreader/ui/YouTubeLinks.kt new file mode 100644 index 0000000..e2b3b62 --- /dev/null +++ b/app/src/main/java/org/terst/feedreader/ui/YouTubeLinks.kt @@ -0,0 +1,127 @@ +package org.terst.feedreader.ui + +/** + * Recognising a YouTube placeholder link, and building a page its embed will actually play in. + * + * Split out of ReaderWebView for the usual reason: this is string work with no WebView, no + * gestures and no Compose in it, so it can be exercised without a device -- which matters here + * because the bug it fixes is invisible until a video is tapped. + * + * **Why the embed needs a page at all.** The player dialog used to call + * `loadUrl("https://www.youtube-nocookie.com/embed/<id>")`, loading the embed as the WebView's + * own top-level document. YouTube refuses that: the player comes up showing "Video player + * configuration error -- Error 153", which is reported as "YouTube play is misconfigured". + * Confirmed by loading the same URL three ways in a real browser on an Android user agent: + * + * | how the embed is loaded | result | + * |------------------------------------------|-----------------------------| + * | top-level, as the app did | Error 153 | + * | top-level with `&origin=` | Error 153 | + * | in an `<iframe>` on an ordinary https page | plays | + * + * An embed is meant to be embedded. Given a host page it behaves exactly as it does on any + * site that includes a video, so that is what [youTubeEmbedPageHtml] provides. + */ + +/** Video id characters, per YouTube: 11 of these and nothing else. */ +private val VIDEO_ID_CHARS = Regex("[^A-Za-z0-9_-]") + +/** Hosts whose links are YouTube's. Matched whole, so `notyoutube.com` is not one of them. */ +private val YOUTUBE_HOSTS = setOf("youtube.com", "www.youtube.com", "m.youtube.com", "youtu.be", "www.youtu.be") + +/** Thumbnail hosts. YouTube serves these from i.ytimg.com and numbered variants of it. */ +private val THUMBNAIL_HOST = Regex("^i[0-9]*\\.ytimg\\.com$") + +/** `/vi/<id>/...` or `/vi_webp/<id>/...` -- the id is the first segment after the vi folder. */ +private val THUMBNAIL_PATH = Regex("^/vi(?:_webp)?/([A-Za-z0-9_-]{11})/") + +/** `?v=<id>` anywhere in the query. */ +private val WATCH_QUERY = Regex("[?&]v=([A-Za-z0-9_-]{11})(?:&|$)") + +/** `/<id>` as the whole path, which is what youtu.be links are. */ +private val SHORT_PATH = Regex("^/([A-Za-z0-9_-]{11})/?$") + +/** + * The origin the embed is hosted on. + * + * Deliberately not a youtube.com domain: the embed checks that it is framed by a page other + * than its own, which is the condition the top-level load failed. Nothing is ever fetched from + * this address -- it is the base URL for `loadDataWithBaseURL`, so it only has to be a + * well-formed https origin for the document to have one. + */ +const val YOUTUBE_EMBED_BASE_URL = "https://reader.feedreader.invalid/" + +/** + * The video a long-press reported, as a watch URL, or null when the long-press was not on one. + * + * **What a long-press actually reports.** Not the link's href. The reader's video placeholder is + * `<a href="https://www.youtube.com/watch?v=ID"><img src="https://i.ytimg.com/vi/ID/hq720.jpg"></a>`, + * and for an image inside a link WebView's hit test gives SRC_IMAGE_ANCHOR_TYPE with the + * *image's* URL in `getExtra()`, not the anchor's. Confirmed on the device: long-pressing the + * thumbnail opened the ordinary link menu showing the i.ytimg.com JPEG. So the thumbnail URL is + * the shape that has to be recognised, and it carries the video id in its path. + * + * Watch and youtu.be links are recognised too, for wherever one does arrive -- a plain text link + * to a video in an article, which is not wrapped in an image. + * + * Anything on YouTube that is not one specific video -- a channel, a playlist -- returns null and + * gets the ordinary actions menu, since "open this directly" has no single video to mean. + */ +fun youTubeTargetForLongPress(url: String): String? { + val host = hostOf(url) ?: return null + val path = pathOf(url) + val id = when { + THUMBNAIL_HOST.matches(host) -> THUMBNAIL_PATH.find(path)?.groupValues?.get(1) + host !in YOUTUBE_HOSTS -> null + host.endsWith("youtu.be") -> SHORT_PATH.find(path)?.groupValues?.get(1) + else -> WATCH_QUERY.find(url)?.groupValues?.get(1) + } + return id?.let { youTubeWatchUrl(it) } +} + +/** Where a video lives, in the form a phone hands to the YouTube app. */ +fun youTubeWatchUrl(videoId: String): String = "https://www.youtube.com/watch?v=${scrub(videoId)}" + +/** + * A minimal page holding nothing but the player, filling it. + * + * `allow="autoplay"` is required for the autoplay in the src to be honoured inside an iframe -- + * without it the embed loads and then sits there, which would look like a different bug with + * the same symptom. + */ +fun youTubeEmbedPageHtml(videoId: String): String { + val id = scrub(videoId) + return """<!doctype html><html><head><meta name="viewport" content="width=device-width,initial-scale=1">""" + + """<style>html,body{margin:0;padding:0;height:100%;background:#000;overflow:hidden}""" + + """iframe{display:block;border:0;width:100%;height:100%}</style></head><body>""" + + """<iframe src="https://www.youtube-nocookie.com/embed/$id?autoplay=1&playsinline=1&rel=0" """ + + """allow="autoplay; encrypted-media; picture-in-picture; fullscreen" allowfullscreen></iframe>""" + + """</body></html>""" +} + +/** + * The id with anything outside YouTube's own alphabet removed. + * + * The ids reaching here are already matched against `[a-zA-Z0-9_-]{11}` upstream, so this + * changes nothing today. It exists because the value is spliced straight into an HTML + * attribute, and that should be safe on its own terms rather than because of a check in a + * different file that a later edit might move. + */ +private fun scrub(videoId: String): String = VIDEO_ID_CHARS.replace(videoId, "") + +/** The path of [url], or "/" when it has none. */ +private fun pathOf(url: String): String { + val afterScheme = url.substringAfter("://", missingDelimiterValue = "") + val afterAuthority = afterScheme.substringAfter('/', missingDelimiterValue = "") + val path = "/" + afterAuthority.substringBefore('?').substringBefore('#') + return path +} + +/** The host of [url], lowercased, or null when it is not a URL with one. */ +private fun hostOf(url: String): String? { + val afterScheme = url.substringAfter("://", missingDelimiterValue = "") + if (afterScheme.isEmpty()) return null + val authority = afterScheme.substringBefore('/').substringBefore('?').substringBefore('#') + val host = authority.substringAfterLast('@').substringBefore(':') + return host.lowercase().ifEmpty { null } +} |