diff options
| author | Peter Stone <thepeterstone@gmail.com> | 2026-10-21 19:07:17 +0000 |
|---|---|---|
| committer | Peter Stone <thepeterstone@gmail.com> | 2026-10-21 19:07:17 +0000 |
| commit | 9210bf463a8818595f7ca42486a787d4f1941d55 (patch) | |
| tree | 3a38cf5d71a20393a33eef5a19b1a97554055e75 /app/src/test/java/org | |
| parent | e50f4fbbca21b85648f6c4045cbd50f672d315cd (diff) | |
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 <link rel="alternate"> 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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018AwnALZL6bD4VBtATDVv3B
Diffstat (limited to 'app/src/test/java/org')
5 files changed, 386 insertions, 0 deletions
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 = "<p>" + "The actual article body. ".repeat(60) + "</p>" + val widget = "<div data-mrf-recirculation=\"IL_x\"><p><h3><span>Trending</span></h3></p></div>" + + 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 <link rel="alternate"> that every other + * reader looks for and this one did not. + */ +class FeedDiscoveryTest { + + /** Verbatim from the reported page, 2026-10-21. */ + private val substackHead = + """<!doctype html><html><head><title>Letters from an American</title>""" + + """<link rel="alternate" type="application/rss+xml" href="/feed" """ + + """title="Letters from an American"/></head><body>...</body></html>""" + + @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 = """<link rel="alternate" type="application/atom+xml" href="/atom.xml">""" + + assertEquals("https://example.com/atom.xml", FeedDiscovery.findFeedUrl(html, "https://example.com/")) + } + + @Test + fun `keeps an absolute href as it stands`() { + val html = """<link rel="alternate" type="application/rss+xml" href="https://feeds.example.net/x.xml">""" + + 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 = """<link rel="alternate" type="application/rss+xml" href="feed.xml">""" + + 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 = """<link type='application/rss+xml' href='/rss' rel='alternate'>""" + + assertEquals("https://example.com/rss", FeedDiscovery.findFeedUrl(html, "https://example.com")) + } + + @Test + fun `prefers the first feed when a page advertises several`() { + val html = + """<link rel="alternate" type="application/rss+xml" href="/feed" title="Posts">""" + + """<link rel="alternate" type="application/rss+xml" href="/comments/feed" title="Comments">""" + + assertEquals("https://example.com/feed", FeedDiscovery.findFeedUrl(html, "https://example.com/")) + } + + @Test + fun `ignores an alternate link that is not a feed`() { + val html = + """<link rel="alternate" hreflang="fr" href="/fr/">""" + + """<link rel="alternate" type="application/json" href="/feed.json">""" + + 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("<html><head></head><body>hi</body></html>", "https://example.com/")) + } + + @Test + fun `an http feed link is upgraded, since the app cannot fetch cleartext`() { + val html = """<link rel="alternate" type="application/rss+xml" href="http://example.com/feed">""" + + 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("<?xml version=\"1.0\"?><rss><channel></channel></rss>", "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 = "<p>" + "Real article sentence. ".repeat(40) + "</p>" + val widget = "<div data-mrf-recirculation=\"IL_x\"><p><h3><span>Trending</span></h3></p></div>" + 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 = + """<div data-mrf-recirculation="IL_gKyXYBmnRnamdDxChJw6lw" data-testid="text-html" """ + + """data-mrf-experience="IL_gKyXYBmnRnamdDxChJw6lw"><p><h3><span>Trending</span></h3></p></div>""" + + @Test + fun `strips a recirculation widget from the middle of an article`() { + val html = "<p>Real sentence one.</p>$realWidget<p>Real sentence two.</p>" + + 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 = "<p>One.</p>$realWidget<p>Two.</p>$realWidget<p>Three.</p>" + + 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 = "<div><p>Nothing to do here.</p><h2>A real heading</h2><p>More.</p></div>" + + 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 = """<div data-mrf-recirculation="X"><p>$paragraph</p></div>""" + + 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 = "<p>a</p><div data-mrf-recirculation='Y'><h3>Trending</h3></div><p>b</p>" + + assertFalse(RecirculationWidgets.strip(html)!!.contains("Trending")) + } + + @Test + fun `survives a nested div inside the widget`() { + val html = "<p>a</p><div data-mrf-recirculation=\"Z\"><div><h3>Trending</h3></div></div><p>b</p>" + + val cleaned = RecirculationWidgets.strip(html)!! + assertFalse(cleaned.contains("Trending")) + assertTrue(cleaned.contains("<p>a</p>")) + assertTrue(cleaned.contains("<p>b</p>")) + } + + @Test + fun `an unclosed widget div does not eat the rest of the article`() { + val html = "<p>Before.</p><div data-mrf-recirculation=\"W\"><h3>Trending</h3><p>After.</p>" + + 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 `<a href="...watch?v=ID"><img src="...i.ytimg.com/vi/ID/..."></a>`, + * 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("<iframe")) + assertTrue(html.contains("https://www.youtube-nocookie.com/embed/dQw4w9WgXcQ")) + } + + @Test + fun `the embed page keeps autoplay and inline playback`() { + val html = youTubeEmbedPageHtml("dQw4w9WgXcQ") + + assertTrue(html.contains("autoplay=1")) + assertTrue(html.contains("playsinline=1")) + assertTrue("without this the iframe is not allowed to autoplay", html.contains("allow=\"autoplay")) + } + + @Test + fun `the base URL is a real https origin, since that is what the embed requires`() { + assertTrue(YOUTUBE_EMBED_BASE_URL.startsWith("https://")) + } + + @Test + fun `the base URL is not YouTube's own origin`() { + assertFalse( + "the embed has to be a guest on somebody else's page, not its own", + YOUTUBE_EMBED_BASE_URL.contains("youtube") + ) + } + + @Test + fun `a video id that could break out of the attribute is scrubbed`() { + val html = youTubeEmbedPageHtml("""a"><script>alert(1)</script>""") + + assertFalse("markup from the id reached the page: $html", html.contains("<script")) + assertFalse(html.contains("alert(1)")) + // What is left is only ever id characters, so the src attribute still closes where it should. + assertTrue(html.contains("embed/ascriptalert1script?")) + } + + @Test + fun `the watch link for a video is the one a phone can hand to the YouTube app`() { + assertEquals("https://www.youtube.com/watch?v=dQw4w9WgXcQ", youTubeWatchUrl("dQw4w9WgXcQ")) + } +} |
