mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-10-05 19:28:25 +00:00
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PVoAJLF4DkQBZoD54xTv9a
This commit is contained in:
+18
-8
@@ -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
|
||||
|
||||
+21
@@ -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)
|
||||
|
||||
+4
-3
@@ -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
|
||||
|
||||
+8
-2
@@ -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) {
|
||||
|
||||
+1
-1
@@ -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
|
||||
|
||||
+53
-30
@@ -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<TextRange?>(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<Pair<String, List<OmniboxSuggestions.Suggestion>>>(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://"))
|
||||
|
||||
+1
-1
@@ -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
|
||||
|
||||
+11
-6
@@ -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
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user