diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/napplet/NappletBrokerService.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/napplet/NappletBrokerService.kt index 0a70c9ef76..e0d57da103 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/napplet/NappletBrokerService.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/napplet/NappletBrokerService.kt @@ -230,7 +230,11 @@ class NappletBrokerService : Service() { val url = data.getString(NappletIpc.KEY_HISTORY_URL)?.takeIf { it.isNotBlank() } ?: return true val history = Amethyst.instance.browserHistory history.init() - history.record(url, data.getString(NappletIpc.KEY_HISTORY_TITLE).orEmpty()) + history.record( + url = url, + title = data.getString(NappletIpc.KEY_HISTORY_TITLE).orEmpty(), + replaces = data.getString(NappletIpc.KEY_HISTORY_REPLACES), + ) return true } diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/browser/BrowserHistoryRegistry.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/browser/BrowserHistoryRegistry.kt index 87530bfe3b..aa64388c54 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/browser/BrowserHistoryRegistry.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/browser/BrowserHistoryRegistry.kt @@ -92,15 +92,33 @@ class BrowserHistoryRegistry( /** * Records a successful visit to [url], moving it to the front. An existing entry for the same URL is * bumped (visit count +1, title refreshed if non-blank); otherwise a new entry is prepended. + * + * [replaces] is the URL this page replaced in place — a search engine rewriting its own query, a JS + * redirect. That was one visit, not two, so the visit moves off the replaced entry: it is dropped if + * this was its only visit, otherwise its count goes back down by one. */ fun record( url: String, title: String, + replaces: String? = null, ) { val host = OmniboxInput.hostOf(url) ?: url val now = TimeUtils.nowMillis() val key = historyKey(url) - update { current -> + val replacedKey = replaces?.let(::historyKey)?.takeIf { it != key } + update { history -> + val current = + if (replacedKey == null) { + history + } else { + history.mapNotNull { + when { + historyKey(it.url) != replacedKey -> it + it.visitCount <= 1 -> null + else -> it.copy(visitCount = it.visitCount - 1) + } + } + } val existing = current.firstOrNull { historyKey(it.url) == key } val entry = if (existing != null) { diff --git a/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/browser/BrowserHistoryRegistryTest.kt b/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/browser/BrowserHistoryRegistryTest.kt index 108389ccfb..b3de6a2688 100644 --- a/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/browser/BrowserHistoryRegistryTest.kt +++ b/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/browser/BrowserHistoryRegistryTest.kt @@ -215,4 +215,70 @@ class BrowserHistoryRegistryTest { assertEquals("only the one", listOf("https://keep.example"), registry.history.value.map { it.url }) } + + /** + * One DuckDuckGo search left three rows: the typed `%20` query, the `+` query the page rewrote it to, + * and the `&ia=web` it rewrote that to. Each rewrite replaced the page in place, so it is one visit. + */ + @Test + fun aPageReplacedInPlaceIsOneVisit() = + runTest { + val (registry, _) = session(newFile()) + registry.init() + advanceUntilIdle() + + registry.record("https://duckduckgo.com/?q=nostr%20protocol", "DuckDuckGo") + registry.record("https://duckduckgo.com/?q=nostr+protocol", "DuckDuckGo", replaces = "https://duckduckgo.com/?q=nostr%20protocol") + registry.record("https://duckduckgo.com/?q=nostr+protocol&ia=web", "nostr protocol at DuckDuckGo", replaces = "https://duckduckgo.com/?q=nostr+protocol") + + assertEquals( + "only the landed url", + listOf("https://duckduckgo.com/?q=nostr+protocol&ia=web"), + registry.history.value.map { it.url }, + ) + assertEquals( + "one visit", + 1, + registry.history.value + .single() + .visitCount, + ) + } + + /** A page visited before keeps its row when a later visit to it is replaced — it only loses that visit. */ + @Test + fun replacingAnEarlierVisitedPageOnlyTakesBackOneVisit() = + runTest { + val (registry, _) = session(newFile()) + registry.init() + advanceUntilIdle() + + registry.record("https://example.com/a", "A") + registry.record("https://example.com/a", "A") + registry.record("https://example.com/b", "B", replaces = "https://example.com/a") + + val byUrl = registry.history.value.associate { it.url to it.visitCount } + assertEquals("a keeps its first visit", 1, byUrl["https://example.com/a"]) + assertEquals("b got the replaced one", 1, byUrl["https://example.com/b"]) + } + + /** A reload reports the page as replacing itself; that must not cost it a visit. */ + @Test + fun replacingAPageWithItselfIsAPlainRevisit() = + runTest { + val (registry, _) = session(newFile()) + registry.init() + advanceUntilIdle() + + registry.record("https://example.com/a", "A") + registry.record("https://example.com/a", "A", replaces = "https://example.com/a") + + assertEquals( + "still counted", + 2, + registry.history.value + .single() + .visitCount, + ) + } } diff --git a/nappletHost/src/main/kotlin/com/vitorpamplona/amethyst/napplethost/NappletBrowserActivity.kt b/nappletHost/src/main/kotlin/com/vitorpamplona/amethyst/napplethost/NappletBrowserActivity.kt index c4c10be001..15ccd56185 100644 --- a/nappletHost/src/main/kotlin/com/vitorpamplona/amethyst/napplethost/NappletBrowserActivity.kt +++ b/nappletHost/src/main/kotlin/com/vitorpamplona/amethyst/napplethost/NappletBrowserActivity.kt @@ -165,6 +165,12 @@ class NappletBrowserActivity : ComponentActivity() { private var resumed = false + // The back-stack slot the last history record came from. A page that finishes in the same slot of the + // same WebView replaced that page in place (a JS redirect, a rewritten query), so it is the same visit. + private var lastRecordedView: WebView? = null + private var lastRecordedIndex = -1 + private var lastRecordedUrl: String? = null + // The pill, find, console and page dialogs — the shared Compose chrome (see BrowserChromeHost). private var chrome: BrowserChromeHost? = null @@ -840,7 +846,7 @@ class NappletBrowserActivity : ComponentActivity() { if (!mainFrameLoadFailed) hideLoadError() // Record only a clean http(s) main-frame load — never a typed-but-failed address. if (!mainFrameLoadFailed && (url.startsWith("https://") || url.startsWith("http://"))) { - recordHistory(url, view.title) + recordHistory(view, url, view.title) scheduleFaviconSniff(view, url) } } @@ -938,15 +944,23 @@ class NappletBrowserActivity : ComponentActivity() { /** Relays a successfully loaded page to the main-process broker for the device-local visit history. */ private fun recordHistory( + view: WebView, url: String, title: String?, ) { + val index = view.copyBackForwardList().currentIndex + val replaces = lastRecordedUrl?.takeIf { view === lastRecordedView && index == lastRecordedIndex && it != url } + lastRecordedView = view + lastRecordedIndex = index + lastRecordedUrl = url + val msg = Message.obtain(null, NappletIpc.MSG_RECORD_HISTORY).apply { data = Bundle().apply { putString(NappletIpc.KEY_HISTORY_URL, url) putString(NappletIpc.KEY_HISTORY_TITLE, title.orEmpty()) + if (replaces != null) putString(NappletIpc.KEY_HISTORY_REPLACES, replaces) } } queueToBroker(msg) diff --git a/nappletHost/src/main/kotlin/com/vitorpamplona/amethyst/napplethost/NappletIpc.kt b/nappletHost/src/main/kotlin/com/vitorpamplona/amethyst/napplethost/NappletIpc.kt index 389c7144c5..20f31797ab 100644 --- a/nappletHost/src/main/kotlin/com/vitorpamplona/amethyst/napplethost/NappletIpc.kt +++ b/nappletHost/src/main/kotlin/com/vitorpamplona/amethyst/napplethost/NappletIpc.kt @@ -78,7 +78,8 @@ object NappletIpc { /** * Host → broker (browser mode): record a *successfully loaded* page in the device-local visit history - * (main process only). Carries [KEY_HISTORY_URL] (the landed URL) and [KEY_HISTORY_TITLE]. Sent only + * (main process only). Carries [KEY_HISTORY_URL] (the landed URL), [KEY_HISTORY_TITLE] and, when the page + * replaced the previously recorded one in place, [KEY_HISTORY_REPLACES]. Sent only * after a clean main-frame page-finish — never for a typed-but-failed address — so misspellings never * enter history. The `:napplet` process can't touch the main process's store, so it relays it here. */ @@ -189,6 +190,9 @@ object NappletIpc { /** The page title of a successfully loaded browser page, for the visit-history record. */ const val KEY_HISTORY_TITLE = "historyTitle" + /** The previously recorded URL a page replaced in place (a redirect or a rewritten query), if any. */ + const val KEY_HISTORY_REPLACES = "historyReplaces" + /** The host a captured favicon belongs to. */ const val KEY_ICON_HOST = "iconHost"