Merge the browser naming and history fixes into claude/relaxed-maxwell-hlc0a2

BrowserHistoryRegistry moved to commons on main, so the normalised history key lands there (with TimeUtils.nowMillis, as the commons copy uses). BrowserHistoryKeyTest moves to commons' jvmTest with it, since historyKey is now internal to commons. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TEkj7Eo2xbidVHAoF8GZKQ
2026-10-05 19:28:25 +00:00 · 2026-09-27 01:29:10 +00:00
parent 6a9b42311f 62fe456810
commit b903f8e433
11 changed files with 395 additions and 12 deletions
@@ -17,6 +17,17 @@
            <action android:name="android.intent.action.TTS_SERVICE" />
        </intent>

+        <!-- So the browser's hand-off tile can name the browser it would open.
+             Android 11+ package visibility answers resolveActivity with nothing
+             for a scheme that is not declared here, so without this the tile can
+             only ever say "Open in browser". Read-only: it names the target, the
+             hand-off itself still goes through a chooser. -->
+        <intent>
+            <action android:name="android.intent.action.VIEW" />
+            <category android:name="android.intent.category.BROWSABLE" />
+            <data android:scheme="https" />
+        </intent>
+
        <!-- NIP-A3 payment targets. Android 11+ package visibility means
             queryIntentActivities returns NOTHING for a scheme not declared here,
             so without these the zap picker's pay-to chip is invisible on every
@@ -745,7 +745,11 @@ private fun RecentRow(
                overflow = TextOverflow.Ellipsis,
            )
            Text(
-                entry.host,
+                // Host and path, not just the host: two pages of one site usually carry the
+                // same <title>, so `primal.net/home` and `primal.net` arrived as two rows
+                // reading "Primal / primal.net" and there was no way to tell them apart or
+                // to know which one a tap would open.
+                remember(entry.url, entry.host) { recentSubtitle(entry.url, entry.host) },
                style = MaterialTheme.typography.bodySmall,
                color = MaterialTheme.colorScheme.onSurfaceVariant,
                maxLines = 1,
@@ -818,3 +822,20 @@ private fun SiteIcon(

 /** The host of [url] for a favorite's default label, falling back to the raw string. */
 private fun hostOf(url: String): String = OmniboxInput.hostOf(url) ?: url
+
+/**
+ * The second line of a Recent row: what a reader needs to tell two rows of one site apart.
+ *
+ * The scheme and a bare trailing slash are noise at this size, so they go; everything after the
+ * host stays, because that is the part that differs. Falls back to the host when the URL has
+ * nothing more to say.
+ */
+internal fun recentSubtitle(
+    url: String,
+    host: String,
+): String {
+    val schemeEnd = url.indexOf("://")
+    val afterScheme = if (schemeEnd > 0) url.substring(schemeEnd + 3) else url
+    val trimmed = afterScheme.removeSuffix("/")
+    return trimmed.ifBlank { host }
+}
@@ -0,0 +1,62 @@
+/*
+ * 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.favorites
+
+import com.vitorpamplona.amethyst.ui.screen.loggedIn.browser.recentSubtitle
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertNotEquals
+import org.junit.Test
+
+/** The two rows that both read "Primal / primal.net" and could not be told apart. */
+class RecentSubtitleTest {
+    @Test
+    fun `two pages of one site no longer read the same`() {
+        assertNotEquals(
+            recentSubtitle("https://primal.net/", "primal.net"),
+            recentSubtitle("https://primal.net/home", "primal.net"),
+        )
+    }
+
+    @Test
+    fun `the root shows the bare host`() {
+        assertEquals("primal.net", recentSubtitle("https://primal.net/", "primal.net"))
+    }
+
+    @Test
+    fun `a path is shown after the host`() {
+        assertEquals("primal.net/home", recentSubtitle("https://primal.net/home", "primal.net"))
+    }
+
+    @Test
+    fun `a port is kept, since it is part of where you are going`() {
+        assertEquals("localhost:8000/t.html", recentSubtitle("http://localhost:8000/t.html", "localhost"))
+    }
+
+    @Test
+    fun `a query is kept`() {
+        assertEquals("x.com/search?q=nostr", recentSubtitle("https://x.com/search?q=nostr", "x.com"))
+    }
+
+    @Test
+    fun `a url with nothing to add falls back to the host`() {
+        assertEquals("primal.net", recentSubtitle("", "primal.net"))
+    }
+}
@@ -99,8 +99,9 @@ class BrowserHistoryRegistry(
    ) {
        val host = OmniboxInput.hostOf(url) ?: url
        val now = TimeUtils.nowMillis()
+        val key = historyKey(url)
        update { current ->
-            val existing = current.firstOrNull { it.url == url }
+            val existing = current.firstOrNull { historyKey(it.url) == key }
            val entry =
                if (existing != null) {
                    existing.copy(
@@ -112,18 +113,22 @@ class BrowserHistoryRegistry(
                } else {
                    BrowserHistoryEntry(url = url, title = title, host = host, lastVisitedAt = now, visitCount = 1)
                }
-            (listOf(entry) + current.filterNot { it.url == url }).take(MAX_ENTRIES)
+            (listOf(entry) + current.filterNot { historyKey(it.url) == key }).take(MAX_ENTRIES)
        }
    }

-    fun remove(url: String) = update { current -> current.filterNot { it.url == url } }
+    fun remove(url: String) =
+        update { current ->
+            val key = historyKey(url)
+            current.filterNot { historyKey(it.url) == key }
+        }

    fun clear() = update { emptyList() }

    private fun dedupeNewestFirst(list: List<BrowserHistoryEntry>): List<BrowserHistoryEntry> =
        list
            .sortedByDescending { it.lastVisitedAt }
-            .distinctBy { it.url }
+            .distinctBy { historyKey(it.url) }
            .take(MAX_ENTRIES)

    private inline fun update(transform: (List<BrowserHistoryEntry>) -> List<BrowserHistoryEntry>) {
@@ -158,3 +163,26 @@ class BrowserHistoryRegistry(
        private const val MAX_ENTRIES = 500
    }
 }
+
+/**
+ * What counts as "the same page" in the history list.
+ *
+ * Keying on the raw URL string put `primal.net` in the list twice: a trailing slash, a different case
+ * in the host, or a leftover `#fragment` reads as one page and compares as two Strings. So the scheme
+ * and host are lowercased, a fragment is dropped, and a trailing slash is dropped when the path is
+ * nothing but that slash.
+ *
+ * The path and the query are kept and compared as-is. Two pages of the same site are two entries, and
+ * guessing which query parameters are load-bearing is how you lose one of them.
+ */
+internal fun historyKey(url: String): String {
+    val noFragment = url.substringBefore('#')
+    val schemeEnd = noFragment.indexOf("://")
+    if (schemeEnd <= 0) return noFragment
+    val scheme = noFragment.substring(0, schemeEnd).lowercase()
+    val rest = noFragment.substring(schemeEnd + 3)
+    val authorityEnd = rest.indexOfFirst { it == '/' || it == '?' }
+    val authority = (if (authorityEnd < 0) rest else rest.substring(0, authorityEnd)).lowercase()
+    val tail = if (authorityEnd < 0) "" else rest.substring(authorityEnd)
+    return scheme + "://" + authority + if (tail == "/") "" else tail
+}
@@ -0,0 +1,69 @@
+/*
+ * 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.browser
+
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertNotEquals
+import org.junit.Test
+
+/** The list showed `primal.net` twice; these are the pairs that produced that. */
+class BrowserHistoryKeyTest {
+    @Test
+    fun `a trailing slash on the root is the same page`() {
+        assertEquals(historyKey("https://primal.net"), historyKey("https://primal.net/"))
+    }
+
+    @Test
+    fun `the host is compared without case`() {
+        assertEquals(historyKey("https://Primal.NET/"), historyKey("https://primal.net/"))
+    }
+
+    @Test
+    fun `a fragment is not a different page`() {
+        assertEquals(historyKey("https://primal.net/home"), historyKey("https://primal.net/home#top"))
+    }
+
+    @Test
+    fun `a path is still its own page`() {
+        assertNotEquals(historyKey("https://primal.net/"), historyKey("https://primal.net/home"))
+    }
+
+    @Test
+    fun `a query is still its own page`() {
+        assertNotEquals(historyKey("https://x.com/search?q=a"), historyKey("https://x.com/search?q=b"))
+    }
+
+    @Test
+    fun `a trailing slash deeper in the path is left alone`() {
+        // Servers are free to treat these as different, so we do not decide for them.
+        assertNotEquals(historyKey("https://primal.net/home"), historyKey("https://primal.net/home/"))
+    }
+
+    @Test
+    fun `http and https are different origins`() {
+        assertNotEquals(historyKey("http://primal.net/"), historyKey("https://primal.net/"))
+    }
+
+    @Test
+    fun `something that is not a url is returned as itself`() {
+        assertEquals("not a url", historyKey("not a url"))
+    }
+}
@@ -5479,6 +5479,7 @@
    <string name="browser_pill_add_home">Add to Home</string>
    <string name="browser_pill_desktop">Desktop site</string>
    <string name="browser_pill_other_browser">Open in browser</string>
+    <string name="browser_pill_other_browser_named">Open in %1$s</string>
    <string name="browser_pill_full_screen">Full screen</string>
    <string name="browser_pill_left_site">You left %1$s</string>
    <string name="browser_pill_back_to_app">Back to app</string>
@@ -82,6 +82,7 @@ import com.vitorpamplona.amethyst.commons.resources.browser_pill_close
 import com.vitorpamplona.amethyst.commons.resources.browser_pill_decision_allowed
 import com.vitorpamplona.amethyst.commons.resources.browser_pill_decision_blocked
 import com.vitorpamplona.amethyst.commons.resources.browser_pill_left_site
+import com.vitorpamplona.amethyst.commons.resources.browser_pill_other_browser_named
 import com.vitorpamplona.amethyst.commons.resources.browser_pill_permission_state
 import com.vitorpamplona.amethyst.commons.resources.browser_pill_privacy
 import com.vitorpamplona.amethyst.commons.resources.browser_pill_site_settings_none
@@ -352,7 +353,7 @@ private fun OutOfScopeBanner(
    }
 }

-/** Page actions as tiles, four per row; toggles (desktop site, text size) fill while on. */
+/** Page actions as tiles, at most four per row; toggles (desktop site, text size) fill while on. */
@Composable
 private fun TileGrid(
    actions: List<Action>,
@@ -360,15 +361,26 @@ private fun TileGrid(
    textSizeOpen: Boolean,
    onAction: (Action) -> Unit,
 ) {
-    val columns = 4
+    // Spread over the rows we are going to use anyway, rather than filling each
+    // one to four and leaving the remainder stranded. Six actions read as 3 + 3
+    // instead of 4 + 2 with a hole beside it, and five as 3 + 2 instead of 4 + 1.
+    // Every row is chunked to the same width, so the tiles stay a uniform size
+    // and a gap appears only when the count is not divisible — seven is 4 + 3
+    // either way.
+    var taken = 0
+    val rows = balancedRowSizes(actions.size, MAX_TILE_COLUMNS).map { size -> actions.subList(taken, taken + size).also { taken += size } }
    Column(verticalArrangement = Arrangement.spacedBy(8.dp)) {
-        actions.chunked(columns).forEach { rowActions ->
+        rows.forEach { rowActions ->
            Row(horizontalArrangement = Arrangement.spacedBy(8.dp)) {
                rowActions.forEach { action ->
+                    // The hand-off tile names the browser it will actually open when the
+                    // system will name one; "Open in browser" is what is left when the
+                    // answer is a chooser. See BrowserPillUi.defaultBrowserName.
+                    val named = ui.defaultBrowserName?.takeIf { action == Action.OPEN_IN_BROWSER_APP }
                    ActionTile(
                        symbol = pillSymbolFor(action) ?: MaterialSymbols.Info,
-                        label = stringRes(pillTileLabelFor(action)),
-                        description = stringRes(pillLabelFor(action)),
+                        label = named?.let { stringRes(Res.string.browser_pill_other_browser_named, it) } ?: stringRes(pillTileLabelFor(action)),
+                        description = named?.let { stringRes(Res.string.browser_pill_other_browser_named, it) } ?: stringRes(pillLabelFor(action)),
                        onClick = { onAction(action) },
                        selected =
                            when (action) {
@@ -379,13 +391,35 @@ private fun TileGrid(
                        modifier = Modifier.weight(1f),
                    )
                }
-                // Keep the last row's tiles the same width as the others.
-                repeat(columns - rowActions.size) { Spacer(Modifier.weight(1f)) }
+                // Keep every row's tiles the same width, whatever the row holds.
+                repeat(MAX_TILE_COLUMNS - rowActions.size) { Spacer(Modifier.weight(1f)) }
            }
        }
    }
 }

+/** Most tiles on one row. Four is what fits at the pill's width without the labels wrapping twice. */
+private const val MAX_TILE_COLUMNS = 4
+
+/**
+ * How many tiles go on each row, spread as evenly as the count allows.
+ *
+ * The number of rows is fixed by [max] either way — this only decides how the
+ * items are shared out between them, so the leftovers do not all land on the
+ * last row. Ten tiles are 4 + 3 + 3, not 4 + 4 + 2. Rows are padded back out
+ * to [max] where they draw, so the tiles stay a uniform width throughout.
+ */
+internal fun balancedRowSizes(
+    count: Int,
+    max: Int,
+): List<Int> {
+    if (count <= 0) return emptyList()
+    val rows = (count + max - 1) / max
+    val base = count / rows
+    val remainder = count % rows
+    return List(rows) { index -> base + if (index < remainder) 1 else 0 }
+}
+
 /** Text size: a stepped slider between a small and a large "A", the value, and a way back to 100%. */
@Composable
 fun TextSizeControl(
@@ -44,6 +44,13 @@ data class BrowserPillUi(
    val consoleErrors: Int = 0,
    /** Answers this site already has (camera / mic / location), for the site-settings summary. */
    val sitePermissions: Map<BrowserSitePermission, BrowserSitePermission.Decision> = emptyMap(),
+    /**
+     * The default browser's name, when the system will name one and it is not us.
+     *
+     * Null means the hand-off shows a chooser, and the tile has to stay "Open in browser":
+     * no default is set, the device hides it, or the only handler is Amethyst itself.
+     */
+    val defaultBrowserName: String? = null,
 ) {
    val security: BrowserChrome.Security get() = BrowserChrome.security(chrome)
    val host: String get() = BrowserChrome.displayHost(chrome.url)
@@ -0,0 +1,80 @@
+/*
+ * 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.browser.ui.pill
+
+import kotlin.test.Test
+import kotlin.test.assertEquals
+
+class BalancedRowSizesTest {
+    private fun rows(
+        count: Int,
+        max: Int = 4,
+    ) = balancedRowSizes(count, max)
+
+    @Test
+    fun `six fills two rows evenly instead of stranding two`() {
+        assertEquals(listOf(3, 3), rows(6))
+    }
+
+    @Test
+    fun `five is three and two, not four and one`() {
+        assertEquals(listOf(3, 2), rows(5))
+    }
+
+    @Test
+    fun `seven cannot be even and stays four and three`() {
+        assertEquals(listOf(4, 3), rows(7))
+    }
+
+    @Test
+    fun `a full row is left alone`() {
+        assertEquals(listOf(4), rows(4))
+        assertEquals(listOf(4, 4), rows(8))
+    }
+
+    @Test
+    fun `no row is ever wider than the maximum, and every tile is placed`() {
+        (1..24).forEach { count ->
+            val rows = rows(count)
+            assertEquals(true, rows.all { it in 1..4 }, "count=$count rows=$rows")
+            assertEquals(count, rows.sum(), "count=$count rows=$rows")
+        }
+    }
+
+    @Test
+    fun `rows never differ by more than one`() {
+        (1..24).forEach { count ->
+            val rows = rows(count)
+            assertEquals(true, rows.max() - rows.min() <= 1, "count=$count rows=$rows")
+        }
+    }
+
+    @Test
+    fun `it never puts everything on one row it cannot fit`() {
+        assertEquals(listOf(4, 4, 4), rows(12))
+        assertEquals(listOf(4, 3, 3), rows(10))
+    }
+
+    @Test
+    fun `an empty grid does not divide by zero`() {
+        assertEquals(emptyList(), balancedRowSizes(0, 4))
+    }
+}
@@ -0,0 +1,69 @@
+/*
+ * 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.napplethost
+
+import android.content.Context
+import android.content.Intent
+import android.content.pm.PackageManager
+import android.content.pm.ResolveInfo
+import androidx.core.net.toUri
+
+/**
+ * Who "open in a browser" would actually hand the page to.
+ *
+ * Only used to say so on the tile. The hand-off itself still goes through a chooser, so a wrong
+ * or missing answer here costs a label, never a mis-launch.
+ */
+object DefaultBrowser {
+    /** A probe URL: the scheme is what selects browsers, the host is never contacted. */
+    private val PROBE = Intent(Intent.ACTION_VIEW, "https://example.com".toUri()).addCategory(Intent.CATEGORY_BROWSABLE)
+
+    /**
+     * The default browser's app label, or null when the tile should stay generic.
+     *
+     * Null in three cases that all mean the same thing to a reader — you are going to get a
+     * chooser, so do not promise a name:
+     *  - no default is set, and the system resolves to its own picker;
+     *  - the only handler is Amethyst, so "open in a browser" means anything but us;
+     *  - package visibility hides it. Android 11+ answers `resolveActivity` with nothing unless
+     *    the manifest declares a matching `<queries>` entry, which is why one exists for
+     *    http/https alongside the payment schemes.
+     */
+    fun label(context: Context): String? =
+        runCatching {
+            val pm = context.packageManager
+
+            @Suppress("DEPRECATION")
+            val match: ResolveInfo = pm.resolveActivity(PROBE, PackageManager.MATCH_DEFAULT_ONLY) ?: return null
+            val pkg = match.activityInfo?.packageName ?: return null
+            // The system picker resolves for everything; naming it would be a lie.
+            if (pkg == context.packageName || isResolver(match)) return null
+            match.loadLabel(pm).toString().takeIf { it.isNotBlank() }
+        }.getOrNull()
+
+    /**
+     * Whether this is Android's own chooser rather than a browser.
+     *
+     * `resolveActivity` returns the resolver when several apps match and none is default; it
+     * reports `exported=false` from the `android` package, which is the cheap way to tell.
+     */
+    private fun isResolver(info: ResolveInfo): Boolean = info.activityInfo?.packageName == "android"
+}
@@ -1337,6 +1337,7 @@ class NappletBrowserActivity : ComponentActivity() {
                            torOn = if (proxyPort > 0) useTor else null,
                        ),
                    isFavorite = intent.getBooleanExtra(EXTRA_IS_FAVORITE, false),
+                    defaultBrowserName = DefaultBrowser.label(this),
                ),
            listener = chromeListener,
        )