From 322726ab517ee3722f2143f096d37c3815de44ae Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 1 Oct 2026 13:44:27 +0000 Subject: [PATCH 1/3] feat: compact link preview from , meta description and favicon Pages without OpenGraph (e.g. most nsites) never got a preview card: the fetch was only kept when it found an image, so they fell back to a bare link even when they had a perfectly good title and description. - MetaTagsParser can now also yield the document <title> and icon <link>s (opt-in via includeTitleAndIcons, used by HtmlParser). - OpenGraphParser falls back to <title> when no og/twitter/meta title exists, and picks the best icon (scalable > largest declared size; apple-touch-icon defaults to 180px; mask-icon ignored). - UrlInfoItem keeps a page with a title or description, and resolves its icon (falling back to /favicon.ico at the origin). - UrlPreviewCard renders a short, wide card (icon left; title, description and host right) when there is no image, with a globe placeholder until the icon loads. The composer's thumbnail falls back to the icon too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KdPp7UiETpzv8jQ5qfAFqp --- .../ui/note/creators/previews/PreviewUrl.kt | 10 +- .../amethyst/commons/preview/HtmlParser.kt | 2 +- .../commons/preview/MetaTagsParser.kt | 123 +++++++++++- .../commons/preview/OpenGraphParser.kt | 67 ++++++- .../amethyst/commons/preview/UrlInfoItem.kt | 26 ++- .../preview/TitleAndIconFallbackTest.kt | 179 +++++++++++++++++ .../amethyst/commons/preview/UrlPreview.kt | 1 + .../preview/UrlInfoItemTextPreviewTest.kt | 87 ++++++++ .../commons/ui/components/UrlPreviewCard.kt | 187 +++++++++++++++--- 9 files changed, 637 insertions(+), 45 deletions(-) create mode 100644 commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/preview/TitleAndIconFallbackTest.kt create mode 100644 commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/preview/UrlInfoItemTextPreviewTest.kt diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/creators/previews/PreviewUrl.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/creators/previews/PreviewUrl.kt index 72aa4b9509..1c50e33e81 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/creators/previews/PreviewUrl.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/creators/previews/PreviewUrl.kt @@ -26,6 +26,7 @@ import androidx.compose.foundation.layout.aspectRatio import androidx.compose.foundation.layout.fillMaxHeight import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.padding import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text import androidx.compose.runtime.Composable @@ -39,6 +40,7 @@ import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.Color import androidx.compose.ui.layout.ContentScale import androidx.compose.ui.text.style.TextOverflow +import androidx.compose.ui.unit.dp import coil3.compose.AsyncImage import com.vitorpamplona.amethyst.commons.model.navigation.Route import com.vitorpamplona.amethyst.commons.richtext.RichTextParser @@ -239,11 +241,13 @@ private fun MyLoadUrlPreviewDirect( ) } else { Box(contentAlignment = Alignment.BottomCenter, modifier = Modifier.aspectRatio(1f)) { + // A page with no OpenGraph image still has its icon: shown whole, not cropped. + val hasImage = state.previewInfo.imageUrlFullPath.isNotBlank() AsyncImage( - model = state.previewInfo.imageUrlFullPath, + model = if (hasImage) state.previewInfo.imageUrlFullPath else state.previewInfo.iconUrlFullPath, contentDescription = state.previewInfo.title, - contentScale = ContentScale.Crop, - modifier = Modifier.fillMaxSize(), + contentScale = if (hasImage) ContentScale.Crop else ContentScale.Fit, + modifier = if (hasImage) Modifier.fillMaxSize() else Modifier.fillMaxSize().padding(20.dp), ) Text( diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/preview/HtmlParser.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/preview/HtmlParser.kt index 823862bf31..27571bebae 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/preview/HtmlParser.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/preview/HtmlParser.kt @@ -49,7 +49,7 @@ class HtmlParser { ?: bodyBytes.bomCharsetName() ?: HtmlCharsetParser.detectCharset(bodyBytes) val content = decodeBytes(bodyBytes, name) - MetaTagsParser.parse(content) + MetaTagsParser.parse(content, includeTitleAndIcons = true) } private fun ByteArray.bomCharsetName(): String? { diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/preview/MetaTagsParser.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/preview/MetaTagsParser.kt index 62e46ced82..6fefdf4dc2 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/preview/MetaTagsParser.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/preview/MetaTagsParser.kt @@ -22,8 +22,21 @@ package com.vitorpamplona.amethyst.commons.preview import com.vitorpamplona.amethyst.commons.util.codePointToChars +/** The `<head>` element a [MetaTag] was read from. */ +enum class HeadElement { + /** A `<meta>` tag, the only kind [MetaTagsParser.parse] yields unless asked for more. */ + META, + + /** The document's `<title>`, its text carried as the `content` attribute. */ + TITLE, + + /** A `<link>` whose `rel` names an icon (`icon`, `shortcut icon`, `apple-touch-icon`, …). */ + LINK, +} + data class MetaTag( private val attrs: Map<String, String>, + val element: HeadElement = HeadElement.META, ) { /** * Returns a value of an attribute specified by its name (case insensitive), or empty string if it doesn't exist. @@ -43,7 +56,16 @@ object MetaTagsParser { private const val NO_QUOTE = ' ' private const val META = "meta" + private const val LINK = "link" private const val HEAD = "head" + private const val ICON = "icon" + private const val REL = "rel" + private const val CONTENT = "content" + + // A `<title>` is only a fallback label, so one that runs on (an unclosed tag swallowing the + // page, a spam keyword dump) is cut rather than carried into the preview cache whole. + private const val MAX_TITLE_LENGTH = 300 + private val WHITESPACE_RUN = Regex("\\s+") // Elements whose content is text rather than markup: script and style hold raw text, title and // textarea hold character data. A `<` inside any of them is not a tag. @@ -66,26 +88,81 @@ object MetaTagsParser { /** The `</head>` that ends the interesting part of the document. */ HEAD_END, + /** A closed `<title>`: its text is [TagScanner.textStart]..<[TagScanner.textEnd]. */ + TITLE, + + /** A `<link …>` whose attribute span mentions `icon`; same span fields as [META]. */ + LINK, + /** Anything else: other elements, comments, declarations, unparseable markup. */ OTHER, } /** * Lazily parse a partial HTML document and extract meta tags. + * + * With [includeTitleAndIcons], the document's `<title>` and its icon `<link>`s are yielded + * too, tagged by [MetaTag.element]. They are what a page without OpenGraph still offers for a + * preview: browsers show exactly these two in a tab. */ - fun parse(input: String): Sequence<MetaTag> = + fun parse( + input: String, + includeTitleAndIcons: Boolean = false, + ): Sequence<MetaTag> = sequence { val s = TagScanner(input) while (!s.exhausted()) { - val kind = s.nextTag() - if (kind == TagKind.HEAD_END) break - if (kind == TagKind.META) { - val attrs = parseAttrs(input, s.attrsStart, s.attrsEnd) ?: continue - yield(MetaTag(attrs)) + when (s.nextTag()) { + TagKind.HEAD_END -> { + break + } + + TagKind.META -> { + val attrs = parseAttrs(input, s.attrsStart, s.attrsEnd) ?: continue + yield(MetaTag(attrs)) + } + + TagKind.TITLE -> { + if (includeTitleAndIcons) { + val text = titleText(input, s.textStart, s.textEnd) + if (text.isNotEmpty()) yield(MetaTag(mapOf(CONTENT to text), HeadElement.TITLE)) + } + } + + TagKind.LINK -> { + if (includeTitleAndIcons) { + val attrs = parseAttrs(input, s.attrsStart, s.attrsEnd) ?: continue + if (isIconRel(attrs[REL])) yield(MetaTag(attrs, HeadElement.LINK)) + } + } + + TagKind.OTHER -> {} } } } + /** + * Whether a `rel` names an icon: one of its space-separated tokens is `icon` or an + * `apple-touch-icon` variant. `mask-icon` is deliberately not one -- it is Safari's monochrome + * pinned-tab silhouette, which renders as a black blob anywhere else. + */ + private fun isIconRel(rel: String?): Boolean = + rel != null && + rel.split(' ', '\t', '\n', '\r', '\u000C').any { + it.equals(ICON, ignoreCase = true) || it.startsWith("apple-touch-icon", ignoreCase = true) + } + + /** The `<title>`'s character data, with references resolved and whitespace collapsed. */ + private fun titleText( + input: String, + from: Int, + to: Int, + ): String { + val raw = input.substring(from, to) + val decoded = if (raw.indexOf('&') < 0) raw else raw.replace(Attrs.RE_CHAR_REF, Attrs.Companion::replaceCharRefs) + return decoded.replace(WHITESPACE_RUN, " ").trim().take(MAX_TITLE_LENGTH) + } + private class TagScanner( private val input: String, ) { @@ -98,6 +175,12 @@ object MetaTagsParser { var attrsEnd = 0 private set + /** Character-data span of the `<title>` [nextTag] last reported as [TagKind.TITLE]. */ + var textStart = 0 + private set + var textEnd = 0 + private set + fun exhausted(): Boolean = p >= length /** @@ -128,6 +211,20 @@ object MetaTagsParser { p = if (end < 0) length else end + 1 } + /** True when `input[from..<to]` contains [lower], ASCII-case-insensitively. */ + private fun spanContains( + from: Int, + to: Int, + lower: String, + ): Boolean { + var i = from + while (i + lower.length <= to) { + if (input.regionMatches(i, lower, 0, lower.length, ignoreCase = true)) return true + i++ + } + return false + } + /** Leaves [p] on the `</name` that closes a raw-text element, or at the end of the input. */ private fun skipRawText(endTag: String) { var i = p @@ -215,14 +312,24 @@ object MetaTagsParser { // the same way an unbalanced quote inside a comment does. Switching on the name length // first keeps the common tag (a `<div>`, a `<link>`) down to one comparison. when (nameEnd - nameStart) { - META.length -> if (nameIs(nameStart, nameEnd, META)) return TagKind.META + META.length -> { + if (nameIs(nameStart, nameEnd, META)) return TagKind.META + // Most `<link>`s are stylesheets and preloads; checking the raw span for `icon` + // keeps their attributes from ever being parsed into a map. + if (nameIs(nameStart, nameEnd, LINK) && spanContains(attrsStart, attrsEnd, ICON)) return TagKind.LINK + } - STYLE.length -> + STYLE.length -> { if (nameIs(nameStart, nameEnd, STYLE)) { skipRawText(STYLE_END) } else if (nameIs(nameStart, nameEnd, TITLE)) { + textStart = p skipRawText(TITLE_END) + textEnd = p + // An unclosed title runs to the end of the input: that is not a title. + if (p < length) return TagKind.TITLE } + } SCRIPT.length -> if (nameIs(nameStart, nameEnd, SCRIPT)) skipRawText(SCRIPT_END) diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/preview/OpenGraphParser.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/preview/OpenGraphParser.kt index 9e4714756b..3c0dfe3667 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/preview/OpenGraphParser.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/preview/OpenGraphParser.kt @@ -22,6 +22,7 @@ package com.vitorpamplona.amethyst.commons.preview class OpenGraphParser { class Result( + /** The OpenGraph/Twitter/meta title, or the document's `<title>` when it declares none. */ val title: String, val description: String, val image: String, @@ -35,6 +36,8 @@ class OpenGraphParser { val videoType: String = "", /** `og:type` — what the page says it *is*, e.g. `music.song`, `video.other`, `article`. */ val type: String = "", + /** The best icon `<link>` the page declares, verbatim (may be relative). Empty when none. */ + val icon: String = "", ) companion object { @@ -112,6 +115,17 @@ class OpenGraphParser { ) private val CONTENT = "content" + private val HREF = "href" + private val REL = "rel" + private val SIZES = "sizes" + private val TYPE = "type" + + // How an icon with no `sizes` is ranked. An `apple-touch-icon` is 180px by Apple's + // convention; a plain `icon` without sizes is usually the 16/32px tab favicon. A scalable + // one (SVG, or `sizes="any"`) beats every bitmap: it is sharp at any card size. + private const val APPLE_TOUCH_ICON_DEFAULT_SIZE = 180 + private const val ICON_DEFAULT_SIZE = 32 + private const val SCALABLE_ICON_SIZE = Int.MAX_VALUE } /** Which field of [Result] a meta tag's key fills, or null when the key is not one we read. */ @@ -156,8 +170,33 @@ class OpenGraphParser { var video = "" var videoType = "" var type = "" + var documentTitle = "" + var icon = "" + var iconSize = -1 metaTags.forEach { + when (it.element) { + HeadElement.META -> {} + + HeadElement.TITLE -> { + if (documentTitle.isEmpty()) documentTitle = it.attr(CONTENT) + return@forEach + } + + HeadElement.LINK -> { + val href = it.attr(HREF) + if (href.isNotBlank()) { + val size = iconSize(it) + // Strictly greater, so among equals the first declared wins. + if (size > iconSize) { + icon = href + iconSize = size + } + } + return@forEach + } + } + // A meta tag names its key in exactly one of these three attributes, but which one // varies by site, so each is tried in turn until one is a key we read. val field = @@ -179,6 +218,32 @@ class OpenGraphParser { null -> Unit } } - return Result(title, description, image, audio, audioType, video, videoType, type) + return Result(title.ifEmpty { documentTitle }, description, image, audio, audioType, video, videoType, type, icon) + } + + /** The pixel size an icon `<link>` is ranked by: its largest declared size, or a default. */ + private fun iconSize(link: MetaTag): Int { + val sizes = link.attr(SIZES) + if (sizes.contains("any", ignoreCase = true)) return SCALABLE_ICON_SIZE + if (link.attr(TYPE).equals("image/svg+xml", ignoreCase = true)) return SCALABLE_ICON_SIZE + if (link + .attr(HREF) + .substringBefore('?') + .substringBefore('#') + .endsWith(".svg", ignoreCase = true) + ) { + return SCALABLE_ICON_SIZE + } + + // `sizes="16x16 32x32"`: rank by the largest, which is the one a decoder will pick. + val declared = + sizes + .split(' ') + .mapNotNull { it.lowercase().substringBefore('x', "").toIntOrNull() } + .maxOrNull() + if (declared != null) return declared + + val isAppleTouch = link.attr(REL).contains("apple-touch-icon", ignoreCase = true) + return if (isAppleTouch) APPLE_TOUCH_ICON_DEFAULT_SIZE else ICON_DEFAULT_SIZE } } diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/preview/UrlInfoItem.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/preview/UrlInfoItem.kt index 5cb955845d..00b24466cf 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/preview/UrlInfoItem.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/preview/UrlInfoItem.kt @@ -43,6 +43,8 @@ class UrlInfoItem( val videoType: String = "", /** `og:type` — what the page says it is, e.g. `music.song`, `video.other`, `article`. */ val type: String = "", + /** The page's best declared icon `<link>` (favicon / apple-touch-icon). Empty when none. */ + val icon: String = "", ) { /** The page's host, or null when [url] is not an absolute URL. */ val verifiedHost = absoluteUrlHost(url) @@ -66,6 +68,14 @@ class UrlInfoItem( // which is what it did before. Only the playable gate below treats unresolvable as refusal. val imageUrlFullPath = resolve(image) ?: image + /** + * The page's icon, absolute: the one it declared, else `/favicon.ico` at its origin -- the + * same guess every browser makes, since most sites that declare nothing still serve one. + * Null for a non-HTML URL (an image or a video is its own preview) or an unresolvable one. + */ + val iconUrlFullPath: String? = + if (mimeType.startsWith("text/html")) resolve(icon) ?: resolve(DEFAULT_FAVICON) else null + /** * The declared `og:video`, absolute, but only when the declaration holds up: a [videoType] the * renderer can actually play (a `video/` MIME, an `audio/` one, or an HLS playlist type), or @@ -119,13 +129,25 @@ class UrlInfoItem( else -> audioType.ifEmpty { null } } + /** + * Whether the page has text to show without an image: a title (OpenGraph's or the document's + * own `<title>`) or a description. Such a page gets the compact icon + text card rather than + * a bare link. + */ + val hasTextPreview: Boolean = title.isNotBlank() || description.isNotBlank() + /** * Whether the fetch produced something worth rendering. An image is the usual evidence, but a * page that declared playable media counts too — a track page that ships no cover art would * otherwise be thrown away as Empty and fall back to a bare link, which is exactly the player - * we went to the trouble of finding. + * we went to the trouble of finding. So does a page with no OpenGraph at all that still has a + * title or description: it renders as the compact card instead of a bare link. */ - fun fetchComplete(): Boolean = url.isNotEmpty() && (image.isNotEmpty() || playableMediaUrl != null) + fun fetchComplete(): Boolean = url.isNotEmpty() && (image.isNotEmpty() || playableMediaUrl != null || hasTextPreview) fun allFetchComplete(): Boolean = title.isNotEmpty() && description.isNotEmpty() && image.isNotEmpty() + + companion object { + private const val DEFAULT_FAVICON = "/favicon.ico" + } } diff --git a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/preview/TitleAndIconFallbackTest.kt b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/preview/TitleAndIconFallbackTest.kt new file mode 100644 index 0000000000..d0f9db83cc --- /dev/null +++ b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/preview/TitleAndIconFallbackTest.kt @@ -0,0 +1,179 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.commons.preview + +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +/** + * A page with no OpenGraph still has what a browser tab shows: its `<title>` and its icon + * `<link>`s. These are read only when asked for, and only ever as a fallback. + */ +class TitleAndIconFallbackTest { + private fun headTags(html: String) = MetaTagsParser.parse(html, includeTitleAndIcons = true).toList() + + private fun extract(html: String) = OpenGraphParser().extractUrlInfo(MetaTagsParser.parse(html, includeTitleAndIcons = true)) + + // Trimmed from the nsite that prompted this: no og:*, no twitter:*, only a title, a meta + // description and an SVG favicon. + private val noOpenGraphPage = + """ + |<!DOCTYPE html> + |<html lang="en"> + |<head> + |<meta charset="utf-8"> + |<meta name="viewport" content="width=device-width, initial-scale=1"> + |<title>Private Provider — Confidential AI + | + | + | + | + |

Hi

+ | + """.trimMargin() + + @Test + fun aPageWithoutOpenGraphStillYieldsTitleDescriptionAndIcon() { + val info = extract(noOpenGraphPage) + + assertEquals("Private Provider — Confidential AI", info.title) + assertEquals("A phone-first AI workspace for Android.", info.description) + assertEquals("favicon.svg", info.icon) + assertEquals("", info.image) + } + + @Test + fun theDefaultParseStillYieldsOnlyMetaTags() { + val tags = MetaTagsParser.parse(noOpenGraphPage).toList() + + assertEquals(3, tags.size) + assertTrue(tags.all { it.element == HeadElement.META }) + } + + @Test + fun onlyIconLinksAreYielded() { + val tags = headTags(noOpenGraphPage) + + val links = tags.filter { it.element == HeadElement.LINK } + assertEquals(1, links.size) + assertEquals("favicon.svg", links[0].attr("href")) + } + + @Test + fun openGraphTitleWinsOverTheDocumentTitle() { + val info = + extract( + """ + | + | Document Title + | + | + """.trimMargin(), + ) + + assertEquals("OG Title", info.title) + } + + @Test + fun titleWhitespaceIsCollapsed() { + val info = extract("\n Spread\n\t Out ") + + assertEquals("Spread Out", info.title) + } + + @Test + fun anUnclosedTitleIsNotATitle() { + val info = extract("runs to the end of the truncated body") + + assertEquals("", info.title) + } + + @Test + fun aTitleOutsideTheHeadIsIgnored() { + // An SVG in the body carries its own <title>; the scan stops at </head> before it. + val info = extract("<head></head><body><svg><title>Close") + + assertEquals("", info.title) + } + + @Test + fun theFirstTitleWins() { + val info = extract("FirstSecond") + + assertEquals("First", info.title) + } + + @Test + fun theLargestIconWins() { + val info = + extract( + """ + | + | + | + | + | + | + """.trimMargin(), + ) + + assertEquals("/apple-touch-icon.png", info.icon) + } + + @Test + fun aScalableIconBeatsBitmaps() { + val info = + extract( + """ + | + | + | + | + """.trimMargin(), + ) + + assertEquals("/icon.svg?v=2", info.icon) + } + + @Test + fun maskIconIsNotUsed() { + // Safari's monochrome pinned-tab silhouette renders as a black blob anywhere else. + val info = + extract( + """ + | + | + | + | + """.trimMargin(), + ) + + assertEquals("/favicon-32.png", info.icon) + } + + @Test + fun iconLinksAfterTheHeadAreIgnored() { + val info = extract("""""") + + assertEquals("", info.icon) + } +} diff --git a/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/preview/UrlPreview.kt b/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/preview/UrlPreview.kt index 78a918541f..3410eec81c 100644 --- a/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/preview/UrlPreview.kt +++ b/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/preview/UrlPreview.kt @@ -77,6 +77,7 @@ class UrlPreview { data.video, data.videoType, data.type, + data.icon, ) } diff --git a/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/preview/UrlInfoItemTextPreviewTest.kt b/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/preview/UrlInfoItemTextPreviewTest.kt new file mode 100644 index 0000000000..eaeeeb96a4 --- /dev/null +++ b/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/preview/UrlInfoItemTextPreviewTest.kt @@ -0,0 +1,87 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.commons.preview + +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertNull +import kotlin.test.assertTrue + +/** + * A page with no OpenGraph image is still worth a card when it has text: the compact icon + text + * card, built from its title, description and favicon. + */ +class UrlInfoItemTextPreviewTest { + private fun page( + url: String = "https://example.nsite.lol/", + title: String = "", + description: String = "", + image: String = "", + icon: String = "", + mimeType: String = "text/html; charset=utf-8", + ) = UrlInfoItem(url = url, title = title, description = description, image = image, mimeType = mimeType, icon = icon) + + @Test + fun aTitleAloneIsEnoughToKeepThePreview() { + assertTrue(page(title = "Private Provider").fetchComplete()) + } + + @Test + fun aDescriptionAloneIsEnoughToKeepThePreview() { + assertTrue(page(description = "A workspace").fetchComplete()) + } + + @Test + fun aPageWithNothingStaysEmpty() { + assertFalse(page().fetchComplete()) + assertFalse(page(title = " ").fetchComplete()) + } + + @Test + fun aRelativeIconIsResolvedAgainstThePage() { + assertEquals( + "https://example.nsite.lol/docs/favicon.svg", + page(url = "https://example.nsite.lol/docs/index.html", icon = "favicon.svg").iconUrlFullPath, + ) + } + + @Test + fun noDeclaredIconFallsBackToTheOriginFavicon() { + assertEquals( + "https://example.nsite.lol/favicon.ico", + page(url = "https://example.nsite.lol/a/b/c.html").iconUrlFullPath, + ) + } + + @Test + fun aNonHttpIconFallsBackToTheOriginFavicon() { + assertEquals( + "https://example.nsite.lol/favicon.ico", + page(icon = "file:///etc/passwd").iconUrlFullPath, + ) + } + + @Test + fun nonHtmlUrlsHaveNoIcon() { + assertNull(page(url = "https://example.com/a.png", image = "https://example.com/a.png", mimeType = "image/png").iconUrlFullPath) + } +} diff --git a/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/UrlPreviewCard.kt b/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/UrlPreviewCard.kt index a084cb0edf..59265eb13d 100644 --- a/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/UrlPreviewCard.kt +++ b/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/UrlPreviewCard.kt @@ -21,24 +21,33 @@ package com.vitorpamplona.amethyst.commons.ui.components import androidx.compose.foundation.ExperimentalFoundationApi +import androidx.compose.foundation.background import androidx.compose.foundation.combinedClickable +import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.Spacer +import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.size import androidx.compose.material3.IconButton import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text import androidx.compose.runtime.Composable +import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.rememberCoroutineScope +import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier +import androidx.compose.ui.draw.clip import androidx.compose.ui.graphics.Color import androidx.compose.ui.layout.ContentScale import androidx.compose.ui.platform.LocalClipboard import androidx.compose.ui.platform.LocalUriHandler import androidx.compose.ui.text.style.TextOverflow +import androidx.compose.ui.unit.dp import coil3.compose.AsyncImage import com.vitorpamplona.amethyst.commons.icons.symbols.Icon import com.vitorpamplona.amethyst.commons.icons.symbols.MaterialSymbols @@ -50,9 +59,12 @@ import com.vitorpamplona.amethyst.commons.resources.link_actions_dialog_title import com.vitorpamplona.amethyst.commons.resources.url_preview_open_in_browser import com.vitorpamplona.amethyst.commons.ui.components.util.setText import com.vitorpamplona.amethyst.commons.ui.stringRes +import com.vitorpamplona.amethyst.commons.ui.theme.DoubleHorzSpacer import com.vitorpamplona.amethyst.commons.ui.theme.DoubleVertSpacer import com.vitorpamplona.amethyst.commons.ui.theme.MaxWidthWithHorzPadding import com.vitorpamplona.amethyst.commons.ui.theme.Size14Modifier +import com.vitorpamplona.amethyst.commons.ui.theme.Size24Modifier +import com.vitorpamplona.amethyst.commons.ui.theme.SmallBorder import com.vitorpamplona.amethyst.commons.ui.theme.innerPostModifier import com.vitorpamplona.amethyst.commons.ui.theme.previewCardImageModifier import kotlinx.coroutines.launch @@ -101,22 +113,42 @@ fun UrlPreviewCard( } } - Column( - modifier = - MaterialTheme.colorScheme.innerPostModifier - .combinedClickable( - onClick = { - if (onCardClick != null) { - onCardClick() - } else { - runCatching { uri.openUri(url) } - } - }, - onLongClick = { - popupExpanded.value = true - }, - ), - ) { + val cardModifier = + MaterialTheme.colorScheme.innerPostModifier + .combinedClickable( + onClick = { + if (onCardClick != null) { + onCardClick() + } else { + runCatching { uri.openUri(url) } + } + }, + onLongClick = { + popupExpanded.value = true + }, + ) + + // Only meaningful when the card's own tap does something else (e.g. opening the comment + // thread); otherwise it would duplicate the card's open-in-browser tap. + val onOpenInBrowser: (() -> Unit)? = onCardClick?.let { { runCatching { uri.openUri(url) } } } + + // A page with no picture to lead with -- most often one with no OpenGraph at all, previewed + // from its ``, meta description and favicon -- gets the short, wide card. Painting the + // big layout without its image would leave a text block that only looks broken. + if (previewInfo.imageUrlFullPath.isBlank() && previewInfo.hasTextPreview) { + CompactUrlPreviewCard(previewInfo, cardModifier, onOpenInBrowser) + } else { + LargeUrlPreviewCard(previewInfo, cardModifier, onOpenInBrowser) + } +} + +@Composable +private fun LargeUrlPreviewCard( + previewInfo: UrlInfoItem, + modifier: Modifier, + onOpenInBrowser: (() -> Unit)?, +) { + Column(modifier = modifier) { // A Loaded preview no longer implies an image: a player page that ships no cover art is // kept (it has media to play), and painting its empty string left a blank 180dp box. if (previewInfo.imageUrlFullPath.isNotBlank()) { @@ -141,20 +173,7 @@ fun UrlPreviewCard( overflow = TextOverflow.Ellipsis, ) - // Only meaningful when the card's own tap does something else (e.g. opening the - // comment thread); otherwise it would duplicate the card's open-in-browser tap. - if (onCardClick != null) { - IconButton( - onClick = { runCatching { uri.openUri(url) } }, - ) { - Icon( - symbol = MaterialSymbols.AutoMirrored.OpenInNew, - contentDescription = stringRes(Res.string.url_preview_open_in_browser), - modifier = Size14Modifier, - tint = Color.Gray, - ) - } - } + onOpenInBrowser?.let { OpenInBrowserButton(it) } } Text( @@ -177,3 +196,111 @@ fun UrlPreviewCard( Spacer(modifier = DoubleVertSpacer) } } + +/** + * The short, wide card: the site's icon on the left, its title, description and host on the + * right. Built from what every page has even without OpenGraph -- what a browser tab shows. + */ +@Composable +private fun CompactUrlPreviewCard( + previewInfo: UrlInfoItem, + modifier: Modifier, + onOpenInBrowser: (() -> Unit)?, +) { + val host = previewInfo.verifiedHost ?: previewInfo.url + + Row( + modifier = modifier.padding(10.dp), + verticalAlignment = Alignment.CenterVertically, + ) { + UrlPreviewIcon(previewInfo.iconUrlFullPath, host) + + Spacer(modifier = DoubleHorzSpacer) + + Column(modifier = Modifier.weight(1f)) { + Text( + text = previewInfo.title.ifBlank { host }, + style = MaterialTheme.typography.bodyMedium, + maxLines = 1, + overflow = TextOverflow.Ellipsis, + ) + + if (previewInfo.description.isNotBlank()) { + Text( + text = previewInfo.description, + style = MaterialTheme.typography.bodySmall, + color = Color.Gray, + maxLines = 2, + overflow = TextOverflow.Ellipsis, + ) + } + + // When there is no title the host already stands in for it on the first line. + if (previewInfo.title.isNotBlank()) { + Text( + text = host, + style = MaterialTheme.typography.bodySmall, + color = Color.Gray, + maxLines = 1, + overflow = TextOverflow.Ellipsis, + ) + } + } + + onOpenInBrowser?.let { OpenInBrowserButton(it) } + } +} + +/** + * The site's icon in a fixed square. A generic globe sits there until the icon actually loads, + * so a site whose favicon is missing (the `/favicon.ico` guess is only a guess) or undecodable + * still gets a tidy card instead of an empty hole. + */ +@Composable +private fun UrlPreviewIcon( + iconUrl: String?, + contentDescription: String, +) { + Box( + modifier = + UrlPreviewIconModifier + .background(MaterialTheme.colorScheme.onSurface.copy(alpha = 0.06f)), + contentAlignment = Alignment.Center, + ) { + var loaded by remember(iconUrl) { mutableStateOf(false) } + + if (!loaded) { + Icon( + symbol = MaterialSymbols.Language, + contentDescription = null, + modifier = Size24Modifier, + tint = Color.Gray, + ) + } + + if (iconUrl != null) { + AsyncImage( + model = iconUrl, + contentDescription = contentDescription, + contentScale = ContentScale.Fit, + modifier = UrlPreviewIconImageModifier, + onSuccess = { loaded = true }, + ) + } + } +} + +@Composable +private fun OpenInBrowserButton(onClick: () -> Unit) { + IconButton(onClick = onClick) { + Icon( + symbol = MaterialSymbols.AutoMirrored.OpenInNew, + contentDescription = stringRes(Res.string.url_preview_open_in_browser), + modifier = Size14Modifier, + tint = Color.Gray, + ) + } +} + +private val UrlPreviewIconModifier = Modifier.size(48.dp).clip(SmallBorder) +private val UrlPreviewIconImageModifier = Modifier.fillMaxSize().padding(6.dp) From ff15246c7b4a378b7af894039bb146ea5dedc676 Mon Sep 17 00:00:00 2001 From: Claude <noreply@anthropic.com> Date: Thu, 1 Oct 2026 14:37:31 +0000 Subject: [PATCH 2/3] fix: harden the title/favicon link preview fallback Audit fixes for the compact link preview: - Stop reading <title> and icon <link>s once <body> starts. </head> is optional, and without it an inline SVG's <title> ("Close", "Menu") was taken as the page title. - Never cut a long <title> inside a surrogate pair. - Match the HTML mime type case-insensitively: UrlPreview stores the server's spelling, so "Text/HTML" pages lost their icon. - Honour data: icons. An inline data:image icon (up to 64 KB) is used as is; `href="data:,"` means "no favicon", so /favicon.ico is no longer requested for those sites. - Remember icon URLs the server refused (HTTP errors only, not network errors) so a missing /favicon.ico is not re-requested every time its card scrolls back into view. - Keep the open-in-browser callback stable across recompositions. - Benchmark rows for the title/icon parse path: no measurable cost on the heavy head, ~1.5 us on a small one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KdPp7UiETpzv8jQ5qfAFqp --- .../commons/preview/MetaTagsParser.kt | 26 ++++++++++++-- .../amethyst/commons/preview/UrlInfoItem.kt | 16 ++++++++- .../preview/TitleAndIconFallbackTest.kt | 34 +++++++++++++++++++ .../preview/UrlInfoItemTextPreviewTest.kt | 22 ++++++++++++ .../prodbench/MetaTagsParserBenchmark.kt | 13 +++++++ .../commons/ui/components/UrlPreviewCard.kt | 24 +++++++++++-- 6 files changed, 129 insertions(+), 6 deletions(-) diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/preview/MetaTagsParser.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/preview/MetaTagsParser.kt index 6fefdf4dc2..5f4bd4cd4b 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/preview/MetaTagsParser.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/preview/MetaTagsParser.kt @@ -58,6 +58,7 @@ object MetaTagsParser { private const val META = "meta" private const val LINK = "link" private const val HEAD = "head" + private const val BODY = "body" private const val ICON = "icon" private const val REL = "rel" private const val CONTENT = "content" @@ -88,6 +89,12 @@ object MetaTagsParser { /** The `</head>` that ends the interesting part of the document. */ HEAD_END, + /** + * A `<body>` start tag. `</head>` is optional, so on a page that omits it this is the only + * sign the head is over. + */ + BODY_START, + /** A closed `<title>`: its text is [TagScanner.textStart]..<[TagScanner.textEnd]. */ TITLE, @@ -111,26 +118,34 @@ object MetaTagsParser { ): Sequence<MetaTag> = sequence { val s = TagScanner(input) + // Meta tags past an implicit end of head are still read, as they always were. The + // title and icons are not: past `<body>`, a `<title>` is an inline SVG's tooltip + // ("Close", "Menu"), not the page's name. + var inBody = false while (!s.exhausted()) { when (s.nextTag()) { TagKind.HEAD_END -> { break } + TagKind.BODY_START -> { + inBody = true + } + TagKind.META -> { val attrs = parseAttrs(input, s.attrsStart, s.attrsEnd) ?: continue yield(MetaTag(attrs)) } TagKind.TITLE -> { - if (includeTitleAndIcons) { + if (includeTitleAndIcons && !inBody) { val text = titleText(input, s.textStart, s.textEnd) if (text.isNotEmpty()) yield(MetaTag(mapOf(CONTENT to text), HeadElement.TITLE)) } } TagKind.LINK -> { - if (includeTitleAndIcons) { + if (includeTitleAndIcons && !inBody) { val attrs = parseAttrs(input, s.attrsStart, s.attrsEnd) ?: continue if (isIconRel(attrs[REL])) yield(MetaTag(attrs, HeadElement.LINK)) } @@ -160,7 +175,11 @@ object MetaTagsParser { ): String { val raw = input.substring(from, to) val decoded = if (raw.indexOf('&') < 0) raw else raw.replace(Attrs.RE_CHAR_REF, Attrs.Companion::replaceCharRefs) - return decoded.replace(WHITESPACE_RUN, " ").trim().take(MAX_TITLE_LENGTH) + val title = decoded.replace(WHITESPACE_RUN, " ").trim() + if (title.length <= MAX_TITLE_LENGTH) return title + // Never end on half of a surrogate pair: a lone high surrogate renders as tofu. + val cut = if (title[MAX_TITLE_LENGTH - 1].isHighSurrogate()) MAX_TITLE_LENGTH - 1 else MAX_TITLE_LENGTH + return title.substring(0, cut) } private class TagScanner( @@ -314,6 +333,7 @@ object MetaTagsParser { when (nameEnd - nameStart) { META.length -> { if (nameIs(nameStart, nameEnd, META)) return TagKind.META + if (nameIs(nameStart, nameEnd, BODY)) return TagKind.BODY_START // Most `<link>`s are stylesheets and preloads; checking the raw span for `icon` // keeps their attributes from ever being parsed into a map. if (nameIs(nameStart, nameEnd, LINK) && spanContains(attrsStart, attrsEnd, ICON)) return TagKind.LINK diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/preview/UrlInfoItem.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/preview/UrlInfoItem.kt index 00b24466cf..4977a6cd67 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/preview/UrlInfoItem.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/preview/UrlInfoItem.kt @@ -71,10 +71,20 @@ class UrlInfoItem( /** * The page's icon, absolute: the one it declared, else `/favicon.ico` at its origin -- the * same guess every browser makes, since most sites that declare nothing still serve one. + * + * A `data:` icon is used as-is when it is an image (the image loader decodes those inline), + * and taken at its word when it is not: `href="data:,"` is how a site tells browsers it has no + * favicon, so guessing `/favicon.ico` there is a request that is known to miss. + * * Null for a non-HTML URL (an image or a video is its own preview) or an unresolvable one. */ val iconUrlFullPath: String? = - if (mimeType.startsWith("text/html")) resolve(icon) ?: resolve(DEFAULT_FAVICON) else null + when { + !mimeType.startsWith("text/html", ignoreCase = true) -> null + icon.startsWith("data:image/", ignoreCase = true) -> if (icon.length <= MAX_INLINE_ICON_LENGTH) icon else resolve(DEFAULT_FAVICON) + icon.startsWith("data:", ignoreCase = true) -> null + else -> resolve(icon) ?: resolve(DEFAULT_FAVICON) + } /** * The declared `og:video`, absolute, but only when the declaration holds up: a [videoType] the @@ -149,5 +159,9 @@ class UrlInfoItem( companion object { private const val DEFAULT_FAVICON = "/favicon.ico" + + // Previews are cached, so an inline icon is held for as long as its card is. Real inline + // favicons are a few KB; anything far past that is not worth keeping in memory. + private const val MAX_INLINE_ICON_LENGTH = 64 * 1024 } } diff --git a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/preview/TitleAndIconFallbackTest.kt b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/preview/TitleAndIconFallbackTest.kt index d0f9db83cc..4822fceaa4 100644 --- a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/preview/TitleAndIconFallbackTest.kt +++ b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/preview/TitleAndIconFallbackTest.kt @@ -176,4 +176,38 @@ class TitleAndIconFallbackTest { assertEquals("", info.icon) } + + @Test + fun aBodyTitleIsIgnoredWhenTheHeadIsNeverClosed() { + // `</head>` is optional: `<body>` closes the head implicitly. An inline SVG's <title> + // ("Close", "Menu") is the classic thing to then mistake for the page title. + val info = extract("""<head><meta name="description" content="D"><body><svg><title>Close""") + + assertEquals("", info.title) + assertEquals("D", info.description) + } + + @Test + fun aBodyIconLinkIsIgnoredWhenTheHeadIsNeverClosed() { + val info = extract("""T""") + + assertEquals("T", info.title) + assertEquals("", info.icon) + } + + @Test + fun aTitleWithAttributesIsRead() { + val info = extract("""Helmet""") + + assertEquals("Helmet", info.title) + } + + @Test + fun aLongTitleIsNotCutInsideASurrogatePair() { + // 299 ASCII chars then an emoji: a plain take(300) keeps only its high surrogate. + val info = extract("" + "a".repeat(299) + "\uD83D\uDE00 tail") + + assertTrue(info.title.isNotEmpty()) + assertTrue(!info.title.last().isHighSurrogate(), "title ends in a dangling high surrogate") + } } diff --git a/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/preview/UrlInfoItemTextPreviewTest.kt b/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/preview/UrlInfoItemTextPreviewTest.kt index eaeeeb96a4..5998927fbe 100644 --- a/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/preview/UrlInfoItemTextPreviewTest.kt +++ b/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/preview/UrlInfoItemTextPreviewTest.kt @@ -84,4 +84,26 @@ class UrlInfoItemTextPreviewTest { fun nonHtmlUrlsHaveNoIcon() { assertNull(page(url = "https://example.com/a.png", image = "https://example.com/a.png", mimeType = "image/png").iconUrlFullPath) } + + @Test + fun anUppercaseHtmlMimeStillGetsAnIcon() { + // UrlPreview stores MediaType.toString(), which keeps the server's spelling. + assertEquals( + "https://example.nsite.lol/favicon.ico", + page(mimeType = "Text/HTML; charset=UTF-8").iconUrlFullPath, + ) + } + + @Test + fun anEmptyDataIconMeansTheSiteHasNoIcon() { + // `` is the common way to tell browsers not to request + // /favicon.ico. Requesting it anyway is a guaranteed miss. + assertNull(page(icon = "data:,").iconUrlFullPath) + } + + @Test + fun anInlineImageIconIsUsedAsIs() { + val inline = "data:image/png;base64,iVBORw0KGgo=" + assertEquals(inline, page(icon = inline).iconUrlFullPath) + } } diff --git a/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/prodbench/MetaTagsParserBenchmark.kt b/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/prodbench/MetaTagsParserBenchmark.kt index 228498fdf1..2a2d0f70db 100644 --- a/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/prodbench/MetaTagsParserBenchmark.kt +++ b/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/prodbench/MetaTagsParserBenchmark.kt @@ -160,6 +160,19 @@ class MetaTagsParserBenchmark { } bench("no , whole doc", open, 2_000) { MetaTagsParser.parse(it).count() } + // What production runs (HtmlParser asks for the and icon <link>s too). Should sit + // on top of the rows above: the 60 preload links never reach the attribute parser. + bench("spa head, +title/icons", spa, 50_000) { MetaTagsParser.parse(it, includeTitleAndIcons = true).count() } + bench("spa head, og: + fallbacks", spa, 50_000) { + OpenGraphParser().extractUrlInfo(MetaTagsParser.parse(it, includeTitleAndIcons = true)).title.length + } + bench("heavy head, +title/icons", heavy, 2_000) { MetaTagsParser.parse(it, includeTitleAndIcons = true).count() } + bench("no </head>, +title/icons", open, 2_000) { MetaTagsParser.parse(it, includeTitleAndIcons = true).count() } + + val spaFallbacks = OpenGraphParser().extractUrlInfo(MetaTagsParser.parse(spa, includeTitleAndIcons = true)) + assertEquals("Example — Your Network. Your Rules.", spaFallbacks.title) + assertEquals("/favicon.svg", spaFallbacks.icon) + assertTrue(MetaTagsParser.parse(open).count() >= 64) } } diff --git a/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/UrlPreviewCard.kt b/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/UrlPreviewCard.kt index 59265eb13d..66853c6bac 100644 --- a/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/UrlPreviewCard.kt +++ b/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/UrlPreviewCard.kt @@ -20,6 +20,7 @@ */ package com.vitorpamplona.amethyst.commons.ui.components +import androidx.collection.LruCache import androidx.compose.foundation.ExperimentalFoundationApi import androidx.compose.foundation.background import androidx.compose.foundation.combinedClickable @@ -49,6 +50,7 @@ import androidx.compose.ui.platform.LocalUriHandler import androidx.compose.ui.text.style.TextOverflow import androidx.compose.ui.unit.dp import coil3.compose.AsyncImage +import coil3.network.HttpException import com.vitorpamplona.amethyst.commons.icons.symbols.Icon import com.vitorpamplona.amethyst.commons.icons.symbols.MaterialSymbols import com.vitorpamplona.amethyst.commons.preview.UrlInfoItem @@ -130,7 +132,15 @@ fun UrlPreviewCard( // Only meaningful when the card's own tap does something else (e.g. opening the comment // thread); otherwise it would duplicate the card's open-in-browser tap. - val onOpenInBrowser: (() -> Unit)? = onCardClick?.let { { runCatching { uri.openUri(url) } } } + val hasCardClick = onCardClick != null + val onOpenInBrowser: (() -> Unit)? = + remember(hasCardClick, url, uri) { + if (hasCardClick) { + { runCatching { uri.openUri(url) } } + } else { + null + } + } // A page with no picture to lead with -- most often one with no OpenGraph at all, previewed // from its `<title>`, meta description and favicon -- gets the short, wide card. Painting the @@ -251,6 +261,13 @@ private fun CompactUrlPreviewCard( } } +/** + * Icon URLs the server refused (404 and the like) in this process. The image loader caches + * successes but not failures, so without this a site with no `/favicon.ico` would be asked for it + * again -- and 404 again -- every time its card scrolled back into view. + */ +private val failedIconUrls = LruCache<String, Unit>(200) + /** * The site's icon in a fixed square. A generic globe sits there until the icon actually loads, * so a site whose favicon is missing (the `/favicon.ico` guess is only a guess) or undecodable @@ -278,13 +295,16 @@ private fun UrlPreviewIcon( ) } - if (iconUrl != null) { + if (iconUrl != null && failedIconUrls[iconUrl] == null) { AsyncImage( model = iconUrl, contentDescription = contentDescription, contentScale = ContentScale.Fit, modifier = UrlPreviewIconImageModifier, onSuccess = { loaded = true }, + // Only a server's answer is remembered. A network error (offline, a timeout) says + // nothing about the icon and must be retried once the connection is back. + onError = { if (it.result.throwable is HttpException) failedIconUrls.put(iconUrl, Unit) }, ) } } From 3d9c5bf0c95b049c9b66d2031868351ba8e25fa2 Mon Sep 17 00:00:00 2001 From: Vitor Pamplona <vitor@vitorpamplona.com> Date: Thu, 1 Oct 2026 11:08:42 -0400 Subject: [PATCH 3/3] fix: resolve relative URLs against a pathless base on Android Android's java.net.URI resolves a relative reference against a base with an empty path by gluing it onto the host: `y18.svg` against `https://news.ycombinator.com` became `https://news.ycombinator.comy18.svg`, so Hacker News' compact link preview never loaded its icon (seen on an SM-T220). The JDK follows RFC 3986 and treats the empty path as `/`, so normalize the base to that before resolving. Rebuilt from the raw parts so an encoded query is not encoded twice. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --- .../util/HttpUrlResolution.jvmAndroid.kt | 22 +++++++- .../commons/util/HttpUrlResolutionTest.kt | 56 +++++++++++++++++++ 2 files changed, 77 insertions(+), 1 deletion(-) create mode 100644 commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/util/HttpUrlResolutionTest.kt diff --git a/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/util/HttpUrlResolution.jvmAndroid.kt b/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/util/HttpUrlResolution.jvmAndroid.kt index bda5b0c2a9..b27eb59efe 100644 --- a/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/util/HttpUrlResolution.jvmAndroid.kt +++ b/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/util/HttpUrlResolution.jvmAndroid.kt @@ -29,7 +29,7 @@ actual fun resolveHttpUrl( reference: String, ): String? = runCatching { - val baseUri = base?.let { runCatching { URI(it).toURL().toURI() }.getOrNull() } + val baseUri = base?.let { runCatching { URI(it).toURL().toURI().withRootPath() }.getOrNull() } val resolved = if (baseUri != null) baseUri.resolve(reference) else URI(reference) if (!resolved.scheme.equals("http", ignoreCase = true) && !resolved.scheme.equals("https", ignoreCase = true) @@ -39,3 +39,23 @@ actual fun resolveHttpUrl( resolved.toURL().toString() } }.getOrNull() + +/** + * `https://host` as `https://host/`. RFC 3986 resolves a relative reference against an empty base path + * as if the path were `/`, and the JDK does, but Android's `java.net.URI` does not: it glued + * `y18.svg` onto `https://news.ycombinator.com` as `https://news.ycombinator.comy18.svg`, so the + * page's icon (or a relative og:image) pointed at a host that does not exist. + */ +private fun URI.withRootPath(): URI = + if (isOpaque || !rawPath.isNullOrEmpty() || rawAuthority == null) { + this + } else { + // From the raw (still-encoded) parts: the multi-argument constructor would encode them again. + URI( + buildString { + append(scheme).append("://").append(rawAuthority).append('/') + rawQuery?.let { append('?').append(it) } + rawFragment?.let { append('#').append(it) } + }, + ) + } diff --git a/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/util/HttpUrlResolutionTest.kt b/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/util/HttpUrlResolutionTest.kt new file mode 100644 index 0000000000..353aacd5d1 --- /dev/null +++ b/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/util/HttpUrlResolutionTest.kt @@ -0,0 +1,56 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.commons.util + +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNull + +/** + * A relative reference against a base with no path resolves as if the path were `/` (RFC 3986). + * Android's `java.net.URI` got this wrong — `y18.svg` against `https://news.ycombinator.com` became + * `https://news.ycombinator.comy18.svg` — so the base is normalized before resolving. + */ +class HttpUrlResolutionTest { + @Test + fun relativeReferenceAgainstAPathlessBase() { + assertEquals("https://news.ycombinator.com/y18.svg", resolveHttpUrl("https://news.ycombinator.com", "y18.svg")) + assertEquals("https://news.ycombinator.com/favicon.ico", resolveHttpUrl("https://news.ycombinator.com", "/favicon.ico")) + } + + @Test + fun pathlessBaseKeepsItsQueryAndEncoding() { + assertEquals("https://a.example/icon.png", resolveHttpUrl("https://a.example?q=a%20b", "icon.png")) + // Encoded characters are carried over as they are, not encoded a second time. + assertEquals("https://a.example/icons/a%20b.png", resolveHttpUrl("https://a.example?q=a%20b", "icons/a%20b.png")) + } + + @Test + fun basesWithAPathAreUnchanged() { + assertEquals("https://a.example/dir/icon.png", resolveHttpUrl("https://a.example/dir/page.html", "icon.png")) + assertEquals("https://cdn.example/x.png", resolveHttpUrl("https://a.example/dir/", "//cdn.example/x.png")) + } + + @Test + fun nonHttpStaysRefused() { + assertNull(resolveHttpUrl("https://a.example", "file:///etc/passwd")) + } +}