From e7a0c9643efc99faa93c65cb763460ad6690dcc4 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 15:12:17 +0000 Subject: [PATCH] fix(browser): audit fixes for the omnibox and paste-and-go - OmniboxInput: text with whitespace is a search even if it contains "://", so pasted prose with a link no longer loads as a broken URL. Adds OmniboxInput.isAddress and Resolved.isSearch so the pill icon and the 'Go to' / 'Search for' row use the same rule as resolve(). - Launcher: track the inline completion explicitly. Tab only accepts a real ghost suffix, and select-all no longer empties 'typed' (which flipped the body back to the home grid); typing over a selection still completes. - Launcher: placeholder moved into the BasicTextField decorationBox so screen readers announce it as the field's hint. - Paste and go re-checks the clipboard when the window regains focus, not only when the field does. - Clipboard.hasText is suspend; the JVM actual runs the AWT flavor query on Dispatchers.IO (a blocking X11 round trip). Android only offers text/plain and text/html clips, the ones getText can read. - In-site AddressEditor shows the globe/search glyph by the same rule. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01PVoAJLF4DkQBZoD54xTv9a --- .../amethyst/commons/browser/OmniboxInput.kt | 26 ++++-- .../commons/browser/OmniboxInputTest.kt | 21 +++++ .../components/util/ClipboardExt.android.kt | 7 +- .../commons/browser/ui/pill/AddressEditor.kt | 10 ++- .../ui/components/util/ClipboardExt.kt | 2 +- .../screen/loggedIn/browser/BrowserScreen.kt | 83 ++++++++++++------- .../ui/components/util/ClipboardExt.ios.kt | 2 +- .../ui/components/util/ClipboardExt.jvm.kt | 17 ++-- 8 files changed, 117 insertions(+), 51 deletions(-) diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/browser/OmniboxInput.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/browser/OmniboxInput.kt index ed555546e9..f7ea23d615 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/browser/OmniboxInput.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/browser/OmniboxInput.kt @@ -27,9 +27,9 @@ package com.vitorpamplona.amethyst.commons.browser * * The rules, in order: * - blank → null (nothing to open) - * - already has a scheme (`foo://…`) → used verbatim + * - already has a scheme (`foo://…`) and no spaces → used verbatim * - looks like a host/URL (no spaces, has a dot, or is `localhost`) → `https://` prepended - * - anything else → a search on [searchPrefix] (DuckDuckGo by default), URL-encoded + * - anything else — including prose that merely contains a link → a search on [searchPrefix] (DuckDuckGo by default), URL-encoded * * [Resolved.forceTor] is set for `.onion` addresses, which only resolve over Tor; the caller ORs it with * the user's per-site choice. Pure and platform-agnostic (no `android.net.Uri`) so it lives in commons @@ -42,6 +42,8 @@ object OmniboxInput { data class Resolved( val url: String, val forceTor: Boolean, + /** True when [url] is a search for the text rather than the address the user typed. */ + val isSearch: Boolean = false, ) fun resolve( @@ -50,14 +52,22 @@ object OmniboxInput { ): Resolved? { val text = raw.trim() if (text.isEmpty()) return null - if (text.contains("://")) return Resolved(text, isOnion(text)) - if (looksLikeHost(text)) { - val url = "https://$text" - return Resolved(url, isOnion(url)) - } - return Resolved(searchPrefix + encodeQuery(text), forceTor = false) + if (!isAddress(text)) return Resolved(searchPrefix + encodeQuery(text), forceTor = false, isSearch = true) + val url = if (hasScheme(text)) text else "https://$text" + return Resolved(url, isOnion(url)) } + /** + * True when [raw] would be opened as an address rather than searched for — the one rule [resolve] + * follows, exposed so the address bar's icon and hint can say what Go will do before it happens. + */ + fun isAddress(raw: String): Boolean { + val text = raw.trim() + return hasScheme(text) || looksLikeHost(text) + } + + private fun hasScheme(text: String): Boolean = text.contains("://") && text.none { it.isWhitespace() } + /** * True when [text] (with no scheme) reads as a hostname/URL rather than a search query: no spaces, and * either `localhost` or a dotted host (so `example.com`, `1.2.3.4`, `localhost:8080` are hosts but diff --git a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/browser/OmniboxInputTest.kt b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/browser/OmniboxInputTest.kt index a8dc9d0c1c..65f48b6e80 100644 --- a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/browser/OmniboxInputTest.kt +++ b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/browser/OmniboxInputTest.kt @@ -57,6 +57,27 @@ class OmniboxInputTest { assertEquals("https://duckduckgo.com/?q=cats", OmniboxInput.resolve("cats")?.url) } + @Test + fun textWithSpacesIsASearchEvenWithAScheme() { + // Pasted prose that merely contains a link must not be loaded as a (broken) URL. + val resolved = OmniboxInput.resolve("check this out https://example.com")!! + assertEquals("https://duckduckgo.com/?q=check%20this%20out%20https%3A%2F%2Fexample.com", resolved.url) + assertTrue(resolved.isSearch) + assertTrue(!OmniboxInput.isAddress("check this out https://example.com")) + } + + @Test + fun isAddressMatchesResolve() { + listOf("example.com", "http://example.com", "localhost:8080", "nostr://npub1abc").forEach { + assertTrue(OmniboxInput.isAddress(it), it) + assertTrue(!OmniboxInput.resolve(it)!!.isSearch, it) + } + listOf("cats", "how to tie a knot").forEach { + assertTrue(!OmniboxInput.isAddress(it), it) + assertTrue(OmniboxInput.resolve(it)!!.isSearch, it) + } + } + @Test fun searchPrefixIsConfigurable() { assertEquals("https://search.example/?s=cats", OmniboxInput.resolve("cats", "https://search.example/?s=")?.url) diff --git a/commonsUI/src/androidMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/util/ClipboardExt.android.kt b/commonsUI/src/androidMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/util/ClipboardExt.android.kt index cbae749d38..a6d91595ae 100644 --- a/commonsUI/src/androidMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/util/ClipboardExt.android.kt +++ b/commonsUI/src/androidMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/util/ClipboardExt.android.kt @@ -37,8 +37,9 @@ actual suspend fun Clipboard.getText(): String? = ?.toString() // The description is metadata: reading it doesn't trigger the "pasted from your clipboard" toast that -// getPrimaryClip does. -actual fun Clipboard.hasText(): Boolean = +// getPrimaryClip does. Only the types whose items carry the text getText reads: a text/uri-list clip +// (ClipData.newUri) has none, so offering to paste it would do nothing. +actual suspend fun Clipboard.hasText(): Boolean = nativeClipboard.primaryClipDescription?.let { - it.hasMimeType(ClipDescription.MIMETYPE_TEXT_PLAIN) || it.hasMimeType("text/*") + it.hasMimeType(ClipDescription.MIMETYPE_TEXT_PLAIN) || it.hasMimeType(ClipDescription.MIMETYPE_TEXT_HTML) } == true diff --git a/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/browser/ui/pill/AddressEditor.kt b/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/browser/ui/pill/AddressEditor.kt index 3175737aec..f453f01c07 100644 --- a/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/browser/ui/pill/AddressEditor.kt +++ b/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/browser/ui/pill/AddressEditor.kt @@ -63,6 +63,7 @@ import androidx.compose.ui.text.input.TextFieldValue import androidx.compose.ui.text.style.TextOverflow import androidx.compose.ui.unit.dp import com.vitorpamplona.amethyst.commons.browser.BrowserChrome +import com.vitorpamplona.amethyst.commons.browser.OmniboxInput import com.vitorpamplona.amethyst.commons.icons.symbols.Icon import com.vitorpamplona.amethyst.commons.icons.symbols.MaterialSymbols import com.vitorpamplona.amethyst.commons.resources.Res @@ -119,11 +120,16 @@ fun AddressEditor( verticalAlignment = Alignment.CenterVertically, ) { // The field shows what it will do: the page's badge while it still holds the page's URL, - // a search glyph once the user types something else. + // then a globe or a search glyph for what Go will do with what they typed. if (field.text == initialUrl) { SecurityIcon(security, size = 20.dp) } else { - Icon(MaterialSymbols.Search, contentDescription = null, modifier = Modifier.size(20.dp), tint = MaterialTheme.colorScheme.onSurfaceVariant) + Icon( + if (OmniboxInput.isAddress(field.text)) MaterialSymbols.Language else MaterialSymbols.Search, + contentDescription = null, + modifier = Modifier.size(20.dp), + tint = MaterialTheme.colorScheme.onSurfaceVariant, + ) } Spacer(Modifier.width(10.dp)) Box(Modifier.weight(1f), contentAlignment = Alignment.CenterStart) { diff --git a/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/util/ClipboardExt.kt b/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/util/ClipboardExt.kt index 2a732fa0b7..0f2ecee75d 100644 --- a/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/util/ClipboardExt.kt +++ b/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/util/ClipboardExt.kt @@ -32,4 +32,4 @@ expect suspend fun Clipboard.getText(): String? * Whether the clipboard holds text, answered from its metadata without reading the contents — so offering * "Paste and go" neither makes Android announce a paste nor iOS prompt for one. Only the tap should read. */ -expect fun Clipboard.hasText(): Boolean +expect suspend fun Clipboard.hasText(): Boolean diff --git a/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/screen/loggedIn/browser/BrowserScreen.kt b/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/screen/loggedIn/browser/BrowserScreen.kt index f1bd5add78..18009f4d45 100644 --- a/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/screen/loggedIn/browser/BrowserScreen.kt +++ b/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/screen/loggedIn/browser/BrowserScreen.kt @@ -80,6 +80,7 @@ import androidx.compose.ui.input.key.onPreviewKeyEvent import androidx.compose.ui.input.key.type import androidx.compose.ui.platform.LocalClipboard import androidx.compose.ui.platform.LocalFocusManager +import androidx.compose.ui.platform.LocalWindowInfo import androidx.compose.ui.text.TextRange import androidx.compose.ui.text.font.FontWeight import androidx.compose.ui.text.input.ImeAction @@ -197,15 +198,24 @@ private fun BrowserLauncher( val searchEngine = SearchEngines.byId(searchEngineId) var field by remember { mutableStateOf(TextFieldValue("")) } + + // The selection an inline completion produced. While the field's selection is still exactly this, the + // highlighted suffix is ghost text the user didn't type; any other selection (select-all, a shift-select) + // is the user's own. + var ghost by remember { mutableStateOf(null) } + val ghostShowing = ghost != null && field.selection == ghost + var focused by remember { mutableStateOf(false) } val focusManager = LocalFocusManager.current val clipboard = LocalClipboard.current val scope = rememberCoroutineScope() - // Checked from the clipboard's metadata each time the field gains focus, so "Paste and go" is only - // offered when there is something to paste; the contents are read only when the user taps it. + // Checked from the clipboard's metadata whenever the field gains focus or the window comes back (the + // user may have left to copy a link), so "Paste and go" is only offered when there is something to + // paste; the contents are read only when the user taps it. + val windowFocused = LocalWindowInfo.current.isWindowFocused var clipboardHasText by remember { mutableStateOf(false) } - LaunchedEffect(focused) { clipboardHasText = focused && clipboard.hasText() } + LaunchedEffect(focused, windowFocused) { clipboardHasText = focused && windowFocused && clipboard.hasText() } // Favorites + visit history + the hardcoded Discover apps, flattened into the neutral candidate shape // the ranker consumes — so typing the omnibox finds a suggested app even before its first visit. The @@ -262,9 +272,8 @@ private fun BrowserLauncher( value = withContext(Dispatchers.Default) { nappletNotes.toDiscoverApps(nappletFollows::matchAuthor, favoriteCoordinates) } } - // What the user actually typed, excluding any selected ghost-completion suffix (selection.min is the - // caret when collapsed, or the start of the highlighted suffix when a completion is showing). - val typed = field.text.take(field.selection.min.coerceIn(0, field.text.length)) + // What the user actually typed, excluding the ghost-completion suffix while one is showing. + val typed = ghost?.takeIf { ghostShowing }?.let { field.text.take(it.start) } ?: field.text // One ranking per typed text: an appended character is ranked in onValueChange (for the inline // completion) and then again for this list on the recomposition that follows — keep the last one. val lastRanking = remember(candidates) { arrayOfNulls>>(1) } @@ -281,6 +290,7 @@ private fun BrowserLauncher( // The site opens in its own window: coming back should land on a fresh launcher, not on the // half-typed address and its suggestions. field = TextFieldValue("") + ghost = null focusManager.clearFocus() } @@ -300,23 +310,27 @@ private fun BrowserLauncher( // Inline autocomplete: when the user appends a character, offer the top host as selected ghost text so // the next keystroke replaces it. On deletion or mid-string edits, leave the value untouched. fun onValueChange(new: TextFieldValue) { - val prevTyped = field.text.take(field.selection.min.coerceIn(0, field.text.length)) + // What is left once the selection (a ghost suffix, or anything the user selected) is typed over. + val kept = field.text.removeRange(field.selection.min, field.selection.max) val newText = new.text val appended = new.selection.collapsed && new.selection.start == newText.length && - newText.length > prevTyped.length && - newText.startsWith(prevTyped) + newText.length > kept.length && + newText.startsWith(kept) if (appended) { // The completion has always looked at the top 8 (rank's default limit). val completion = OmniboxSuggestions.completion(newText, ranked(newText).take(8)) if (completion != null) { // Keep the user's own casing for the typed prefix; append only the remaining suffix. val full = newText + completion.substring(newText.length) - field = TextFieldValue(full, TextRange(newText.length, full.length)) + val suffix = TextRange(newText.length, full.length) + field = TextFieldValue(full, suffix) + ghost = suffix return } } + if (new.text != field.text) ghost = null field = new } @@ -328,8 +342,15 @@ private fun BrowserLauncher( focused = focused, onFocusChange = { focused = it }, onValueChange = ::onValueChange, - onAcceptCompletion = { field = field.copy(selection = TextRange(field.text.length)) }, - onClear = { field = TextFieldValue("") }, + hasCompletion = ghostShowing, + onAcceptCompletion = { + field = field.copy(selection = TextRange(field.text.length)) + ghost = null + }, + onClear = { + field = TextFieldValue("") + ghost = null + }, onOpen = { open(field.text) }, onPasteAndGo = if (focused && field.text.isEmpty() && clipboardHasText) { @@ -401,6 +422,7 @@ private fun OmniBar( focused: Boolean, onFocusChange: (Boolean) -> Unit, onValueChange: (TextFieldValue) -> Unit, + hasCompletion: Boolean, onAcceptCompletion: () -> Unit, onClear: () -> Unit, onOpen: () -> Unit, @@ -409,7 +431,7 @@ private fun OmniBar( val focusRequester = remember { FocusRequester() } val focusManager = LocalFocusManager.current val colors = MaterialTheme.colorScheme - val isAddress = isAddress(field.text) + val isAddress = OmniboxInput.isAddress(field.text) Row( modifier = @@ -447,15 +469,6 @@ private fun OmniBar( ) Spacer(Modifier.width(12.dp)) Box(Modifier.weight(1f), contentAlignment = Alignment.CenterStart) { - if (field.text.isEmpty()) { - Text( - stringRes(Res.string.browser_address_hint), - style = MaterialTheme.typography.bodyLarge, - color = colors.onSurfaceVariant, - maxLines = 1, - overflow = TextOverflow.Ellipsis, - ) - } BasicTextField( value = field, onValueChange = onValueChange, @@ -470,6 +483,22 @@ private fun OmniBar( imeAction = ImeAction.Go, ), keyboardActions = KeyboardActions(onGo = { onOpen() }), + // The placeholder lives inside the field's decoration so it is part of the field's + // semantics: a screen reader announces it as the field's hint, as Material's TextField does. + decorationBox = { innerTextField -> + Box(contentAlignment = Alignment.CenterStart) { + if (field.text.isEmpty()) { + Text( + stringRes(Res.string.browser_address_hint), + style = MaterialTheme.typography.bodyLarge, + color = colors.onSurfaceVariant, + maxLines = 1, + overflow = TextOverflow.Ellipsis, + ) + } + innerTextField() + } + }, modifier = Modifier .fillMaxWidth() @@ -480,7 +509,7 @@ private fun OmniBar( when (event.key) { // Tab takes the ghost completion instead of moving focus away. Key.Tab -> - if (!field.selection.collapsed) { + if (hasCompletion) { onAcceptCompletion() true } else { @@ -516,12 +545,6 @@ private fun OmniBar( } } -/** True when Go would open [text] as an address rather than search for it — the same rule [OmniboxInput.resolve] uses. */ -private fun isAddress(text: String): Boolean { - val trimmed = text.trim() - return trimmed.contains("://") || OmniboxInput.looksLikeHost(trimmed) -} - /** * The first row while typing: exactly what Go / Enter will do with the [entered] text — open it as an * address, or search for it — so the user never has to guess which one they're about to get. @@ -533,7 +556,7 @@ private fun EnteredRow( onClick: () -> Unit, ) { val target = OmniboxInput.resolve(entered, searchEngine.queryPrefix) ?: return - val isAddress = isAddress(entered) + val isAddress = !target.isSearch val title = if (isAddress) { stringRes(Res.string.browser_omnibox_go_to, target.url.removePrefix("https://")) diff --git a/commonsUI/src/iosMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/util/ClipboardExt.ios.kt b/commonsUI/src/iosMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/util/ClipboardExt.ios.kt index a76114a99c..d16cd4ac99 100644 --- a/commonsUI/src/iosMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/util/ClipboardExt.ios.kt +++ b/commonsUI/src/iosMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/util/ClipboardExt.ios.kt @@ -35,4 +35,4 @@ actual suspend fun Clipboard.getText(): String? { return if (pasteboard.hasStrings) pasteboard.string else null } -actual fun Clipboard.hasText(): Boolean = UIPasteboard.generalPasteboard.hasStrings +actual suspend fun Clipboard.hasText(): Boolean = UIPasteboard.generalPasteboard.hasStrings diff --git a/commonsUI/src/jvmMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/util/ClipboardExt.jvm.kt b/commonsUI/src/jvmMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/util/ClipboardExt.jvm.kt index e1d23c0ffe..7649ef9b39 100644 --- a/commonsUI/src/jvmMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/util/ClipboardExt.jvm.kt +++ b/commonsUI/src/jvmMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/util/ClipboardExt.jvm.kt @@ -23,6 +23,8 @@ package com.vitorpamplona.amethyst.commons.ui.components.util import androidx.compose.ui.ExperimentalComposeUiApi import androidx.compose.ui.platform.ClipEntry import androidx.compose.ui.platform.Clipboard +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.withContext import java.awt.Toolkit import java.awt.datatransfer.DataFlavor import java.awt.datatransfer.StringSelection @@ -44,10 +46,13 @@ actual suspend fun Clipboard.getText(): String? { } } -actual fun Clipboard.hasText(): Boolean = - try { - Toolkit.getDefaultToolkit().systemClipboard.isDataFlavorAvailable(DataFlavor.stringFlavor) - } catch (_: Exception) { - // Headless, or another app holds the clipboard open. - false +// Off the UI thread: on X11 the flavor query is a round trip to whichever app owns the clipboard. +actual suspend fun Clipboard.hasText(): Boolean = + withContext(Dispatchers.IO) { + try { + Toolkit.getDefaultToolkit().systemClipboard.isDataFlavorAvailable(DataFlavor.stringFlavor) + } catch (_: Exception) { + // Headless, or another app holds the clipboard open. + false + } }