mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-10-06 03:38:23 +00:00
fix(browser): record a page that replaced itself as one history visit
One DuckDuckGo search left three Recent rows (?q=a%20b, ?q=a+b, ?q=a+b&ia=web): the page rewrote its own query in place and each rewrite finished as a new page. A finish in the same back-stack slot of the same WebView as the last recorded page replaced it, so the browser now sends it as a replacement and the registry moves that visit onto the new URL. Real navigation pushes a new slot and is never folded. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
8ed75761ae
commit
f242aad275
@@ -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
|
||||
}
|
||||
|
||||
|
||||
+19
-1
@@ -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) {
|
||||
|
||||
+66
@@ -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,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
+15
-1
@@ -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)
|
||||
|
||||
@@ -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"
|
||||
|
||||
|
||||
Reference in New Issue
Block a user