Make an image zoomable even when the feed didn't spell out where it lives

Tapping an image in a Ftrain article did nothing. The zoom affordance was
there, and the rendering path was the ordinary one -- the feed's own HTML, with
reader mode off -- but a tap went nowhere at all.

Tap detection works by wrapping every article image in a link to a scheme
nothing can navigate to, which the WebView then hands to
shouldOverrideUrlLoading. That href has to carry the complete image URL on its
own, so an image whose src was relative got no wrapper and no tap: better an
image that can't be zoomed, the reasoning went, than a link that goes nowhere.
Ftrain's <content:encoded> is entirely site-relative -- every image in it is
src="/art/uploads/..." -- so not one of them was zoomable.

That reasoning had an unexamined premise. The page itself resolves those srcs
perfectly well, against the base URL loadReaderHtml already hands
loadDataWithBaseURL: the article's own link. Resolving them the same way at
HTML-assembly time produces exactly the URL the WebView went and fetched, so
the wrapper can be built after all. Both reader surfaces now pass that base URL
down to the zoom pass. Where there genuinely is no base URL to resolve
against -- nothing else in the app loads reader HTML without one today -- the
old behavior stands and the image stays un-zoomable rather than acquiring a
dead link, and a data: or javascript: src is still refused however it was
reached, since the viewer could never open one.

This is why the report could only have come from a feed with reader mode off.
Reader mode never hits it: Readability rewrites every src to an absolute URL
before the extracted HTML reaches us, which is also why the same images zoom
fine the moment the feed's reader toggle is on.

TDD: site-relative and path-relative resolution, and the whole trip through
wrapReaderHtml, were written first against the unused parameter and went red on
assertion mismatches (3 of 38 in ImageZoomTest), green on the resolution
itself. The data:-src case was written at the same time and passed immediately
-- it pins existing behavior against the new code path rather than having
driven it.

Verified on the emulator against the reported article itself, subscribed live
from ftrain.com/feed.xml with reader mode off: on the previous build, a tap on
the header image left the screen byte-identical; on this one the same tap at
the same point opens the full-screen viewer.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018AwnALZL6bD4VBtATDVv3B

4 files changed, 47 insertions, 13 deletions

diff --git a/app/src/main/java/org/terst/feedreader/ui/ImageZoomLinks.kt b/app/src/main/java/org/terst/feedreader/ui/ImageZoomLinks.kt
index b83b600..d924c02 100644
--- a/app/src/main/java/org/terst/feedreader/ui/ImageZoomLinks.kt
+++ b/app/src/main/java/org/terst/feedreader/ui/ImageZoomLinks.kt
@@ -1,5 +1,7 @@
 package org.terst.feedreader.ui
 
+import java.net.URI
+
 // How a tap on an article image is noticed at all, and turned into a URL the viewer
 // (ImageZoom.kt) can open. Split from the viewer itself because this half runs at
 // HTML-assembly time, inside wrapReaderHtml -- it's reader-markup plumbing, not gestures.
@@ -35,7 +37,7 @@ private val imageFileExtensions = listOf(".jpg", ".jpeg", ".png", ".gif", ".webp
  * and wrapping it would swallow the tap that plays the video). Those are still zoomable when
  * the link points at an image file, which [zoomableImageTarget] handles on the other side.
  */
-fun linkifyImagesForZoom(html: String): String = buildString {
+fun linkifyImagesForZoom(html: String, baseUrl: String? = null): String = buildString {
     var copiedUpTo = 0
     var anchorDepth = 0
     for (tag in anchorOrImageTagRegex.findAll(html)) {
@@ -45,7 +47,7 @@ fun linkifyImagesForZoom(html: String): String = buildString {
             text.startsWith("<a", ignoreCase = true) -> anchorDepth++
             anchorDepth == 0 -> {
                 val src = imgSrcRegex.find(text)?.let { it.groupValues[1].ifEmpty { it.groupValues[2] } }
-                val href = src?.let { zoomHrefForSrc(it) }
+                val href = src?.let { zoomHrefForSrc(it, baseUrl) }
                 if (href != null) {
                     append(html, copiedUpTo, tag.range.first)
                     append("<a href=\"").append(href).append("\">").append(text).append("</a>")
@@ -57,17 +59,14 @@ fun linkifyImagesForZoom(html: String): String = buildString {
     append(html, copiedUpTo, html.length)
 }
 
-private fun zoomHrefForSrc(src: String): String? {
+private fun zoomHrefForSrc(src: String, baseUrl: String?): String? {
     val trimmed = src.trim()
     val absolute = when {
         // Protocol-relative (//host/path) is common in feed markup and resolves fine in the
         // page itself, but our scheme-prefixed href has to carry a complete URL on its own.
         trimmed.startsWith("//") -> "https:$trimmed"
         trimmed.startsWith("http://", ignoreCase = true) || trimmed.startsWith("https://", ignoreCase = true) -> trimmed
-        // Anything else (site-relative, data:, javascript:) is skipped rather than guessed at:
-        // a relative src resolves against the WebView's base URL, which a custom-scheme href
-        // can't reproduce, so wrapping it would only build a dead link.
-        else -> null
+        else -> resolvedAgainstBase(trimmed, baseUrl)
     } ?: return null
     // The src came out of a quoted attribute and goes straight back into one, already escaped
     // as the source wrote it. A raw quote or angle bracket could only arrive via a
@@ -76,6 +75,32 @@ private fun zoomHrefForSrc(src: String): String? {
 }
 
 /**
+ * What a relative src actually points at, or null if there's no way to tell or it isn't
+ * something the viewer could open.
+ *
+ * A feed's own HTML routinely keeps its images site-relative (Ftrain's <content:encoded> is
+ * entirely `<img src="/art/uploads/...">`), and that path is what the non-reader rendering
+ * shows verbatim -- reader mode never hits this because Readability rewrites every src to an
+ * absolute URL before we see it. The page resolves those against the base URL that
+ * loadReaderHtml hands loadDataWithBaseURL, which is the article's own link, so resolving them
+ * the same way here yields exactly the URL the WebView itself fetched. With no base URL there
+ * is still nothing to resolve against, and the image stays un-zoomable rather than getting a
+ * dead link.
+ */
+private fun resolvedAgainstBase(src: String, baseUrl: String?): String? {
+    if (src.isEmpty() || baseUrl.isNullOrBlank()) return null
+    // URI() rejects a src with a space or other illegal character in it rather than guessing,
+    // which is the right answer here too -- an un-zoomable image beats a dead link.
+    val resolved = runCatching { URI(baseUrl).resolve(src) }.getOrNull()
+    // data: and javascript: srcs are already absolute, so resolve() hands them straight back;
+    // only a real network URL is something Coil could ever open.
+    return when (resolved?.scheme?.lowercase()) {
+        "http", "https" -> resolved.toString()
+        else -> null
+    }
+}
+
+/**
  * The image a tapped URL should open full-screen, or null if it isn't an image at all.
  *
  * Covers both halves of the problem: links [linkifyImagesForZoom] built, and links the article
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 9f9f74a..69de173 100644
--- a/app/src/main/java/org/terst/feedreader/ui/LinkReaderScreen.kt
+++ b/app/src/main/java/org/terst/feedreader/ui/LinkReaderScreen.kt
@@ -265,14 +265,14 @@ fun LinkReaderScreen(url: String, onBack: () -> Unit, onOpenLink: (String) -> Un
                                     onFontSizeChanged = { newSizePx -> scope.launch { settings.setReaderFontSizePx(newSizePx) } }
                                 )
                             ).apply {
-                                val html = wrapReaderHtml(currentExtractedContent, extractedTitle, fontSizePx = fontSizePx)
+                                val html = wrapReaderHtml(currentExtractedContent, extractedTitle, fontSizePx = fontSizePx, baseUrl = url)
                                 loadReaderHtml(url, html, fontSizePx)
                                 loadedKey.value = currentExtractedContent to fontSizePx
                             }
                         },
                         update = { webView ->
                             if (loadedKey.value != currentExtractedContent to fontSizePx) {
-                                val html = wrapReaderHtml(currentExtractedContent, extractedTitle, fontSizePx = fontSizePx)
+                                val html = wrapReaderHtml(currentExtractedContent, extractedTitle, fontSizePx = fontSizePx, baseUrl = url)
                                 webView.loadReaderHtml(url, html, fontSizePx)
                                 loadedKey.value = currentExtractedContent to fontSizePx
                             }
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 0d326ea..5a90c9e 100644
--- a/app/src/main/java/org/terst/feedreader/ui/ReaderScreen.kt
+++ b/app/src/main/java/org/terst/feedreader/ui/ReaderScreen.kt
@@ -611,14 +611,18 @@ private fun ReaderContent(
                                 onFontSizeChanged = { newSizePx -> scope.launch { settings.setReaderFontSizePx(newSizePx) } }
                             )
                         ).apply {
-                            val html = wrapReaderHtml(content, article?.title, article?.publishedAt, article?.imageUrl, fontSizePx)
+                            val html = wrapReaderHtml(
+                                content, article?.title, article?.publishedAt, article?.imageUrl, fontSizePx, baseUrl
+                            )
                             loadReaderHtml(baseUrl, html, fontSizePx)
                             loadedKey.value = content to fontSizePx
                         }
                     },
                     update = { webView ->
                         if (loadedKey.value != content to fontSizePx) {
-                            val html = wrapReaderHtml(content, article?.title, article?.publishedAt, article?.imageUrl, fontSizePx)
+                            val html = wrapReaderHtml(
+                                content, article?.title, article?.publishedAt, article?.imageUrl, fontSizePx, baseUrl
+                            )
                             webView.loadReaderHtml(baseUrl, html, fontSizePx)
                             loadedKey.value = content to fontSizePx
                         }
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 68562e8..fd0c6af 100644
--- a/app/src/main/java/org/terst/feedreader/ui/ReaderWebView.kt
+++ b/app/src/main/java/org/terst/feedreader/ui/ReaderWebView.kt
@@ -508,12 +508,17 @@ private fun escapeHtml(text: String): String = text
 // on whatever heading Readability happened to keep in bodyContent (increasingly nothing --
 // see ReaderExtractor's <header> stripping) or on the top app bar's title, which truncates
 // to one line and was the only place a long title showed up before this.
+// Six arguments, but five of them are defaulted document-assembly inputs that no caller has to
+// spell out, and the only thing they have in common is "something the page needs" -- bundling
+// them into a type purely to satisfy the count would make the call sites longer, not shorter.
+@Suppress("LongParameterList")
 fun wrapReaderHtml(
     bodyContent: String,
     title: String? = null,
     publishedAt: Long? = null,
     imageUrl: String? = null,
-    fontSizePx: Int = DEFAULT_READER_FONT_SIZE_PX
+    fontSizePx: Int = DEFAULT_READER_FONT_SIZE_PX,
+    baseUrl: String? = null
 ): String {
     val processedBody = replaceGenericIframes(replaceYouTubeIframes(bodyContent))
     // NB: linkifyImagesForZoom runs at the very end (below, over hero + body together), after
@@ -576,6 +581,6 @@ fun wrapReaderHtml(
     </style></head>
     <body>${if (!title.isNullOrBlank()) "<h1 class=\"article-title\">${escapeHtml(title)}</h1>" else ""}${
         if (publishedAt != null) "<p class=\"article-date\">${escapeHtml(absoluteDateText(publishedAt))}</p>" else ""
-    }${linkifyImagesForZoom(heroHtml + processedBody)}</body></html>
+    }${linkifyImagesForZoom(heroHtml + processedBody, baseUrl)}</body></html>
     """.trimIndent()
 }