diff options
Diffstat (limited to 'app')
5 files changed, 93 insertions, 16 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() } diff --git a/app/src/test/java/org/terst/feedreader/ui/ImageZoomTest.kt b/app/src/test/java/org/terst/feedreader/ui/ImageZoomTest.kt index 3f7078d..aead885 100644 --- a/app/src/test/java/org/terst/feedreader/ui/ImageZoomTest.kt +++ b/app/src/test/java/org/terst/feedreader/ui/ImageZoomTest.kt @@ -38,14 +38,46 @@ class ImageZoomTest { } @Test - fun `a relative image source is left alone`() { - // Relative srcs resolve against the WebView's base URL, which a custom-scheme href - // can't reproduce -- better to leave the image un-zoomable than to build a dead link. + fun `a relative image source is left alone when nothing says where it came from`() { + // Relative srcs resolve against the WebView's base URL -- with no base URL to resolve + // against, better to leave the image un-zoomable than to build a dead link. val original = """<img src="/local/a.jpg">""" assertEquals(original, linkifyImagesForZoom(original)) } @Test + fun `a site-relative image source resolves against the page it came from`() { + // Verbatim shape of the Ftrain feed's own markup, which is where this surfaced: + // every image in <content:encoded> is site-relative, so none of them were zoomable. + val html = linkifyImagesForZoom( + """<img src="/art/uploads/header.png" alt="x">""", + baseUrl = "https://ftrain.com/aboard-let-s-have-a-mouse-party" + ) + assertEquals( + """<a href="feedreader-image:https://ftrain.com/art/uploads/header.png">""" + + """<img src="/art/uploads/header.png" alt="x"></a>""", + html + ) + } + + @Test + fun `a path-relative image source resolves against the article's own directory`() { + val html = linkifyImagesForZoom( + """<img src="pic.png">""", + baseUrl = "https://ex.com/posts/2026/hello" + ) + assertTrue(html.contains("""<a href="feedreader-image:https://ex.com/posts/2026/pic.png">""")) + } + + @Test + fun `a data uri image is still left alone when there is a page url`() { + // Resolving a data: src against a base URL just gives the data: URI back -- it is + // not something the viewer could ever open, so it must not become a zoom target. + val original = """<img src="data:image/png;base64,AAAA">""" + assertEquals(original, linkifyImagesForZoom(original, baseUrl = "https://ex.com/post")) + } + + @Test fun `a data uri image is left alone`() { val original = """<img src="data:image/png;base64,AAAA">""" assertEquals(original, linkifyImagesForZoom(original)) @@ -87,6 +119,17 @@ class ImageZoomTest { } @Test + fun `reader html makes a feed article's relative images zoomable`() { + // The non-reader path renders the feed's own HTML, which routinely keeps relative + // srcs -- the reader path never hits this because Readability absolutizes them. + val html = wrapReaderHtml( + """<p>x</p><img src="/art/a.png">""", + baseUrl = "https://ftrain.com/aboard-let-s-have-a-mouse-party" + ) + assertTrue(html.contains("""<a href="feedreader-image:https://ftrain.com/art/a.png">""")) + } + + @Test fun `reader html makes the synthesized hero image zoomable too`() { val html = wrapReaderHtml("<p>no image of its own</p>", imageUrl = "https://ex.com/hero.jpg") assertTrue(html.contains("""<a href="feedreader-image:https://ex.com/hero.jpg">""")) |