From 3a91961fb5495a6fdb1cabc7dca39454628f153b Mon Sep 17 00:00:00 2001 From: GlassOnTin Date: Sun, 23 Aug 2026 21:38:20 +0100 Subject: [PATCH] fix(selection): copy the cells that were highlighted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit getSelectedText decided each row's column bounds with minOf/maxOf on the two anchor columns, while SelectionRange.contains — the predicate that draws the highlight — is anchor-aware: the first row runs from the START anchor's column, the last row up to the END anchor's column. Those agree only when the drag happens to go down and to the right. Drag down and LEFT and they diverge. Selecting from row 0 col 8 to row 1 col 2 highlights "IJ" then "abc", and copied "CDEFGHIJ" then "abcdefghi" — six characters per row the user never selected, silently, with the highlight still showing the smaller region. Uses the existing getStartPosition/getEndPosition helpers, which already encode the anchor-aware rule the highlight uses, so the two now derive the same bounds from one place. The new test asserts the clipboard against the highlight rather than against a hardcoded string: it reads the selected cells straight off SelectionRange.contains and requires getSelectedText to return exactly that. It fails on the old code with the mismatch above, and a second test pins the down-and-right case that already worked. 43 selection tests pass; termlib and feature:terminal suites green. Found while investigating a separate copy report (Haven #581), which this does NOT explain — that one is smart-copy's panel-border stripping. This bug is unreported. --- .../connectbot/terminal/SelectionManager.kt | 14 +++- .../terminal/SelectionManagerTest.kt | 70 +++++++++++++++++++ 2 files changed, 82 insertions(+), 2 deletions(-) diff --git a/lib/src/main/java/org/connectbot/terminal/SelectionManager.kt b/lib/src/main/java/org/connectbot/terminal/SelectionManager.kt index b7344cf8..fa2d9207 100644 --- a/lib/src/main/java/org/connectbot/terminal/SelectionManager.kt +++ b/lib/src/main/java/org/connectbot/terminal/SelectionManager.kt @@ -373,6 +373,16 @@ internal class SelectionManager { val minRow = minOf(range.startRow, range.endRow) val maxRow = maxOf(range.startRow, range.endRow) + // Anchor-aware, so the clipboard matches what SelectionRange.contains + // drew: the first row starts at the anchor the selection began from and + // the last row ends at the one it finished on. Using minOf/maxOf on the + // two columns agreed with the highlight only for a down-and-right drag + // — dragging down and LEFT copied text the user never highlighted (a + // selection from row 0 col 8 to row 1 col 2 highlighted "IJ"/"abc" but + // copied "CDEFGHIJ"/"abcdefghi"). + val (_, selStartCol) = range.getStartPosition() + val (_, selEndCol) = range.getEndPosition() + return buildString { for (row in minRow..maxRow) { // Get line from the appropriate source based on scrollback position @@ -402,11 +412,11 @@ internal class SelectionManager { SelectionMode.CHARACTER, SelectionMode.WORD -> { val startCol = when (row) { - minRow -> minOf(range.startCol, range.endCol) + minRow -> selStartCol else -> 0 } val endCol = when (row) { - maxRow -> maxOf(range.startCol, range.endCol) + maxRow -> selEndCol else -> line.cells.size - 1 } diff --git a/lib/src/test/java/org/connectbot/terminal/SelectionManagerTest.kt b/lib/src/test/java/org/connectbot/terminal/SelectionManagerTest.kt index 6a4d1c33..7977aabd 100644 --- a/lib/src/test/java/org/connectbot/terminal/SelectionManagerTest.kt +++ b/lib/src/test/java/org/connectbot/terminal/SelectionManagerTest.kt @@ -583,4 +583,74 @@ class SelectionManagerTest { range = selectionManager.selectionRange!! assertEquals(11, range.endRow) // Still at 11 } + + /** + * The highlight and the clipboard must agree on which cells are selected. + * + * [SelectionRange.contains] — the predicate that draws the highlight — is + * anchor-aware: the first row runs from the START anchor's column and the + * last row up to the END anchor's column. `getSelectedText` used + * `minOf`/`maxOf` on the two columns instead, which gives the same answer + * only when the drag happens to go down and to the right. + * + * Drag down and LEFT and they disagree. Selecting from row 0 col 8 to + * row 1 col 2 highlights "IJ" then "abc", but the extraction took + * min(8,2)=2 on the first row and max(8,2)=8 on the last, copying + * "CDEFGHIJ" then "abcdefghi" — six characters per row that were never + * highlighted, while the highlight still showed the smaller region. + */ + @Test + fun testSelectedTextMatchesHighlightWhenDraggedDownAndLeft() { + val snapshot = makeTwoLineSnapshot("ABCDEFGHIJ", "abcdefghij") + + selectionManager.startSelection(0, 8, cols = 10, mode = SelectionMode.CHARACTER) + selectionManager.updateSelection(1, 2) + selectionManager.endSelection() + + val range = selectionManager.selectionRange!! + val rowText = listOf("ABCDEFGHIJ", "abcdefghij") + + // Read the highlight straight off the drawing predicate. + val highlighted = (0..1).joinToString("\n") { row -> + (0..9).filter { col -> range.contains(row, col) } + .joinToString("") { col -> rowText[row][col].toString() } + } + assertEquals("IJ\nabc", highlighted) + + // What reaches the clipboard must be exactly that. + assertEquals(highlighted, selectionManager.getSelectedText(snapshot)) + } + + /** The down-and-right drag that already behaved must keep behaving. */ + @Test + fun testSelectedTextMatchesHighlightWhenDraggedDownAndRight() { + val snapshot = makeTwoLineSnapshot("ABCDEFGHIJ", "abcdefghij") + + selectionManager.startSelection(0, 2, cols = 10, mode = SelectionMode.CHARACTER) + selectionManager.updateSelection(1, 8) + selectionManager.endSelection() + + assertEquals("CDEFGHIJ\nabcdefghi", selectionManager.getSelectedText(snapshot)) + } + + private fun makeTwoLineSnapshot(first: String, second: String): TerminalSnapshot { + val cols = maxOf(first.length, second.length) + val lines = listOf(first, second).mapIndexed { i, t -> + TerminalLine(row = i, cells = t.padEnd(cols, ' ').map { cell(it) }) + } + return TerminalSnapshot( + lines = lines, + scrollback = emptyList(), + cursorRow = 1, + cursorCol = 0, + cursorVisible = true, + cursorBlink = true, + cursorShape = CursorShape.BLOCK, + terminalTitle = "", + rows = 2, + cols = cols, + timestamp = System.currentTimeMillis(), + sequenceNumber = 1L, + ) + } }