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 --- .coverage-baseline | 2 +- BACKLOG.md | 14 +++ .../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 ++++ 17 files changed, 868 insertions(+), 19 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 diff --git a/.coverage-baseline b/.coverage-baseline index c6d8499..0d25359 100644 --- a/.coverage-baseline +++ b/.coverage-baseline @@ -1 +1 @@ -17.89 +24.09 diff --git a/BACKLOG.md b/BACKLOG.md index 2918517..bb7f075 100644 --- a/BACKLOG.md +++ b/BACKLOG.md @@ -51,6 +51,20 @@ doesn't just hand off to the YouTube app/browser as originally planned — dialog, main-frame navigation locked to YouTube's own domains), so watching a video doesn't leave the reader. +**Fixed 2026-10-21, reported as "YouTube play is misconfigured":** the dialog +loaded the embed as its own top-level document, and YouTube answers that with +"Video player configuration error -- Error 153". An embed has to be embedded; +it now sits 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