From 9210bf463a8818595f7ca42486a787d4f1941d55 Mon Sep 17 00:00:00 2001 From: Peter Stone Date: Wed, 21 Oct 2026 19:07:17 +0000 Subject: Fix stray promo headers, site-URL subscribing, and YouTube playback Three reported bugs. **"West Hawaii today articles get a bunch of stray Trending headers."** The publisher's widget platform injects promo blocks between the real paragraphs, inside the article's own container, so every structural signal tells Readability they belong. What survives is a heading labelling nothing, because the links under it are filled in by a script that never runs in the reader. Confirmed on 9 of the 12 articles in that feed, two each. Blocks marked `data-mrf-recirculation` are now removed -- the attribute says what the block is for in the widget platform's own words, which is a better signal than the word "Trending", since an article may legitimately discuss what is trending. Nothing longer than a label and a few headlines is ever removed. Also applied to already-cached articles, in place rather than by discarding them: the article was fine, only the litter has to go. **"Can't subscribe to heathercoxrichardson.substack.com."** Adding a feed handed whatever URL it was given straight to the XML parser, so a site's home page -- the only URL a reader can actually see and copy -- failed on the root element. That page advertises its feed the way sites have since 2002, in a in the head. It is now read, resolved against the page, and fetched. Only what the page declares: guessing /feed and /rss would add a burst of 404s to every failed add and eventually subscribe somebody to the wrong thing. **"YouTube play is misconfigured."** It was, and YouTube said so: the player dialog loaded the embed as its own top-level document, which answers with "Video player configuration error -- Error 153". An embed has to be embedded. It now sits in an iframe on a minimal host page. Measured in the app's own WebView: top-level, no requests for the video stream and the error on screen; framed, the stream loads and there is no error. **"Long click on YouTube placeholder should open the URL directly."** It does now. Worth recording why this is not simply the link's href: for an image inside a link, WebView's hit test reports the image's URL, so a long-press on the thumbnail arrives as the i.ytimg.com JPEG. The video id is resolved out of that. Verified on a device for all four: the Substack site URL adds "Letters from an American" and fetches 20 articles; a West Hawaii Today article extracts with no Trending headers and its body intact; the video plays; and a long-press reaches the YouTube app. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_018AwnALZL6bD4VBtATDVv3B --- .../org/terst/feedreader/data/CachedExtraction.kt | 10 +- .../org/terst/feedreader/data/FeedDiscovery.kt | 88 +++++++++++++ .../terst/feedreader/readability/EvalDecision.kt | 9 +- .../feedreader/readability/RecirculationWidgets.kt | 128 +++++++++++++++++++ .../java/org/terst/feedreader/ui/AddFeedScreen.kt | 65 +++++++--- .../org/terst/feedreader/ui/LinkReaderScreen.kt | 1 + .../java/org/terst/feedreader/ui/ReaderScreen.kt | 1 + .../java/org/terst/feedreader/ui/ReaderWebView.kt | 32 ++++- .../java/org/terst/feedreader/ui/YouTubeLinks.kt | 127 +++++++++++++++++++ .../terst/feedreader/data/CachedExtractionTest.kt | 19 +++ .../org/terst/feedreader/data/FeedDiscoveryTest.kt | 93 ++++++++++++++ .../feedreader/readability/EvalOutcomeTest.kt | 18 +++ .../readability/RecirculationWidgetsTest.kt | 117 +++++++++++++++++ .../org/terst/feedreader/ui/YouTubeLinksTest.kt | 139 +++++++++++++++++++++ .../resources/west-hawaii-today-extracted.html | 24 ++++ 15 files changed, 853 insertions(+), 18 deletions(-) create mode 100644 app/src/main/java/org/terst/feedreader/data/FeedDiscovery.kt create mode 100644 app/src/main/java/org/terst/feedreader/readability/RecirculationWidgets.kt create mode 100644 app/src/main/java/org/terst/feedreader/ui/YouTubeLinks.kt create mode 100644 app/src/test/java/org/terst/feedreader/data/FeedDiscoveryTest.kt create mode 100644 app/src/test/java/org/terst/feedreader/readability/RecirculationWidgetsTest.kt create mode 100644 app/src/test/java/org/terst/feedreader/ui/YouTubeLinksTest.kt create mode 100644 app/src/test/resources/west-hawaii-today-extracted.html (limited to 'app') diff --git a/app/src/main/java/org/terst/feedreader/data/CachedExtraction.kt b/app/src/main/java/org/terst/feedreader/data/CachedExtraction.kt index 3a791cc..63614d1 100644 --- a/app/src/main/java/org/terst/feedreader/data/CachedExtraction.kt +++ b/app/src/main/java/org/terst/feedreader/data/CachedExtraction.kt @@ -1,6 +1,7 @@ package org.terst.feedreader.data import org.terst.feedreader.readability.ConsentNotice +import org.terst.feedreader.readability.RecirculationWidgets import org.terst.feedreader.readability.UnrenderedTex /** What a cached `articles.extractedContent` value turned out to be worth. */ @@ -58,7 +59,14 @@ suspend fun loadCachedExtraction( discardedUnrenderedTex = true ) else -> null - } ?: return CachedExtraction(content = cached, discardedConsentNotice = false) + } ?: return CachedExtraction( + // Recirculation widgets are deliberately not in the `discard` list above. A cached + // article carrying the publisher's promo blocks is a good article with litter in it, + // not a poisoned row: cleaning it in place shows the right thing immediately, where + // discarding it would throw away a usable body and fetch the identical one back. + content = RecirculationWidgets.strip(cached), + discardedConsentNotice = false + ) clearCache() return discard diff --git a/app/src/main/java/org/terst/feedreader/data/FeedDiscovery.kt b/app/src/main/java/org/terst/feedreader/data/FeedDiscovery.kt new file mode 100644 index 0000000..98ce5b3 --- /dev/null +++ b/app/src/main/java/org/terst/feedreader/data/FeedDiscovery.kt @@ -0,0 +1,88 @@ +package org.terst.feedreader.data + +import java.net.URI + +/** + * Finds the feed a web page advertises. + * + * Reported live: "Can't subscribe to https://heathercoxrichardson.substack.com/". Adding a feed + * fetched whatever URL it was given and handed it straight to [FeedParser], so a site's home + * page -- the only URL a reader can actually see and copy -- failed on the XML parse with a + * message about an unrecognised root element. + * + * Nothing exotic is needed to fix it. Sites have advertised their feeds in the document head + * since RSS autodiscovery was agreed in 2002, and that Substack page does: + * + * ``` + * + * ``` + * + * Only what the page itself declares is used. Guessing paths (`/feed`, `/rss`, `/index.xml`) + * would add a burst of 404s on every failed add and would eventually subscribe somebody to the + * wrong thing; a site that declares nothing is reported as having no feed, which is the truth. + */ +object FeedDiscovery { + + /** Feed content types, as `type=` gives them. */ + private val FEED_TYPES = listOf("application/rss+xml", "application/atom+xml", "application/rdf+xml", "text/xml") + + private val LINK_TAG = Regex("]*>", RegexOption.IGNORE_CASE) + + /** `name="value"`, `name='value'` or `name=value`, in any order within the tag. */ + private val ATTRIBUTE = + Regex("""(?[a-zA-Z-]+)\s*=\s*("(?[^"]*)"|'(?[^']*)'|(?[^\s">]+))""") + + /** + * The feed URL [html] advertises, absolute, or null when it advertises none. + * + * [pageUrl] is the address the HTML came from, which a relative href resolves against -- + * against the page, not the site root, since a feed link on `/blog/` routinely says + * `feed.xml` and means `/blog/feed.xml`. + */ + fun findFeedUrl(html: String, pageUrl: String): String? { + // Already a feed: there is nothing to discover, and scanning XML for tags would + // only find Atom's own self-links and send the caller round in a circle. + if (html.trimStart().startsWith(" + attrs["rel"]?.contains("alternate", ignoreCase = true) == true && + attrs["type"]?.lowercase()?.trim() in FEED_TYPES + } + ?.get("href") + ?.takeIf { it.isNotBlank() } + ?.let { resolve(it, pageUrl) } + } + + private fun attributesOf(tag: String): Map = + ATTRIBUTE.findAll(tag).associate { match -> + // Named rather than numbered: which of the three quoting alternatives matched is + // the whole question here, and "groupValues[4]" does not say that to a reader. + val value = match.groups["dq"]?.value + ?: match.groups["sq"]?.value + ?: match.groups["bare"]?.value.orEmpty() + match.groups["name"]!!.value.lowercase() to value + } + + /** + * [href] as an absolute https URL. + * + * The https upgrade is the same rule the add-feed screen already applies to what the user + * types, for the same reason: Android blocks cleartext, so an http feed would fail every + * fetch forever. A site that advertises an http feed almost always serves it over https too. + */ + private fun resolve(href: String, pageUrl: String): String? = try { + val absolute = URI(pageUrl).resolve(href.trim()).toString() + if (absolute.startsWith("http://", ignoreCase = true)) { + "https://" + absolute.removePrefix("http://").removePrefix("HTTP://") + } else { + absolute + } + } catch (ignored: IllegalArgumentException) { + // A malformed href is a page problem, not a crash: report no feed and let the caller + // say so, the same as a page that advertises none. Nothing is lost by not reporting + // it -- "no feed found at that address" is exactly as true and as useful either way. + null + } +} diff --git a/app/src/main/java/org/terst/feedreader/readability/EvalDecision.kt b/app/src/main/java/org/terst/feedreader/readability/EvalDecision.kt index ed0033d..dd77014 100644 --- a/app/src/main/java/org/terst/feedreader/readability/EvalDecision.kt +++ b/app/src/main/java/org/terst/feedreader/readability/EvalDecision.kt @@ -39,7 +39,14 @@ internal fun decideEvalOutcome( !isConsentNotice return when { - usable -> EvalDecision.Done(ExtractionResult.Success(article!!)) + // Cleaned on the way out rather than at the point of display: this is the one place + // every fresh extraction passes through, so the widgets never reach the screen and + // never reach `articles.extractedContent` either. + usable -> EvalDecision.Done( + ExtractionResult.Success( + article!!.copy(content = RecirculationWidgets.strip(article.content)) + ) + ) // Readability ran and objected: that verdict will not change. readabilityError != null -> diff --git a/app/src/main/java/org/terst/feedreader/readability/RecirculationWidgets.kt b/app/src/main/java/org/terst/feedreader/readability/RecirculationWidgets.kt new file mode 100644 index 0000000..a6515c1 --- /dev/null +++ b/app/src/main/java/org/terst/feedreader/readability/RecirculationWidgets.kt @@ -0,0 +1,128 @@ +package org.terst.feedreader.readability + +/** + * Removes "recirculation" widgets -- the promo blocks a publisher's widget platform injects + * into the middle of the article body to send the reader off to other stories. + * + * Reported live: West Hawaii Today articles "get a bunch of stray 'Trending' headers". + * Confirmed by running the vendored Readability.js over the rendered page for the 12 articles + * in that feed: 9 carried two surviving widgets each. The three that didn't were paywalled + * stubs too short for the widgets to be reached. + * + * Readability keeps them because they are not in a sidebar -- they are inside the article's own + * container, between real paragraphs, so every structural signal says they belong. What arrives + * is a heading labelling nothing, because the list of links under it is filled in later by a + * script that never runs in the reader: + * + * ``` + *

Trending

+ * ``` + * + * The signal is the attribute, not the word "Trending": `data-mrf-recirculation` says what the + * block is for, in the widget platform's own vocabulary, and no article body is ever inside one. + * Matching on the heading text instead would be both narrower (the label varies by site and + * section) and more dangerous (an article may legitimately discuss what is trending). + * + * [MAX_WIDGET_TEXT_LENGTH] is the same safety valve [ConsentNotice] uses: however the attribute + * is applied, nothing with real writing in it is ever thrown away. + */ +object RecirculationWidgets { + + /** + * Text length past which a block is kept whatever attribute it carries. A widget is a label + * and a few headlines; anything longer is somebody's writing and is not ours to delete. + */ + private const val MAX_WIDGET_TEXT_LENGTH = 600 + + /** + * Attributes marking a block as recirculation. Only ones whose own name says so -- this list + * grows on evidence from a real article, never on a guess about what a vendor prefix means. + */ + private val WIDGET_ATTRIBUTES = listOf("data-mrf-recirculation") + + private val OPENING_DIV = Regex("]*>", RegexOption.IGNORE_CASE) + private val TAG_REGEX = Regex("<[^>]*>") + private val WHITESPACE_REGEX = Regex("\\s+") + + private const val OPEN_DIV = "` closing the div that opened before [from], or null when + * the markup never closes it. + */ + private fun matchingClose(html: String, from: Int): Int? { + var depth = 1 + var i = from + var end: Int? = null + while (end == null && i < html.length) { + val next = html.indexOf('<', i) + when { + next < 0 -> i = html.length + html.startsWith(CLOSE_DIV, next, ignoreCase = true) -> { + val close = html.indexOf('>', next) + depth-- + when { + close < 0 -> i = html.length + depth == 0 -> end = close + 1 + else -> i = close + 1 + } + } + html.startsWith(OPEN_DIV, next, ignoreCase = true) -> { + depth++ + i = next + OPEN_DIV.length + } + else -> i = next + 1 + } + } + return end + } + + private fun tooLongToBeAWidget(block: String): Boolean = + block.replace(TAG_REGEX, " ").replace(WHITESPACE_REGEX, " ").trim().length > MAX_WIDGET_TEXT_LENGTH +} 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/")`, 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 `""" + + """""" +} + +/** + * 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 } +} diff --git a/app/src/test/java/org/terst/feedreader/data/CachedExtractionTest.kt b/app/src/test/java/org/terst/feedreader/data/CachedExtractionTest.kt index fc9c32e..442663f 100644 --- a/app/src/test/java/org/terst/feedreader/data/CachedExtractionTest.kt +++ b/app/src/test/java/org/terst/feedreader/data/CachedExtractionTest.kt @@ -96,4 +96,23 @@ class CachedExtractionTest { assertFalse(result.discardedConsentNotice) assertFalse(cleared) } + + /** + * A cached article carrying the publisher's "Trending" promo widgets. Unlike a cookie + * notice, the article itself is fine -- only the widgets have to go, and re-extracting + * would be a pointless round trip that produced the same body again. + */ + @Test + fun `a cached article has its recirculation widgets stripped without being re-extracted`() = runTest { + var cleared = false + val body = "

" + "The actual article body. ".repeat(60) + "

" + val widget = "

Trending

" + + val result = loadCachedExtraction(body + widget + body) { cleared = true } + + assertFalse("the stray header survived: ${result.content}", result.content!!.contains("Trending")) + assertTrue(result.content!!.contains("The actual article body.")) + assertFalse("the article was fine; nothing should be thrown away", result.discarded) + assertFalse("re-extracting would fetch the same body again", cleared) + } } diff --git a/app/src/test/java/org/terst/feedreader/data/FeedDiscoveryTest.kt b/app/src/test/java/org/terst/feedreader/data/FeedDiscoveryTest.kt new file mode 100644 index 0000000..eee238a --- /dev/null +++ b/app/src/test/java/org/terst/feedreader/data/FeedDiscoveryTest.kt @@ -0,0 +1,93 @@ +package org.terst.feedreader.data + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Test + +/** + * Finding the feed when what you were given is a site. + * + * Reported live: "Can't subscribe to https://heathercoxrichardson.substack.com/". Pasting a + * site's home page was the only thing to hand -- a site does not advertise its feed URL + * anywhere a reader can see, it advertises it in a that every other + * reader looks for and this one did not. + */ +class FeedDiscoveryTest { + + /** Verbatim from the reported page, 2026-10-21. */ + private val substackHead = + """Letters from an American""" + + """...""" + + @Test + fun `finds the feed a Substack home page advertises, resolved against the site`() { + assertEquals( + "https://heathercoxrichardson.substack.com/feed", + FeedDiscovery.findFeedUrl(substackHead, "https://heathercoxrichardson.substack.com/") + ) + } + + @Test + fun `finds an Atom feed too`() { + val html = """""" + + assertEquals("https://example.com/atom.xml", FeedDiscovery.findFeedUrl(html, "https://example.com/")) + } + + @Test + fun `keeps an absolute href as it stands`() { + val html = """""" + + assertEquals("https://feeds.example.net/x.xml", FeedDiscovery.findFeedUrl(html, "https://example.com/blog/")) + } + + @Test + fun `resolves a relative href against the page, not the site root`() { + val html = """""" + + assertEquals("https://example.com/blog/feed.xml", FeedDiscovery.findFeedUrl(html, "https://example.com/blog/")) + } + + @Test + fun `copes with the attributes in any order and single quotes`() { + val html = """""" + + assertEquals("https://example.com/rss", FeedDiscovery.findFeedUrl(html, "https://example.com")) + } + + @Test + fun `prefers the first feed when a page advertises several`() { + val html = + """""" + + """""" + + assertEquals("https://example.com/feed", FeedDiscovery.findFeedUrl(html, "https://example.com/")) + } + + @Test + fun `ignores an alternate link that is not a feed`() { + val html = + """""" + + """""" + + assertNull(FeedDiscovery.findFeedUrl(html, "https://example.com/")) + } + + @Test + fun `a page with no feed at all reports nothing rather than guessing a path`() { + assertNull(FeedDiscovery.findFeedUrl("hi", "https://example.com/")) + } + + @Test + fun `an http feed link is upgraded, since the app cannot fetch cleartext`() { + val html = """""" + + assertEquals("https://example.com/feed", FeedDiscovery.findFeedUrl(html, "https://example.com/")) + } + + @Test + fun `xml declaration means this was already a feed, so there is nothing to discover`() { + assertNull(FeedDiscovery.findFeedUrl("", "https://x/f")) + } +} diff --git a/app/src/test/java/org/terst/feedreader/readability/EvalOutcomeTest.kt b/app/src/test/java/org/terst/feedreader/readability/EvalOutcomeTest.kt index 8995e22..3e2b28f 100644 --- a/app/src/test/java/org/terst/feedreader/readability/EvalOutcomeTest.kt +++ b/app/src/test/java/org/terst/feedreader/readability/EvalOutcomeTest.kt @@ -1,6 +1,7 @@ package org.terst.feedreader.readability import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue import org.junit.Test @@ -86,4 +87,21 @@ class EvalOutcomeTest { assertTrue(f.userMessage().lowercase().contains("cookie") || f.userMessage().lowercase().contains("consent")) assertTrue(f.userMessage() != ExtractionFailure.NoContent.userMessage()) } + + /** + * A fresh extraction is cleaned before it is accepted, so the widgets never reach the + * screen and never reach `articles.extractedContent` either. + */ + @Test + fun `a successful extraction has its recirculation widgets stripped`() { + val body = "

" + "Real article sentence. ".repeat(40) + "

" + val widget = "

Trending

" + val article = ExtractedArticle(title = "T", content = body + widget + body) + + val decision = decideEvalOutcome(article, rawResult = "{}", attempt = 1, maxAttempts = 6) + + val content = ((decision as EvalDecision.Done).result as ExtractionResult.Success).article.content!! + assertFalse("the stray header reached the caller: $content", content.contains("Trending")) + assertTrue(content.contains("Real article sentence.")) + } } diff --git a/app/src/test/java/org/terst/feedreader/readability/RecirculationWidgetsTest.kt b/app/src/test/java/org/terst/feedreader/readability/RecirculationWidgetsTest.kt new file mode 100644 index 0000000..0926b80 --- /dev/null +++ b/app/src/test/java/org/terst/feedreader/readability/RecirculationWidgetsTest.kt @@ -0,0 +1,117 @@ +package org.terst.feedreader.readability + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * Promo blocks a publisher's widget platform injects into the middle of the article body. + * + * Reported live on West Hawaii Today: "articles get a bunch of stray 'Trending' headers". + * Confirmed on 9 of the 12 articles in its feed, two per article. + */ +class RecirculationWidgetsTest { + + /** The exact markup that survives extraction, from a West Hawaii Today article. */ + private val realWidget = + """

Trending

""" + + @Test + fun `strips a recirculation widget from the middle of an article`() { + val html = "

Real sentence one.

$realWidget

Real sentence two.

" + + val cleaned = RecirculationWidgets.strip(html)!! + + assertFalse("the stray header is still there: $cleaned", cleaned.contains("Trending")) + assertTrue(cleaned.contains("Real sentence one.")) + assertTrue(cleaned.contains("Real sentence two.")) + } + + @Test + fun `strips every widget, not just the first`() { + val html = "

One.

$realWidget

Two.

$realWidget

Three.

" + + val cleaned = RecirculationWidgets.strip(html)!! + + assertEquals(0, Regex("Trending").findAll(cleaned).count()) + assertTrue(cleaned.contains("Three.")) + } + + @Test + fun `leaves an article with no widgets exactly as it was`() { + val html = "

Nothing to do here.

A real heading

More.

" + + assertEquals(html, RecirculationWidgets.strip(html)) + } + + @Test + fun `never removes a block long enough to be real writing`() { + val paragraph = "Beaches across the island lost coral cover. ".repeat(20) + val html = """

$paragraph

""" + + assertTrue( + "a block this long is article text whatever attribute it carries", + RecirculationWidgets.strip(html)!!.contains("Beaches across the island") + ) + } + + @Test + fun `handles an unquoted or single-quoted attribute`() { + val html = "

a

Trending

b

" + + assertFalse(RecirculationWidgets.strip(html)!!.contains("Trending")) + } + + @Test + fun `survives a nested div inside the widget`() { + val html = "

a

Trending

b

" + + val cleaned = RecirculationWidgets.strip(html)!! + assertFalse(cleaned.contains("Trending")) + assertTrue(cleaned.contains("

a

")) + assertTrue(cleaned.contains("

b

")) + } + + @Test + fun `an unclosed widget div does not eat the rest of the article`() { + val html = "

Before.

Trending

After.

" + + assertTrue( + "a malformed widget must not take the article with it", + RecirculationWidgets.strip(html)!!.contains("Before.") + ) + } + + @Test + fun `null and blank are left alone`() { + assertEquals(null, RecirculationWidgets.strip(null)) + assertEquals("", RecirculationWidgets.strip("")) + } + + /** + * The whole extracted article, captured from the live page on 2026-10-21 by running the + * vendored Readability.js over it exactly as the app does. + * + * The unit cases above use the widget markup on its own, which proves the matcher but not + * that it meets reality in one piece. This is the article Peter would have been reading. + */ + @Test + fun `the real extracted article loses both widgets and keeps its text`() { + val extracted = checkNotNull( + javaClass.getResourceAsStream("/west-hawaii-today-extracted.html") + ).bufferedReader().use { it.readText() } + assertEquals("fixture should carry the reported bug", 2, Regex("Trending").findAll(extracted).count()) + + val cleaned = RecirculationWidgets.strip(extracted)!! + + assertEquals(0, Regex("Trending").findAll(cleaned).count()) + assertTrue(cleaned.contains("Leavitt officially stepped down as White House press secretary")) + assertTrue(cleaned.contains("The New York Times")) + assertTrue( + "barely any of the article should have gone with the widgets", + cleaned.length > extracted.length - 600 + ) + } +} diff --git a/app/src/test/java/org/terst/feedreader/ui/YouTubeLinksTest.kt b/app/src/test/java/org/terst/feedreader/ui/YouTubeLinksTest.kt new file mode 100644 index 0000000..3afba16 --- /dev/null +++ b/app/src/test/java/org/terst/feedreader/ui/YouTubeLinksTest.kt @@ -0,0 +1,139 @@ +package org.terst.feedreader.ui + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * The YouTube placeholder: recognising one, and building a page its embed will actually play in. + * + * Both halves come from the same report: "YouTube play is misconfigured, long click on YouTube + * placeholder should open the URL directly." + */ +class YouTubeLinksTest { + + /** + * What a long-press on the reader's video placeholder actually reports. + * + * Not the link's href, which is what the first version of this assumed and got wrong on the + * device: the placeholder is ``, + * and for an image inside a link WebView's hit test reports SRC_IMAGE_ANCHOR_TYPE with the + * *image's* URL in `extra`, not the anchor's. Long-pressing the thumbnail produced the + * ordinary link menu showing `https://i.ytimg.com/vi/dQw4w9WgXcQ/hq720.jpg`. + * + * The thumbnail URL carries the video id, so it is resolved back to the video. + */ + @Test + fun `a long-press on the thumbnail resolves to the video, which is what WebView reports`() { + assertEquals( + "https://www.youtube.com/watch?v=dQw4w9WgXcQ", + youTubeTargetForLongPress("https://i.ytimg.com/vi/dQw4w9WgXcQ/hq720.jpg") + ) + } + + @Test + fun `any thumbnail size resolves, since the reader has used more than one`() { + assertEquals( + "https://www.youtube.com/watch?v=dQw4w9WgXcQ", + youTubeTargetForLongPress("https://i.ytimg.com/vi/dQw4w9WgXcQ/maxresdefault.jpg") + ) + assertEquals( + "https://www.youtube.com/watch?v=dQw4w9WgXcQ", + youTubeTargetForLongPress("https://i9.ytimg.com/vi_webp/dQw4w9WgXcQ/hq720.webp") + ) + } + + @Test + fun `a watch link resolves to itself, for wherever one does come through`() { + assertEquals( + "https://www.youtube.com/watch?v=dQw4w9WgXcQ", + youTubeTargetForLongPress("https://www.youtube.com/watch?v=dQw4w9WgXcQ") + ) + } + + @Test + fun `a short youtu-be link resolves to the video`() { + assertEquals( + "https://www.youtube.com/watch?v=dQw4w9WgXcQ", + youTubeTargetForLongPress("https://youtu.be/dQw4w9WgXcQ") + ) + } + + @Test + fun `an ordinary article link is not a video`() { + assertNull(youTubeTargetForLongPress("https://www.nytimes.com/2026/10/20/us/politics/x.html")) + } + + @Test + fun `an ordinary article image is not a video`() { + assertNull(youTubeTargetForLongPress("https://www.westhawaiitoday.com/wp-content/uploads/reef.jpg")) + } + + @Test + fun `a lookalike host is not YouTube`() { + assertNull(youTubeTargetForLongPress("https://notyoutube.com/watch?v=dQw4w9WgXcQ")) + assertNull(youTubeTargetForLongPress("https://youtube.com.evil.example/watch?v=dQw4w9WgXcQ")) + assertNull(youTubeTargetForLongPress("https://i.ytimg.com.evil.example/vi/dQw4w9WgXcQ/hq720.jpg")) + } + + @Test + fun `a YouTube link that is not one video stays on the ordinary menu`() { + assertNull(youTubeTargetForLongPress("https://www.youtube.com/@someone")) + assertNull(youTubeTargetForLongPress("https://www.youtube.com/playlist?list=PL123")) + } + + @Test + fun `rubbish is not a video`() { + assertNull(youTubeTargetForLongPress("")) + assertNull(youTubeTargetForLongPress("not a url at all")) + } + + // --- the embed page ------------------------------------------------------------------ + + @Test + fun `the embed page puts the player in an iframe, which is the whole point`() { + val html = youTubeEmbedPageHtml("dQw4w9WgXcQ") + + assertTrue("loading the embed top-level is what YouTube rejects: $html", html.contains("""") + + assertFalse("markup from the id reached the page: $html", html.contains("
+ + + + + +

Published 12:05 am Wednesday, October 21, 2026

+ +
+

+ By Maggie Haberman and Jonathan Swan NYTimes News Service +

+ +
+

WASHINGTON — In late August, during one of Karoline Leavitt’s final meetings with President Donald Trump in the Oval Office, he pitched her on an idea in front of several senior aides. He told her to move reporters out of the James S. Brady Press Briefing Room, which would effectively remove news organizations from the White House. It would be, he told her, her “last legacy.”

+

But the proposal never moved forward while she was there. The president had put the request to her several times over months, according to three people with knowledge of the discussions who were not authorized to speak publicly. Leavitt officially stepped down as White House press secretary on Aug. 27.

+

Trump has told aides he would prefer to bring back the indoor pool President Franklin D. Roosevelt added in 1933 as therapy for his polio-afflicted legs; President Richard Nixon later covered it over to make room for the news media.

+

Trending

While moving the media out of the White House may be a relatively small debate considering Trump’s attacks on journalists more broadly, it would be highly symbolic. The proximity of the media to the seat of U.S. executive power has long been an acknowledgment of the importance of a free press in a constitutional democracy.

+

Trump has brought up the press-eviction idea before, dating to his first term, and he has never quite dropped it.

+

As recently as Sept. 27, Trump shared a pair of images on Truth Social showing the briefing room as it is now, captioned “Fake News,” alongside another that shows the space reimagined as a gilded swimming pool, captioned “FDR Pool.”

+

The Wall Street Journal reported last month that Trump had repeatedly discussed moving the press corps out of the White House, and that his advisers had so far either offered pushback or waited for him to move on.

+

What has changed since Trump’s first term or even earlier this term is the staff around him. The president’s core team has shrunk from the already-tight unit that he brought into the White House last year. During earlier discussions, most of Trump’s senior aides agreed it served him better to keep the media nearby. But Trump has grown more enraged about his press coverage, and other advisers of growing influence have encouraged him to take drastic measures to punish journalists.

+

This article originally appeared in The New York Times.

+

Trending

© 2026 The New York Times Company

\ No newline at end of file -- cgit v1.2.3