From 190818910076746de05f857f1ef09be644aa359a Mon Sep 17 00:00:00 2001 From: Laurens Bezemer | DodoTech Date: Tue, 25 Aug 2026 14:46:36 +0200 Subject: [PATCH] Make the terminal's right button copy and paste, and never open a menu MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Selecting output and right-clicking it opened the WebView's own context menu with Copy on it, which is correct behaviour for a web page and wrong for a terminal. What a terminal's right button has meant since the Windows console got QuickEdit, and since PuTTY shipped, is this: with something selected it copies and drops the selection, with nothing selected it pastes. No menu, no second click. The gesture can only live in the page. The selection is drawn into a canvas the host never sees, and once a terminal has focus the host's window receives no input events at all — the same reason the Ctrl+plus chords are handled there. ◆ THE CLIPBOARD IS THE HOST'S IN BOTH DIRECTIONS, AND THE READING IS WHY. navigator.clipboard.readText() is gated behind the clipboard-read permission in every Chromium, and an embedded WebView with no PermissionRequested handler answers a permission request by putting a prompt on the screen. Trading a context menu for a permission dialog is not this change. So the page asks and the host answers, exactly as requestFontSizeStep does. Writing does not go the other way either, although navigator.clipboard.writeText would have worked from the page on Windows: a clipboard that is the host's in one direction and the page's in the other is two mechanisms to keep working across WebView2, WebKitGTK and Android's WebView. The host has owned a clipboard delegate since the keychain grew its COPY buttons, so each App.axaml.cs grows a ClipboardReader beside its ClipboardWriter and MainWindowViewModel takes it. Three opcodes: client ClipboardCopy and ClipboardPaste, server Clipboard. ◆ Clipboard IS NOT Paste, AND THE DIFFERENCE IS THE BRACKETING. Paste carries a snippet off the sidebar, and the page deliberately *types* a single-line one rather than bracketing it, because bash marks a bracketed paste as an active region and a snippet should read as though it had been typed. A clipboard paste is the case those markers are for — readline uses them to decide it has been pasted into — so Clipboard is always bracketed and never followed by an Enter. One opcode for both would have needed a flag meaning "which of the two kinds of paste is this", on a payload whose existing flag byte means something else. Both client frames are dispatched before the session lookup, which is what keeps the gesture working in a pane whose shell has just ended: squinting at the message a remote left behind is exactly when somebody copies the line. Dispatch was past MA0051's length once it grew a second such branch, so the frames no pump needs to see moved into Announce() and the font-size chord went with them. ◆ CAPTURED AND STOPPED, RATHER THAN MERELY PREVENTED. preventDefault alone suppresses the menu and still lets xterm's own contextmenu handler run, which moves its helper textarea under the pointer at z-index 1000 and leaves the selection sitting in it, selected. Both of those exist to serve the native menu. Taking the event in the capture phase on the pane and stopping it there is what keeps the gesture wholly the page's. rightClickSelectsWord is now false, which on macOS is a change of default rather than a restatement of it: xterm sets it true there alone, so a right-click would have selected the word under the pointer and copied that instead of pasting — wrong on one platform only, which is the worst way for it to be wrong. The renderer harness grows the pane it has been inviting since RendererReconnectionTests: a fake Terminal, the two addons, and a real UTF-8 codec, because both directions run through the page's own TextEncoder and TextDecoder and a Latin-1 stub would pass a test with an em dash in the selection. Its warning about declaring names terminal.js declares now covers the half that does not SyntaxError — a harness frame() was silently replaced by the page's frame() and failed six tests on a type error a long way from the cause. Manual checks 1.11 and 11.13 are what is left for a person: that a real contextmenu event reaches a capture listener, that no native menu is drawn once it is prevented, and that xterm's real paste() brackets. 11.13 also records what is unverified — whether Android's WebView delivers a pointer's secondary click as a contextmenu event at all. ClipboardReader is wired on that head regardless, because the alternative is a head where the paste half silently does nothing and nobody finds out; that head is not compiled here either, this machine having no android workload. Co-Authored-By: Claude Opus 5 --- docs/manual-checks.md | 70 ++++ src/DodoSSH.Client.Android/App.axaml.cs | 32 +- src/DodoSSH.Client.App/App.axaml.cs | 12 + .../ViewModels/MainWindowViewModel.cs | 119 ++++++ .../WebAssets/terminal.js | 120 ++++++ .../WebAssets/vendor/README.md | 10 +- .../TerminalDataPlane.cs | 80 +++- src/DodoSSH.Client.Terminal/TerminalFrame.cs | 71 ++++ .../TerminalWorkspace.cs | 95 +++++ .../DodoSSH.Client.App.Tests/FakeRenderer.cs | 57 +++ .../ShellFlowTests.cs | 97 +++++ .../Renderer/renderer-harness.js | 347 +++++++++++++++++- .../RendererClipboardTests.cs | 276 ++++++++++++++ .../RendererPage.cs | 54 +++ .../TerminalDataPlaneTests.cs | 76 ++++ .../TerminalWorkspaceTests.cs | 63 ++++ 16 files changed, 1557 insertions(+), 22 deletions(-) create mode 100644 tests/DodoSSH.Client.Terminal.Tests/RendererClipboardTests.cs diff --git a/docs/manual-checks.md b/docs/manual-checks.md index ca62099..6886b5f 100644 --- a/docs/manual-checks.md +++ b/docs/manual-checks.md @@ -197,6 +197,56 @@ That is what the "is this still the page's socket" guard in `connect()` exists t input while the banner is *clear* is neither: the socket is open and dead, which nothing on this page can currently see — see the note at the end of 11.12a. +### 1.11 The right button copies and pastes, and never opens a menu + +Open a shell and run something that prints a few lines — `ls -la /etc`. Drag a selection across part of the +output with the left button, then **right-click inside the selection**. + +**Pass:** the highlight disappears at once, **no menu appears anywhere**, and the text is on the clipboard — +paste it into Notepad, or into the connect box, to be sure. The pane still takes typing straight afterwards. + +Now **right-click again with nothing selected.** + +**Pass:** the clipboard's text appears at the prompt as one insertion and **Enter is not pressed** — it sits +there waiting. On bash it will be highlighted in reverse video until the next keystroke, which is right: that +is bash marking a bracketed paste, and it is how you can see the markers arrived. + +**And the multi-line case, which is the one worth the most care.** Copy three separate lines out of a text +editor — `cd /tmp`, `pwd`, `ls` — and right-click in the terminal. + +**Pass:** all three lines land at the prompt as one thing being edited and **nothing runs** until Enter is +pressed. Three commands executing the moment you right-click is the failure this is looking for. + +**On a Mac, one more:** right-click over a word with nothing selected and confirm it pastes rather than +selecting the word under the pointer. xterm.js defaults `rightClickSelectsWord` to true on macOS alone; +`terminal.js` turns it off, and this is the one platform where forgetting to would show. + +**What the tests already own.** `RendererClipboardTests` runs `terminal.js` itself and proves which frame a +right-click sends, which session it is addressed to, that the selection is dropped after a copy and that the +answer comes back through `paste()` rather than `input()`. `ShellFlowTests` proves the round trip through the +host's own clipboard. What is left here is the WebView: that a real `contextmenu` event reaches a listener +registered in the capture phase, that no native menu is drawn once it is prevented, and that xterm's real +`paste()` brackets the text — none of which a Jint engine can answer. + +**Failure means:** + +- **A context menu appears.** The event is not being prevented, or not being seen. `handleRightClick` takes + it in the capture phase on the pane; a listener registered without the third argument to + `addEventListener` would still prevent the menu, so a menu means the listener is not attached at all. +- **A permission prompt appears** — *"…wants to see text and images copied to the clipboard"*. Somebody has + routed the read through `navigator.clipboard.readText()`, which is exactly what the `ClipboardPaste` + opcode exists to avoid. The clipboard is the host's. +- **A drag started exactly where you last right-clicked selects nothing.** xterm's own `contextmenu` + handler ran: it parks its invisible helper textarea under the pointer, 20 pixels square at z-index 1000, + to serve the native menu. The page stops the event as well as preventing it, and `stopPropagation` is the + half that was left out. Subtle enough to be worth looking for on purpose rather than waiting to notice. +- **Three commands run on a multi-line paste.** The text went through `input()` rather than `paste()`, so + bracketed paste never happened and the shell read each newline as "run this". That is the whole reason the + `Clipboard` opcode is separate from `Paste`; see `TerminalFrame.cs`. +- **Nothing happens at all, in either direction.** The head is not passing a clipboard. Both pass one — see + `ClipboardWriter` and `ClipboardReader` in each `App.axaml.cs` — and the view model is deliberately silent + when neither is there, because there is no status line on this screen to say it on. + --- ## Phase 2 — Known Hosts as its own page @@ -1993,6 +2043,26 @@ without either end noticing, which a suspended renderer can leave behind — sho connected and swallows what is typed. If that is ever seen, it is a different bug from this one and needs a liveness probe rather than a faster retry. +### 11.13 The right button on a phone with a pointer · **needs a mouse, a trackpad or a dock** + +The same page as 1.11 runs on this head, so the gesture is here too — but only for a device that has a +secondary click at all. Attach a mouse or use a keyboard case's trackpad, or dock the phone, open a shell, +select some output and right-click it; then right-click again with nothing selected. + +**Pass:** exactly as 1.11 — the selection drops with no menu, and the second click pastes. + +**Unverified, and that is why this check exists.** Nothing here has been run against Android's WebView with +a pointer attached, so whether it delivers a secondary click as a DOM `contextmenu` event at all is an open +question. `ClipboardReader` is wired on this head regardless, because the alternative is a head where the +paste half silently does nothing and nobody finds out. + +**Failure means:** if the copy works and the paste does not, the clipboard read is being refused — Android +returns nothing from the clipboard to an app that is not the foreground one, and a WebView taking the +secondary click may be leaving Avalonia's window in a state where `TopLevel` has none. If neither works, +Android is not delivering the click as a `contextmenu` event, and the gesture needs a native listener on the +`WebView` instead of a page one. Neither costs this head anything it has today: a phone without a pointer +has no right button to miss. + --- ## Phase 12 — Shared vaults: the operations that span two accounts diff --git a/src/DodoSSH.Client.Android/App.axaml.cs b/src/DodoSSH.Client.Android/App.axaml.cs index 91f4ce3..ad53d6b 100644 --- a/src/DodoSSH.Client.Android/App.axaml.cs +++ b/src/DodoSSH.Client.Android/App.axaml.cs @@ -86,7 +86,13 @@ public sealed partial class DodoSshApp : Avalonia.Application var shell = new PhoneShell(); var viewModel = ComposeShell( - paths, caches, workspace, knownHosts, connections, ClipboardWriter(shell)); + paths, + caches, + workspace, + knownHosts, + connections, + ClipboardWriter(shell), + ClipboardReader(shell)); shell.DataContext = viewModel; @@ -176,6 +182,26 @@ public sealed partial class DodoSshApp : Avalonia.Application } }; + /// Reads this phone's clipboard. + /// + /// + /// The other direction, looked up per call for the same reason and with the same fall-through. One + /// caller: the terminal's right-click paste, which the renderer cannot serve itself — see + /// TerminalClientOpcode.ClipboardPaste. + /// + /// + /// Wired on this head although a phone has no right button, and not for symmetry: this head runs the same + /// renderer page, and a device with a mouse, a trackpad case or a desktop dock has a secondary click. + /// Whether Android's WebView delivers one as a DOM contextmenu event is unverified — see + /// docs/manual-checks.md 11.13 — and leaving this out in the meantime would make the paste half + /// the one thing that worked on one head and silently did nothing on the other. + /// + /// + private static Func> ClipboardReader(Visual surface) => + async () => TopLevel.GetTopLevel(surface) is { Clipboard: { } clipboard } + ? await clipboard.TryGetTextAsync().ConfigureAwait(false) + : null; + /// /// Split from only for length. The division is a real one though: above this is /// the platform graph, and below it is the shell every head shares. @@ -186,7 +212,8 @@ public sealed partial class DodoSshApp : Avalonia.Application TerminalWorkspace workspace, VaultKnownHostStore knownHosts, SshNetConnectionFactory connections, - Func copyToClipboard) + Func copyToClipboard, + Func> readClipboard) { // Difference 3: the Android keystore, with a fingerprint or the device credential releasing the // key. A straight implementation of the interface the session layer has always taken. @@ -236,6 +263,7 @@ public sealed partial class DodoSshApp : Avalonia.Application deviceName: PhoneEnvironment.DeviceName, copyToClipboard: copyToClipboard, + readClipboard: readClipboard, updates: updates); diff --git a/src/DodoSSH.Client.App/App.axaml.cs b/src/DodoSSH.Client.App/App.axaml.cs index 090b3c4..69e3e85 100644 --- a/src/DodoSSH.Client.App/App.axaml.cs +++ b/src/DodoSSH.Client.App/App.axaml.cs @@ -72,6 +72,17 @@ internal sealed partial class DodoSshApp : Application } }; + /// Reads this machine's clipboard. + /// + /// The other direction, looked up per call for the same reason and with the same fall-through. One + /// caller: the terminal's right-click paste, which the renderer cannot serve itself — see + /// TerminalClientOpcode.ClipboardPaste. + /// + private static Func> ClipboardReader(IClassicDesktopStyleApplicationLifetime desktop) => + async () => TopLevel.GetTopLevel(desktop.MainWindow) is { Clipboard: { } clipboard } + ? await clipboard.TryGetTextAsync().ConfigureAwait(false) + : null; + /// The terminal workspace, with its loopback listener already up. /// /// Extracted so that constructing it and starting it cannot drift apart: the data plane's socket has to @@ -169,6 +180,7 @@ internal sealed partial class DodoSshApp : Application .ConfigureAwait(false), copyToClipboard: ClipboardWriter(desktop), + readClipboard: ClipboardReader(desktop), updates: updates, openInEditor: OpenInDefaultEditorAsync, watchFile: WatchFile); diff --git a/src/DodoSSH.Client.Shell/ViewModels/MainWindowViewModel.cs b/src/DodoSSH.Client.Shell/ViewModels/MainWindowViewModel.cs index 9ef7a75..f1b7610 100644 --- a/src/DodoSSH.Client.Shell/ViewModels/MainWindowViewModel.cs +++ b/src/DodoSSH.Client.Shell/ViewModels/MainWindowViewModel.cs @@ -328,6 +328,15 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp /// private readonly Func? copyToClipboard; + /// + /// The other direction, and it exists for one caller: a right-click in a terminal with nothing selected. + /// The renderer cannot read a clipboard — says why — + /// so the gesture crosses the socket and is answered here. Null on a head that wires none, which is now + /// no head at all: it stays nullable because this class has to be constructible without a visual, which + /// is what every test that drives it does. + /// + private readonly Func>? readClipboard; + /// private readonly Action post; @@ -493,6 +502,10 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp /// absence of a line rather than by a line somebody has to remember to keep a no-op; and ADR 0011 settles /// the Android head's distribution separately, so it must never acquire one by accident. /// + /// + /// Reads this machine's clipboard, or null on a head that cannot. Answers the terminal's right-click + /// paste and nothing else — see the field it is kept in. + /// /// /// Opens a local path with whatever this machine opens that kind of file with, or null on a head that /// has no such notion. Forwarded straight to , which is the only thing @@ -527,6 +540,7 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp Argon2Profile? passphraseProfile = null, ResumeHandler? resume = null, Func? copyToClipboard = null, + Func>? readClipboard = null, string? deviceName = null, IUpdateChannel? updates = null, Action? post = null, @@ -545,6 +559,7 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp this.clock = clock; this.passphraseProfile = passphraseProfile; this.copyToClipboard = copyToClipboard; + this.readClipboard = readClipboard; // Whitespace is treated as absent rather than honoured: an empty device name reaches the server as a // blank one, which RegisterDevice rejects, and a phone whose model string came back empty would fail @@ -575,6 +590,8 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp // list. Detached in DisposeAsync, which is the only point either of them ends. this.workspace.SessionEnded += OnWorkspaceSessionEnded; this.workspace.FontSizeStepRequested += OnFontSizeStepRequested; + this.workspace.ClipboardCopyRequested += OnClipboardCopyRequested; + this.workspace.ClipboardPasteRequested += OnClipboardPasteRequested; this.workspace.RendererReattached += OnRendererReattached; settings = new ClientSettingsStore(paths); @@ -726,6 +743,106 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp private void OnFontSizeStepRequested(object? sender, TerminalFontSizeStepEventArgs e) => Dispatcher.UIThread.Post(() => StepTerminalFontSize(e.Step)); + /// + /// A right-click over a selection: the page has sent the text, and this puts it on the clipboard. + /// + /// + /// + /// Marshalled for the reason is — this arrives on the data plane's + /// receive loop — plus one of its own, which is the stronger: an Avalonia clipboard is reached through a + /// TopLevel, and a TopLevel is only safe to touch from the thread that owns the window. + /// + /// + /// Through rather than Dispatcher.UIThread directly, unlike the handlers + /// beside it: those predate the delegate and are the ones this shell's suite records as out of its + /// reach. There is no reason to add another. + /// + /// + private void OnClipboardCopyRequested(object? sender, TerminalClipboardCopyEventArgs e) => + post(() => _ = CopyFromTerminalAsync(e.Text)); + + /// + /// A right-click with nothing selected: reads the clipboard and hands it to the pane that asked. + /// + /// Marshalled for the reasons gives. + private void OnClipboardPasteRequested(object? sender, TerminalClipboardPasteEventArgs e) => + post(() => _ = PasteIntoTerminalAsync(e.SessionId)); + + /// + /// + /// Silence where there is no clipboard, and that is a different answer from the keychain's COPY buttons + /// — which say so, because a button that appears to do nothing is worse than a refusal. There is nowhere + /// to say it from here: the shell's status line is drawn on the enroll, sign-in and unlock cards, none + /// of which is on screen while somebody is right-clicking a terminal. Inventing a channel for a branch + /// no shipped head reaches would be the wrong trade, and both heads wire a clipboard. + /// + /// + /// A throw is swallowed rather than left to become an unobserved task exception. A clipboard is a shared + /// machine resource — on Windows another process holding it open is an ordinary, transient failure — and + /// a copy that did not happen is worth exactly one more right-click. + /// + /// + private async Task CopyFromTerminalAsync(string text) + { + if (copyToClipboard is null) + { + return; + } + + try + { + await copyToClipboard(text).ConfigureAwait(true); + } + catch (Exception) + { + // Deliberately silent; see the remark for both halves of why. Nothing is left half done — the + // clipboard either took the text or did not. + } + } + + /// + /// + /// An empty clipboard ends here rather than at the renderer. Sending it would bracket nothing and hand + /// the remote an empty paste, which is a keystroke nobody made; "there was nothing to paste" is a fact + /// about the machine rather than about this terminal, and the honest answer to it is that the + /// right-click did nothing — which is what every other terminal does with an empty clipboard too. + /// + /// + /// The answer for a session whose shell has ended is discarded for the reason + /// gives for refusing it: nothing visible would + /// happen either way, and there is nowhere on this screen to say so. + /// + /// + /// A throw is swallowed, as in and for the same reason. + /// + /// + private async Task PasteIntoTerminalAsync(uint sessionId) + { + if (readClipboard is null) + { + return; + } + + try + { + var text = await readClipboard().ConfigureAwait(true); + + if (string.IsNullOrEmpty(text)) + { + return; + } + + await workspace + .SendClipboardAsync(sessionId, text, CancellationToken.None) + .ConfigureAwait(true); + } + catch (Exception) + { + // Deliberately silent; see the remark. The read and the send are both inside it, because + // whichever of the two failed leaves the same nothing on screen. + } + } + /// /// /// Marshalled for the same reason as : this arrives on the data @@ -3203,6 +3320,8 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp workspace.SessionEnded -= OnWorkspaceSessionEnded; workspace.FontSizeStepRequested -= OnFontSizeStepRequested; + workspace.ClipboardCopyRequested -= OnClipboardCopyRequested; + workspace.ClipboardPasteRequested -= OnClipboardPasteRequested; workspace.RendererReattached -= OnRendererReattached; transfers.PropertyChanged -= OnTransfersPropertyChanged; diff --git a/src/DodoSSH.Client.Shell/WebAssets/terminal.js b/src/DodoSSH.Client.Shell/WebAssets/terminal.js index 20b3e85..50c91b7 100644 --- a/src/DodoSSH.Client.Shell/WebAssets/terminal.js +++ b/src/DodoSSH.Client.Shell/WebAssets/terminal.js @@ -33,11 +33,14 @@ const SERVER_SESSION_ACTIVATED = 4; const SERVER_SESSION_REMOVED = 5; const SERVER_PASTE = 6; const SERVER_FONT_SIZE = 7; +const SERVER_CLIPBOARD = 8; const CLIENT_INPUT = 1; const CLIENT_ACKNOWLEDGE = 2; const CLIENT_RESIZE = 3; const CLIENT_FONT_SIZE_STEP = 4; +const CLIENT_CLIPBOARD_COPY = 5; +const CLIENT_CLIPBOARD_PASTE = 6; const HEADER_LENGTH = 5; const SCROLLBACK_LINES = 5000; @@ -279,6 +282,66 @@ function handleKey(event) { return true; } +/* + ── THE RIGHT-CLICK IS A COPY OR A PASTE, AND NEVER A MENU ─────────────────────────────────────────── + + What a terminal's right button has meant since the Windows console got QuickEdit and since PuTTY + shipped: with something selected it copies and drops the selection, with nothing selected it pastes. + No menu, no second click. This page used to answer with the WebView's own context menu — xterm stuffs + its helper textarea with the selection on contextmenu precisely so that the browser's Copy entry + works — which is correct behaviour for a web page and wrong for a terminal. + + ◆ THE CLIPBOARD IS THE HOST'S, IN BOTH DIRECTIONS, AND THE READING IS WHY. + + navigator.clipboard.readText() is gated behind the clipboard-read permission in every Chromium, and an + embedded WebView with no PermissionRequested handler answers a permission request by putting a prompt + on the screen. Trading a context menu for a permission dialog is not this change. So the page asks and + the host answers, exactly as the font-size chords do — see requestFontSizeStep. Writing would have + worked from here on Windows, and does not go that way either: a clipboard that is the host's in one + direction and the page's in the other is two mechanisms to keep working across WebView2, WebKitGTK and + Android's WebView. + + ◆ CAPTURED AND STOPPED, RATHER THAN MERELY PREVENTED. + + preventDefault alone would suppress the menu and still let xterm's own contextmenu handler run, which + moves the helper textarea under the pointer at z-index 1000 and leaves the selection sitting in it, + selected. Both of those exist to serve the native menu; with no menu they are an invisible textarea + floating over the grid and a DOM selection nothing will clear. Taking the event in the capture phase + and stopping it there is what keeps the gesture wholly ours. + + rightClickSelectsWord is off for the same reason from the other side — see createSession. +*/ +function handleRightClick(event, sessionId) { + event.preventDefault(); + event.stopPropagation(); + + const session = sessions.get(sessionId); + + if (!session) { + return; + } + + const selection = session.term.getSelection(); + + if (selection.length > 0) { + send(CLIENT_CLIPBOARD_COPY, sessionId, new TextEncoder().encode(selection)); + + /* + And the selection goes, which is the console's behaviour and is load-bearing rather than cosmetic. + It is the only acknowledgement there is that the copy happened — nothing else on screen moves — and + it is what makes the *next* right-click a paste. Left standing, a selection somebody had forgotten + about would turn every later right-click into a copy of it, and the paste would look broken. + */ + session.term.clearSelection(); + } else { + send(CLIENT_CLIPBOARD_PASTE, sessionId); + } + + // Right-clicking a pane is a way of arriving at it. The caret follows, for the reason the paste frame's + // own focus() call gives. + session.term.focus(); +} + /** * Builds a pane for a session, or returns the one already there. * @@ -302,6 +365,19 @@ function createSession(sessionId) { convertEol: false, cursorBlink: true, fontSize, + + /* + Off, and on macOS that is a change from the default rather than a restatement of it: xterm defaults + this to true there, so a right-click would select the word under the pointer. With the gesture + meaning copy-or-paste, a right-click that quietly makes a selection is a right-click that copies a + word instead of pasting — and only on one platform, which is the worst way for it to be wrong. + + Belt and braces today, because handleRightClick takes the event in the capture phase and xterm never + sees it. It is here for the upgrade that moves xterm's handler to mousedown, which is where it + already is on the Firefox path. + */ + rightClickSelectsWord: false, + scrollback: SCROLLBACK_LINES, // Matches terminal.css, so the canvas and the page agree on the background. Both are the v2 // design's terminal surface — `TerminalSurface` in Palette.axaml. See the note in terminal.css. @@ -346,6 +422,10 @@ function createSession(sessionId) { term.attachCustomKeyEventHandler(handleKey); + // On the pane rather than on xterm's own element, and in the capture phase, which is what puts this + // ahead of xterm's handler on the element inside it. See handleRightClick. + pane.addEventListener('contextmenu', (event) => handleRightClick(event, sessionId), true); + term.onData((data) => { send(CLIENT_INPUT, sessionId, new TextEncoder().encode(data)); }); @@ -562,6 +642,35 @@ function handleFrame(buffer) { break; } + case SERVER_CLIPBOARD: { + const session = sessions.get(sessionId); + + if (!session || payload.length < 1) { + break; + } + + /* + paste() unconditionally, and that is the whole difference from SERVER_PASTE above. This text came + off the system clipboard, which is the case bracketed paste is *for*: readline uses the markers to + decide it has been pasted into, and a shell that gets a multi-line command inside them treats every + newline as text rather than as "run this". The reasoning that makes a single-line *snippet* worth + typing instead — bash marking a bracketed paste as an active region, so the command sits at the + prompt looking selected — is the reasoning for a snippet reading as though it had been typed. A + clipboard paste is not pretending to be typing, and every other terminal highlights it too. + + No execute flag, either: a paste inserts and stops. Pressing Enter for somebody is what the + sidebar's RUN does, and it is a thing they asked for by name. + */ + session.term.paste(new TextDecoder().decode(payload)); + + // The caret goes where the text landed, for the reason SERVER_PASTE's own focus() call gives — half + // of it, anyway. Here the other half is already right: the gesture that asked for this was a click + // on the pane, so the host's window never took Win32 focus away. + session.term.focus(); + + break; + } + case SERVER_FONT_SIZE: { if (payload.length < 1) { break; @@ -808,4 +917,15 @@ document.addEventListener('visibilitychange', () => { window.addEventListener('focus', reconnectIfDown); window.addEventListener('online', reconnectIfDown); +/* + And nowhere on this page has a context menu, which the panes' own handler nearly finishes on its own: they + are positioned at inset 0, so every right-click over a terminal lands on one and is stopped there before it + reaches here. + + What is left is the status banner, which is drawn over the bottom of the pane while it has something to say + — so a right-click on "Reconnecting the terminal view…" would have been the one place in a terminal window + that still opened a browser menu. Prevented and not stopped: there is nothing below this to stop it from. +*/ +document.addEventListener('contextmenu', (event) => event.preventDefault()); + connect(); diff --git a/src/DodoSSH.Client.Shell/WebAssets/vendor/README.md b/src/DodoSSH.Client.Shell/WebAssets/vendor/README.md index 06a65bd..24fa42f 100644 --- a/src/DodoSSH.Client.Shell/WebAssets/vendor/README.md +++ b/src/DodoSSH.Client.Shell/WebAssets/vendor/README.md @@ -24,5 +24,11 @@ These are the UMD builds, so they attach to `globalThis`: `Terminal`, `FitAddon. 1. Download the four files at the new versions and update the table above. 2. Confirm the UMD global names are unchanged. -3. Run the terminal suite, then start the app and open a session: nothing here is covered by the - .NET tests, because the renderer they exercise is a `ClientWebSocket` standing in for this page. +3. Confirm the right-click still belongs to `terminal.js`. Two things about xterm are load-bearing for it + and neither is a global: `rightClickSelectsWord` has to still be an option — the page sets it false, and + an option xterm no longer knows is silently ignored — and xterm has to still register its own right-click + handling on `contextmenu`, which `handleRightClick` gets ahead of in the capture phase. A build that moved + that to `mousedown`, as it already is on the Firefox path, would put xterm back in the gesture. +4. Run the terminal suite, then start the app and open a session: `RendererClipboardTests` runs the page's + own logic against a stubbed terminal, so what is left for a person is everything about the real one — + `docs/manual-checks.md` 1.11 is that list. diff --git a/src/DodoSSH.Client.Terminal/TerminalDataPlane.cs b/src/DodoSSH.Client.Terminal/TerminalDataPlane.cs index 6e88232..5bb88ef 100644 --- a/src/DodoSSH.Client.Terminal/TerminalDataPlane.cs +++ b/src/DodoSSH.Client.Terminal/TerminalDataPlane.cs @@ -105,6 +105,23 @@ public sealed class TerminalDataPlane : ITerminalTransport, IAsyncDisposable /// public event EventHandler? FontSizeStepRequested; + /// Raised when the page has text for the system clipboard. + /// + /// A right-click over a selection. Raised on the socket's receive loop, like + /// and with the same obligation on a handler that touches a view + /// model — and the clipboard itself is a UI-thread object on more than one platform, so this one has to + /// be marshalled even where nothing is bound to. + /// + public event EventHandler? ClipboardCopyRequested; + + /// Raised when the page wants the system clipboard's text pasted into one of its panes. + /// + /// A right-click with nothing selected. Raised on the socket's receive loop; see + /// . The answer goes back as + /// , which is 's to send. + /// + public event EventHandler? ClipboardPasteRequested; + /// /// Raised after a socket attaches — the first one, and every later takeover. /// @@ -463,16 +480,8 @@ public sealed class TerminalDataPlane : ITerminalTransport, IAsyncDisposable return; } - // Answered before the session lookup, because it is the one frame that is not about a session. The - // page sends whichever id it had to hand, and a chord pressed in a terminal whose shell has just - // ended is still a request to make the text bigger. - if ((TerminalClientOpcode)opcode is TerminalClientOpcode.FontSizeStep) + if (Announce(opcode, sessionId, payload)) { - if (TerminalFrame.TryReadFontSizeStep(payload, out var step)) - { - FontSizeStepRequested?.Invoke(this, new TerminalFontSizeStepEventArgs(step)); - } - return; } @@ -518,6 +527,59 @@ public sealed class TerminalDataPlane : ITerminalTransport, IAsyncDisposable } } + /// + /// Raises the frames no pump needs to see, and says whether this was one of them. + /// + /// + /// + /// Answered before the session lookup, because none of these three touches a pump. A text-size chord is + /// about the person reading rather than about one terminal, and the page sends whichever id it had to + /// hand. A copy is about the machine's clipboard. A paste request is answered by sending a frame back + /// rather than by writing to a pty, and does its own + /// liveness check — which is the one that can honestly refuse. + /// + /// + /// Which is also what keeps all three working in a pane whose shell has just ended, and that is when + /// they are most wanted: squinting at the message a remote left behind is exactly when somebody makes + /// the text bigger or copies the line. Dropping the frame at the pump lookup would have made a dead tab + /// the one place the gesture silently did nothing. + /// + /// + private bool Announce(byte opcode, uint sessionId, ReadOnlySpan payload) + { + switch ((TerminalClientOpcode)opcode) + { + case TerminalClientOpcode.FontSizeStep: + if (TerminalFrame.TryReadFontSizeStep(payload, out var step)) + { + FontSizeStepRequested?.Invoke(this, new TerminalFontSizeStepEventArgs(step)); + } + + return true; + + case TerminalClientOpcode.ClipboardCopy: + // An empty copy is dropped rather than forwarded. The clipboard is shared with everything + // else on the machine, so "copy nothing" would be a gesture that quietly destroys whatever + // was on it — and the page does not send one, which makes this a guard against a frame + // arriving from something other than the page we shipped. + if (payload.Length > 0) + { + ClipboardCopyRequested?.Invoke( + this, + new TerminalClipboardCopyEventArgs(Encoding.UTF8.GetString(payload))); + } + + return true; + + case TerminalClientOpcode.ClipboardPaste: + ClipboardPasteRequested?.Invoke(this, new TerminalClipboardPasteEventArgs(sessionId)); + return true; + + default: + return false; + } + } + /// /// Computes the Sec-WebSocket-Accept value RFC 6455 §4.2.2 requires. /// diff --git a/src/DodoSSH.Client.Terminal/TerminalFrame.cs b/src/DodoSSH.Client.Terminal/TerminalFrame.cs index 5eda2cf..5d318a0 100644 --- a/src/DodoSSH.Client.Terminal/TerminalFrame.cs +++ b/src/DodoSSH.Client.Terminal/TerminalFrame.cs @@ -95,6 +95,31 @@ public enum TerminalServerOpcode : byte /// /// FontSize = 7, + + /// + /// The system clipboard's text, for this session's pane to paste. Payload is UTF-8. + /// + /// + /// + /// Sent in answer to , which is the only thing that + /// asks for it: a right-click over a pane with nothing selected. The host reads the clipboard because + /// the page cannot — see that opcode for why not. + /// + /// + /// Not , and the difference is the bracketing. That opcode carries a snippet + /// off the sidebar, and the page deliberately types a single-line one rather than bracketing + /// it, because bash marks a bracketed paste as an active region and a snippet should read as though it + /// had been typed. A clipboard paste is the case those markers are for — readline uses them to decide it + /// has been pasted into — so this one is always bracketed. One opcode for both would need a flag + /// meaning "which of the two kinds of paste is this", which is the same distinction spelled less + /// clearly, on a payload that already has a flag byte meaning something else. + /// + /// + /// No flag byte, therefore, and no Enter: a clipboard paste inserts and stops. Pressing Enter for + /// somebody is what the sidebar's RUN does, and it is a thing they asked for by name. + /// + /// + Clipboard = 8, } /// Frames the renderer sends to the host. @@ -131,6 +156,52 @@ public enum TerminalClientOpcode : byte /// /// FontSizeStep = 4, + + /// + /// Text to put on the system clipboard. Payload is UTF-8. + /// + /// + /// + /// A right-click over a pane with something selected. What gets copied is the selection, and only the + /// page knows what that is — xterm draws it into a canvas and the host never sees a glyph of it — so it + /// is the text that travels here rather than a request for it. + /// + /// + /// Through the host rather than through the page's own navigator.clipboard. Writing the + /// clipboard from the page would work on Windows, where the renderer is Blink and + /// clipboard-write is granted to a focused document without asking. It is the reading + /// that cannot be done that way — see — and a clipboard that is the host's + /// in one direction and the page's in the other is two mechanisms to keep working across WebView2, + /// WebKitGTK and Android's WebView. The host has owned a clipboard delegate since the keychain grew its + /// COPY buttons; this is that one. + /// + /// + /// The session id is the pane the selection came from. Nothing routes on it — a clipboard is not a + /// property of one terminal — and it is carried because every frame carries one. + /// + /// + ClipboardCopy = 5, + + /// + /// The page asking for the clipboard's text, to paste into this session. No payload. + /// + /// + /// + /// A right-click over a pane with nothing selected. The answer arrives as + /// , addressed to the session id this frame carried, so the + /// text lands in the pane the pointer was over rather than in whichever one the host believes is + /// selected. They agree in every arrangement that exists today, because only the active pane is + /// visible — but the page is the one that knows, so the page is the one that says. + /// + /// + /// A request rather than the page reading the clipboard itself, and this is the half with no + /// alternative. navigator.clipboard.readText() is gated behind the clipboard-read + /// permission in every Chromium, and an embedded WebView answers a permission request it has no handler + /// for by putting a prompt on the screen. Trading a context menu for a permission dialog is not the + /// change this was. + /// + /// + ClipboardPaste = 6, } /// diff --git a/src/DodoSSH.Client.Terminal/TerminalWorkspace.cs b/src/DodoSSH.Client.Terminal/TerminalWorkspace.cs index 06e68ca..72962d6 100644 --- a/src/DodoSSH.Client.Terminal/TerminalWorkspace.cs +++ b/src/DodoSSH.Client.Terminal/TerminalWorkspace.cs @@ -72,6 +72,28 @@ public sealed class TerminalFontSizeStepEventArgs(int step) : EventArgs public int Step { get; } = step; } +/// The renderer handing over text for the system clipboard. +/// +/// The selection, exactly as the page had it. Never empty — the transport drops an empty copy rather than +/// letting a stray gesture wipe the machine's clipboard. +/// +public sealed class TerminalClipboardCopyEventArgs(string text) : EventArgs +{ + /// What to put on the clipboard. + public string Text { get; } = text; +} + +/// The renderer asking for the system clipboard's text. +/// +/// The pane the pointer was over, which is where the text is to land. The page sends it rather than leaving +/// the host to infer it from whichever tab it believes is selected. +/// +public sealed class TerminalClipboardPasteEventArgs(uint sessionId) : EventArgs +{ + /// The terminal to paste into. + public uint SessionId { get; } = sessionId; +} + /// /// Owns the loopback data plane and every live terminal session. /// @@ -148,6 +170,12 @@ public sealed class TerminalWorkspace : IAsyncDisposable // what remembers it between launches. dataPlane.FontSizeStepRequested += (_, e) => FontSizeStepRequested?.Invoke(this, e); + // Forwarded for the same reason and decided no more than the size is. What a clipboard is and how it + // is reached is a property of the platform the shell is running on, and this object has never seen a + // visual. + dataPlane.ClipboardCopyRequested += (_, e) => ClipboardCopyRequested?.Invoke(this, e); + dataPlane.ClipboardPasteRequested += (_, e) => ClipboardPasteRequested?.Invoke(this, e); + // Fire-and-forget: this fires on the socket-accept thread, in the middle of the data plane's own // handshake handling, and has no business making that wait on however long a replay takes. See // ReplayAfterAttachAsync for what "replay" means and why racing the fresh page's own first frames @@ -313,6 +341,24 @@ public sealed class TerminalWorkspace : IAsyncDisposable /// public event EventHandler? FontSizeStepRequested; + /// Raised when a right-click over a selection asks for it to be copied. + /// + /// The gesture can only be heard by the page, for the reason gives, + /// and the selection can only be read there: xterm draws it into a canvas. So the text arrives + /// here, and what a clipboard is on this platform is the shell's to know. Raised on the data plane's + /// receive loop; marshal before touching one. + /// + public event EventHandler? ClipboardCopyRequested; + + /// Raised when a right-click with nothing selected asks for the clipboard to be pasted. + /// + /// Answered with , which is the other half: this event carries no text + /// because the page has none to give — reading the clipboard is exactly what it cannot do. Raised on the + /// data plane's receive loop; marshal before touching a view model, and before touching a clipboard, + /// which is a UI-thread object on more than one platform. + /// + public event EventHandler? ClipboardPasteRequested; + /// /// Raised once a (re)attached renderer has been sent everything this workspace owns for it. /// @@ -359,6 +405,55 @@ public sealed class TerminalWorkspace : IAsyncDisposable TerminalFrame.CreateFontSizePayload(pixels)), cancellationToken); + /// + /// Hands the system clipboard's text to one pane, to be pasted into it. + /// + /// + /// + /// The answer to , and the only thing that sends this frame. Always + /// bracketed and never followed by an Enter — see for why a + /// clipboard paste is not the same act as inserting a snippet, and why the two do not share an opcode. + /// + /// + /// Refused for a dead session, exactly as is refused and for the weaker of its + /// two reasons: nothing visible would happen either way, because a paste is keystrokes rather than + /// output and there is no shell left to echo them. Checked anyway so that the answer is honest, and so + /// that a caller which grows a message one day has something to hang it on. + /// + /// + /// An empty clipboard is the caller's to notice. Sending nothing is not the same as sending an empty + /// string — the page would bracket it and the remote would see an empty paste — and "there was nothing + /// to paste" is a fact about the machine rather than about this terminal. + /// + /// + /// The terminal to paste into. + /// The clipboard's text. Sent verbatim. + /// Cancellation. + /// when that session's shell is not running. + public async Task SendClipboardAsync( + uint sessionId, + string text, + CancellationToken cancellationToken) + { + ArgumentNullException.ThrowIfNull(text); + + if (!IsSessionLive(sessionId)) + { + return false; + } + + await dataPlane + .SendAsync( + TerminalFrame.Create( + (byte)TerminalServerOpcode.Clipboard, + sessionId, + Encoding.UTF8.GetBytes(text)), + cancellationToken) + .ConfigureAwait(false); + + return true; + } + /// /// Waits until the renderer page has attached its socket. /// diff --git a/tests/DodoSSH.Client.App.Tests/FakeRenderer.cs b/tests/DodoSSH.Client.App.Tests/FakeRenderer.cs index 0f2218e..6c868ca 100644 --- a/tests/DodoSSH.Client.App.Tests/FakeRenderer.cs +++ b/tests/DodoSSH.Client.App.Tests/FakeRenderer.cs @@ -71,6 +71,63 @@ internal sealed partial class FakeRenderer : IAsyncDisposable return renderer; } + /// Sends a client frame, as the page would. + /// + /// Raw rather than through a helper per opcode: the two this exists for carry a session id and either + /// UTF-8 or nothing at all, and a method each would be three lines of ceremony around + /// . + /// + internal async Task SendAsync( + TerminalClientOpcode opcode, + uint sessionId, + ReadOnlyMemory payload, + CancellationToken cancellationToken) => + await socket + .SendAsync( + TerminalFrame.Create((byte)opcode, sessionId, payload.Span), + WebSocketMessageType.Binary, + endOfMessage: true, + cancellationToken) + .ConfigureAwait(false); + + /// Reads frames until one carries that opcode, and answers with it. + /// + /// + /// Skipping rather than expecting the next frame to be the interesting one, because a live session's + /// socket is not quiet: the shell's opening frame, its activation and whatever the remote said are all + /// on the way, and which of them have arrived by the time a test looks is a scheduling accident. + /// + /// + /// Bounded only by the caller's token, which in this suite is the test's own — so a frame that never + /// comes fails the test on its timeout rather than hanging the run. + /// + /// + internal async Task WaitForAsync( + TerminalServerOpcode opcode, + CancellationToken cancellationToken) + { + var buffer = new byte[64 * 1024]; + + while (true) + { + var result = await socket + .ReceiveAsync(buffer.AsMemory(), cancellationToken) + .ConfigureAwait(false); + + if (result.MessageType == WebSocketMessageType.Close) + { + throw new InvalidOperationException( + $"The host closed the socket before sending a {opcode} frame."); + } + + if (TerminalFrame.TryRead(buffer.AsSpan(0, result.Count), out var read, out _, out var payload) + && read == (byte)opcode) + { + return payload.ToArray(); + } + } + } + /// public async ValueTask DisposeAsync() { diff --git a/tests/DodoSSH.Client.App.Tests/ShellFlowTests.cs b/tests/DodoSSH.Client.App.Tests/ShellFlowTests.cs index f311b37..057d859 100644 --- a/tests/DodoSSH.Client.App.Tests/ShellFlowTests.cs +++ b/tests/DodoSSH.Client.App.Tests/ShellFlowTests.cs @@ -33,6 +33,15 @@ public sealed class ShellFlowTests : IAsyncLifetime /// Everything this shell has copied, newest last. private readonly List clipboard = []; + /// + /// What the machine's clipboard holds, as far as this shell can read it. + /// + /// + /// Null rather than empty by default, which is what a clipboard holding something that is not text + /// answers. Set by the one test that needs the shell to read one — the terminal's right-click paste. + /// + private string? machineClipboard; + /// /// Far below the shipped profile, for the same reason as everywhere else: these tests are about the /// state machine, not about how expensive the passphrase is to attack. @@ -164,6 +173,10 @@ public sealed class ShellFlowTests : IAsyncLifetime return Task.CompletedTask; }, + // And the other direction, which only the terminal's right-click paste uses. A closure over the + // field rather than a value captured now, because a test sets it after the shell is built. + readClipboard: () => Task.FromResult(machineClipboard), + // Inline, because this suite has no window and therefore no dispatcher to drain — the same // answer TransferQueueingTests reached, and for the reason its own remark gives: reaching // Dispatcher.UIThread from a test means asserting on a queue owned by whichever class touched @@ -294,6 +307,90 @@ public sealed class ShellFlowTests : IAsyncLifetime } } + // ---- The terminal's right-click, which is a copy or a paste ---- + + /// + /// + /// The gesture is heard by the page and nowhere else — the selection is drawn into a canvas, and a + /// focused terminal leaves the host's window no input events at all — so the shell's part is to be the + /// clipboard. This is that wiring end to end: a real frame on the real socket, through the workspace, to + /// the delegate a head supplies. + /// + /// + /// Sent with no session open at all, deliberately. A copy is not about a terminal, and the frame is + /// dispatched before the session lookup precisely so that copying the last thing a dead tab said still + /// works — which is when somebody is most likely to want it. + /// + /// + [Fact] + public async Task ARightClickCopy_ReachesTheMachinesClipboard() + { + await using var renderer = await FakeRenderer.AttachAsync(workspace, Token); + + await renderer.SendAsync( + TerminalClientOpcode.ClipboardCopy, + sessionId: 0, + System.Text.Encoding.UTF8.GetBytes("error: nothing was written"), + Token); + + await EventuallyAsync( + () => clipboard.Contains("error: nothing was written", StringComparer.Ordinal), + "the copy frame should have reached the clipboard delegate"); + } + + /// + /// The other direction, and the half the page cannot do for itself: reading a clipboard from a WebView + /// needs the clipboard-read permission, which an embedded WebView answers with a prompt. So the + /// page asks, the shell reads, and the answer goes back down the same socket addressed to the pane that + /// asked — which is what this follows all the way round. + /// + [Fact] + public async Task ARightClickPaste_SendsTheMachinesClipboardBackToThePaneThatAsked() + { + var vault = await ReadyToConnectAsync(); + + await using var renderer = await FakeRenderer.AttachAsync(workspace, Token); + await vault.ConnectCommand.ExecuteAsync(null); + + var sessionId = shell.SelectedTab.ShouldNotBeNull().SessionId; + + machineClipboard = "sudo systemctl restart sshd"; + + await renderer.SendAsync(TerminalClientOpcode.ClipboardPaste, sessionId, default, Token); + + var payload = await renderer.WaitForAsync(TerminalServerOpcode.Clipboard, Token); + + System.Text.Encoding.UTF8.GetString(payload).ShouldBe("sudo systemctl restart sshd"); + } + + /// + /// An empty clipboard is answered with nothing rather than with an empty paste, which the page would + /// bracket and hand the remote as a keystroke nobody made. Proven by the frame that does come + /// after it: the receive loop is sequential, so a clipboard frame for the second request arriving means + /// the first was seen and passed over. + /// + [Fact] + public async Task ARightClickPaste_SendsNothingWhenTheClipboardHasNoText() + { + var vault = await ReadyToConnectAsync(); + + await using var renderer = await FakeRenderer.AttachAsync(workspace, Token); + await vault.ConnectCommand.ExecuteAsync(null); + + var sessionId = shell.SelectedTab.ShouldNotBeNull().SessionId; + + machineClipboard = null; + await renderer.SendAsync(TerminalClientOpcode.ClipboardPaste, sessionId, default, Token); + + machineClipboard = "uptime"; + await renderer.SendAsync(TerminalClientOpcode.ClipboardPaste, sessionId, default, Token); + + var payload = await renderer.WaitForAsync(TerminalServerOpcode.Clipboard, Token); + + System.Text.Encoding.UTF8.GetString(payload) + .ShouldBe("uptime", "the empty clipboard sent no frame of its own"); + } + [Fact] public async Task AFreshMachine_AsksForAServer() { diff --git a/tests/DodoSSH.Client.Terminal.Tests/Renderer/renderer-harness.js b/tests/DodoSSH.Client.Terminal.Tests/Renderer/renderer-harness.js index 878be07..10a21bf 100644 --- a/tests/DodoSSH.Client.Terminal.Tests/Renderer/renderer-harness.js +++ b/tests/DodoSSH.Client.Terminal.Tests/Renderer/renderer-harness.js @@ -5,17 +5,36 @@ It fakes exactly what the page's transport touches and nothing else: a WebSocket whose every step is driven from the test, a clock that only moves when a test moves it, and the two DOM objects the page - reads at load. There is no Terminal, no fit addon and no WebGL here, because nothing on the reconnection - path builds a pane — a test that opens a session will have to add them, and should, rather than this - file guessing now at what such a test would want. + reads at load. + + ◆ AND, SINCE RendererClipboardTests, A PANE. + + Which the reconnection tests never needed and this file used to say so, inviting whoever first opened a + session to add the Terminal, the fit addon and the WebGL addon rather than guessing at them in advance. + They are below, and they are stubs with a memory: a fake terminal records what was pasted into it, what + was typed into it and what it was asked to focus, because those three are the whole of what the page does + to a pane and therefore the whole of what a test can ask about. The selection is settable from a test, + since a selection is made with a mouse in a canvas and there is no canvas here. + + What a fake terminal is NOT is xterm. It cannot say whether paste() brackets its argument or whether + getSelection() trims a row's right padding — that is xterm's behaviour, exercised by xterm's own suite and + by docs/manual-checks.md 1.11 here. What these tests can prove is which of the two the page calls, with + what, and addressed to which session, and that is where every mistake in this area has actually been. ◆ NOTHING HERE MAY DECLARE A NAME terminal.js ALSO DECLARES. - Both files are evaluated as scripts into the same global scope, and the page's own `const root` against - a `var root` here is a SyntaxError before a line of either runs. That is why the page's two elements are + Both files are evaluated as scripts into the same global scope, and a collision goes one of two ways. + Against one of the page's `const`s — its own `root`, say — it is a SyntaxError before a line of either + runs, which is the good outcome: it is impossible to miss. That is why the page's two elements are `pageRoot` and `pageBanner` below and reached through getElementById, which is how the page reaches them anyway. + Against one of the page's *functions* there is no error at all. The page is evaluated second, so its + declaration silently replaces this file's and every test calling that name gets the page's function + instead — which is how `frame()` here, meant to read a sent frame back, quietly returned the ArrayBuffer + that terminal.js's own frame() builds instead, and failed six tests on a type error a long way from the + cause. Hence `sentFrames`/`sentFrame` below: prefixed on purpose, not for tidiness. + ◆ AND THE FAKE SOCKET'S BEHAVIOUR IS THE PART TO GET RIGHT. close() is the one method with a rule that is not obvious and that the tests lean on: a socket closed @@ -30,6 +49,15 @@ var clock = { now: 0, next: 1, timers: {} }; /** Every socket the page has opened, in order, live or dead. */ var sockets = []; +/** Every frame the page has sent, in order, as arrays of byte values. */ +var outbound = []; + +/** Every fake terminal the page has opened on a pane, in the order it opened them. */ +var terminals = []; + +/** What the last right-click did about the browser's own menu. See rightClick(). */ +var gesture = { prevented: 0, stopped: 0 }; + function listeners(target) { target.handlers = {}; @@ -41,11 +69,11 @@ function listeners(target) { target.handlers[name].push(handler); }; - target.fire = function (name) { + target.fire = function (name, event) { var handlers = target.handlers[name] || []; for (var i = 0; i < handlers.length; i++) { - handlers[i]({}); + handlers[i](event || {}); } }; @@ -67,7 +95,22 @@ FakeSocket.OPEN = 1; FakeSocket.CLOSING = 2; FakeSocket.CLOSED = 3; -FakeSocket.prototype.send = function () {}; +/** + * Records rather than discards, because half of what the page does on a right-click is a frame. + * + * Kept as raw bytes and decoded on the way out — see sentFrame() — so that a payload the page wrote as + * UTF-8 is read back as UTF-8 rather than as whatever a stringification would have made of it. + */ +FakeSocket.prototype.send = function (buffer) { + var bytes = new Uint8Array(buffer); + var copy = []; + + for (var i = 0; i < bytes.length; i++) { + copy.push(bytes[i]); + } + + outbound.push(copy); +}; /** What the page calls. See the note above on why a connecting socket goes quietly. */ FakeSocket.prototype.close = function () { @@ -93,9 +136,31 @@ var pageRoot = listeners({ var document = listeners({ hidden: false, getElementById: function (id) { return id === 'root' ? pageRoot : pageBanner; }, - createElement: function () { return { dataset: {}, style: {}, remove: function () {} }; }, + createElement: function () { return newElement(); }, }); +/** + * A pane, as far as the page can tell. + * + * With real dimensions, and not because anything would fail without them — which is the point. The page's + * guard is `clientWidth < MINIMUM_FITTABLE_PIXELS`, and an undefined width compares false against every + * number, so a fake with no dimensions slips past a rule it ought to have been caught by and gets fitted + * anyway. A fake that passes for the wrong reason stops matching the page the day that guard is written + * differently. + * + * Listeners, because the right-click handler is registered on this and there is nowhere else to fire it + * from. + */ +function newElement() { + return listeners({ + dataset: {}, + style: {}, + clientWidth: 800, + clientHeight: 600, + remove: function () {}, + }); +} + var window = listeners({}); // The page warns through this on paths no reconnection test reaches. Present so that a test which does @@ -118,6 +183,142 @@ function ResizeObserver() { this.observe = function () {}; } +// ── The pane's three globals, which the page reaches by the names the UMD builds attach ────────────── + +/** + * A terminal that remembers what was done to it. + * + * `selection` is a plain field rather than anything computed, and settable from a test through select(): + * a selection is made by dragging a mouse across a canvas, which is the one part of this there is no + * standing in for. What the tests are about is what the page does with whatever is selected. + */ +function FakeTerminal(options) { + this.options = options || {}; + this.cols = 80; + this.rows = 24; + this.selection = ''; + this.pastes = []; + this.inputs = []; + this.writes = []; + this.focuses = 0; + this.disposed = false; + this.pane = null; + + terminals.push(this); +} + +FakeTerminal.prototype.loadAddon = function () {}; +FakeTerminal.prototype.open = function (pane) { this.pane = pane; }; +FakeTerminal.prototype.attachCustomKeyEventHandler = function (handler) { this.keys = handler; }; +FakeTerminal.prototype.onData = function (handler) { this.data = handler; }; +FakeTerminal.prototype.onResize = function (handler) { this.resized = handler; }; +FakeTerminal.prototype.focus = function () { this.focuses++; }; +FakeTerminal.prototype.paste = function (text) { this.pastes.push(text); }; +FakeTerminal.prototype.input = function (text) { this.inputs.push(text); }; +FakeTerminal.prototype.getSelection = function () { return this.selection; }; +FakeTerminal.prototype.clearSelection = function () { this.selection = ''; }; +FakeTerminal.prototype.dispose = function () { this.disposed = true; }; + +/** + * The write callback is the acknowledgement, so it is called rather than kept. + * + * Synchronously, which a real xterm does not promise — it renders first. The page's contract is that the + * acknowledgement is sent from this callback and not before, and calling it straight away is what lets a + * test assert that without needing a frame clock. + */ +FakeTerminal.prototype.write = function (data, done) { + this.writes.push(data); + + if (done) { + done(); + } +}; + +var Terminal = FakeTerminal; + +var FitAddon = { + FitAddon: function () { + this.fit = function () {}; + }, +}; + +var WebglAddon = { + WebglAddon: function () { + this.onContextLoss = function () {}; + this.dispose = function () {}; + }, +}; + +// ── UTF-8, because the page encodes and decodes every payload through these ────────────────────────── +// +// Written out rather than stubbed as one byte per character. The page sends a selection as UTF-8 and reads +// a clipboard as UTF-8, so a harness that treated the two as Latin-1 would pass a test with an em dash in +// the selection and ship a page that mangled it. + +function TextEncoder() { + this.encode = function (text) { + var bytes = []; + + for (var i = 0; i < text.length; i++) { + var point = text.charCodeAt(i); + + // A surrogate pair is one code point in two units. Combined here, because encoding each half on its + // own is exactly the mojibake this exists to catch. + if (point >= 0xd800 && point <= 0xdbff && i + 1 < text.length) { + var low = text.charCodeAt(i + 1); + + if (low >= 0xdc00 && low <= 0xdfff) { + point = 0x10000 + ((point - 0xd800) << 10) + (low - 0xdc00); + i++; + } + } + + if (point < 0x80) { + bytes.push(point); + } else if (point < 0x800) { + bytes.push(0xc0 | (point >> 6), 0x80 | (point & 0x3f)); + } else if (point < 0x10000) { + bytes.push(0xe0 | (point >> 12), 0x80 | ((point >> 6) & 0x3f), 0x80 | (point & 0x3f)); + } else { + bytes.push( + 0xf0 | (point >> 18), + 0x80 | ((point >> 12) & 0x3f), + 0x80 | ((point >> 6) & 0x3f), + 0x80 | (point & 0x3f)); + } + } + + return new Uint8Array(bytes); + }; +} + +function TextDecoder() { + this.decode = function (bytes) { + var text = ''; + + for (var i = 0; i < bytes.length;) { + var first = bytes[i]; + var length = first < 0x80 ? 1 : first < 0xe0 ? 2 : first < 0xf0 ? 3 : 4; + var point = length === 1 ? first : first & (0x7f >> length); + + for (var n = 1; n < length; n++) { + point = (point << 6) | (bytes[i + n] & 0x3f); + } + + i += length; + + if (point > 0xffff) { + point -= 0x10000; + text += String.fromCharCode(0xd800 + (point >> 10), 0xdc00 + (point & 0x3ff)); + } else { + text += String.fromCharCode(point); + } + } + + return text; + }; +} + // ── What the tests drive the page with ────────────────────────────────────────────────────────────── /** @@ -165,6 +366,134 @@ function banner() { return pageBanner.textContent; } +/** How many frames the page has sent since it loaded. */ +function sentFrames() { + return outbound.length; +} + +/** + * One frame the page sent, as "opcode/session/payload", the payload decoded as UTF-8. + * + * One string rather than three values because it crosses into C# as one, and a test asserting on + * "5/7/whoami" reads as the frame it is. Resize frames land here too and read as rubbish; nothing asserts + * on those, and rendering them some other way would mean a second shape for a reader to learn. + */ +function sentFrame(index) { + var bytes = outbound[index]; + var session = ((bytes[1] << 24) | (bytes[2] << 16) | (bytes[3] << 8) | bytes[4]) >>> 0; + + return bytes[0] + '/' + session + '/' + new TextDecoder().decode(bytes.slice(5)); +} + +/** Hands the page a frame, on whichever socket is its current one. */ +function deliverFrame(opcode, sessionId, payload) { + var bytes = new Uint8Array(5 + payload.length); + var header = new DataView(bytes.buffer); + + header.setUint8(0, opcode); + header.setUint32(1, sessionId); + bytes.set(payload, 5); + + sockets[sockets.length - 1].fire('message', { data: bytes.buffer }); +} + +/** SessionOpened, which is what makes the page build a pane. */ +function openSession(sessionId) { + deliverFrame(2, sessionId, []); +} + +/** Clipboard: the host's answer to a right-click paste. */ +function deliverClipboard(sessionId, text) { + deliverFrame(8, sessionId, new TextEncoder().encode(text)); +} + +/** Paste: a snippet off the sidebar, which is deliberately not the same frame. */ +function deliverSnippet(sessionId, text, execute) { + var body = new TextEncoder().encode(text); + var payload = new Uint8Array(1 + body.length); + + payload[0] = execute ? 1 : 0; + payload.set(body, 1); + + deliverFrame(6, sessionId, payload); +} + +/** The pane behind one session, found the way the page labelled it. */ +function terminalFor(sessionId) { + for (var i = 0; i < terminals.length; i++) { + if (terminals[i].pane && terminals[i].pane.dataset.sessionId === String(sessionId)) { + return terminals[i]; + } + } + + throw new Error('No pane for session ' + sessionId + '.'); +} + +/** Drags a selection across that pane, which is the one thing a fake terminal cannot do for itself. */ +function select(sessionId, text) { + terminalFor(sessionId).selection = text; +} + +/** What is selected in that pane now — how a test sees that a copy dropped it. */ +function selection(sessionId) { + return terminalFor(sessionId).selection; +} + +/** + * The right button, over that pane. + * + * The event carries only the two methods the page calls on it, and counts the calls: suppressing the + * browser's own menu is half of what this gesture has to do, and a handler that copied correctly and left + * the menu up would be a regression no assertion about frames could see. + */ +function rightClick(sessionId) { + gesture = { prevented: 0, stopped: 0 }; + + terminalFor(sessionId).pane.fire('contextmenu', { + preventDefault: function () { gesture.prevented++; }, + stopPropagation: function () { gesture.stopped++; }, + }); +} + +/** + * The right button somewhere that is not a pane — the status banner, in practice. + * + * Fired on the document rather than on an element, which is where the page's fallback rule lives: a pane + * stops the event before it gets here, so this is the path only a click that missed every pane takes. + */ +function rightClickOffAPane() { + gesture = { prevented: 0, stopped: 0 }; + + document.fire('contextmenu', { + preventDefault: function () { gesture.prevented++; }, + stopPropagation: function () { gesture.stopped++; }, + }); +} + +/** Whether the last right-click took the event away from the browser, and from xterm. */ +function prevented() { + return gesture.prevented; +} + +function stopped() { + return gesture.stopped; +} + +/** Everything paste() was given for that pane, joined — the bracketed route. */ +function pasted(sessionId) { + return terminalFor(sessionId).pastes.join('|'); +} + +/** Everything input() was given for that pane, joined — the typed route. */ +function typed(sessionId) { + return terminalFor(sessionId).inputs.join('|'); +} + +/** How many times that pane has been asked for the caret. */ +function focuses(sessionId) { + return terminalFor(sessionId).focuses; +} + /** Whether that attempt has been closed, by the page or by the far end. */ function isClosed(index) { return sockets[index].readyState === FakeSocket.CLOSED; diff --git a/tests/DodoSSH.Client.Terminal.Tests/RendererClipboardTests.cs b/tests/DodoSSH.Client.Terminal.Tests/RendererClipboardTests.cs new file mode 100644 index 0000000..f8c715a --- /dev/null +++ b/tests/DodoSSH.Client.Terminal.Tests/RendererClipboardTests.cs @@ -0,0 +1,276 @@ +namespace DodoSSH.Client.Terminal.Tests; + +/// +/// The renderer page's right button: a copy, a paste, and never a menu. +/// +/// +/// +/// The gesture every terminal has had since the Windows console got QuickEdit — select and right-click to +/// copy, right-click with nothing selected to paste. It lives in the page because that is the only place it +/// can: the selection is drawn into a canvas the host never sees, and once a terminal has focus the host's +/// window receives no input events at all. +/// +/// +/// What is asserted here is which frame the page sends, addressed to which session, and which of xterm's two +/// insertion routes it takes on the way back. What is deliberately not asserted is anything about +/// xterm itself — whether paste() really brackets, whether a selection really loses its right +/// padding — because the fake terminal in the harness is a stub with a memory and not an emulator. That +/// boundary is where docs/manual-checks.md 1.11 picks up. +/// +/// +public sealed class RendererClipboardTests +{ + /// The session every single-pane test here opens. + private const uint SessionId = 7; + + [Fact] + public void ARightClickOverASelection_CopiesIt() + { + var page = Attached(); + + page.Do("select(7, 'error: nothing was written')"); + page.Do("rightClick(7)"); + + page.Sent.ShouldContain("5/7/error: nothing was written"); + } + + /// + /// The console's behaviour, and load-bearing rather than cosmetic. Dropping the selection is the only + /// acknowledgement of the copy there is — nothing else on screen moves — and it is what makes the next + /// right-click a paste rather than a second copy of something forgotten about. + /// + [Fact] + public void ARightClickOverASelection_DropsTheSelectionAfterCopyingIt() + { + var page = Attached(); + + page.Do("select(7, 'systemctl status sshd')"); + page.Do("rightClick(7)"); + + page.Selection(SessionId).ShouldBeEmpty(); + } + + [Fact] + public void ARightClickWithNothingSelected_AsksTheHostForTheClipboard() + { + var page = Attached(); + + page.Do("rightClick(7)"); + + // No payload: the page has nothing to send, which is the whole reason it has to ask. + page.Sent.ShouldContain("6/7/"); + } + + /// + /// The composite property, and the one a user would describe: copy, then paste. It passes only if the + /// copy dropped the selection — a page that left it standing sends a second copy frame here and never a + /// paste request, which is the failure that reads as "paste is broken". + /// + [Fact] + public void ASecondRightClick_PastesBecauseTheFirstOneCopied() + { + var page = Attached(); + + page.Do("select(7, 'df -h')"); + page.Do("rightClick(7)"); + page.Do("rightClick(7)"); + + page.Sent.ShouldContain("5/7/df -h"); + page.Sent.ShouldContain("6/7/"); + } + + /// + /// + /// preventDefault is the WebView's own context menu, which is what this whole change is about. + /// + /// + /// stopPropagation is xterm's, and it is the half that is easy to leave out. xterm registers its + /// own contextmenu handler on the element inside the pane, which parks a 20-pixel textarea under + /// the pointer at z-index 1000 and leaves the selection sitting in it, selected — both of which exist to + /// serve the native menu and are litter without one. The page takes the event in the capture phase to + /// get ahead of it; a listener that forgot the third argument to addEventListener would still + /// suppress the menu and would fail here. + /// + /// + [Fact] + public void ARightClick_TakesTheEventFromTheBrowserAndFromXterm() + { + var page = Attached(); + + page.Do("rightClick(7)"); + + page.Prevented.ShouldBe(1); + page.Stopped.ShouldBe(1); + } + + /// + /// The panes cover the page — they are positioned at inset 0 — so the only right-click that misses one is + /// on the status banner, which is drawn over the bottom of the pane while it has something to say. That + /// used to be the one place in a terminal window that still opened a browser menu. + /// + [Fact] + public void ARightClickThatMissesEveryPane_StillOpensNoMenu() + { + var page = Attached(); + + page.Do("rightClickOffAPane()"); + + page.Prevented.ShouldBe(1); + + // And nothing is stopped, because there is nothing below the document to stop it from — nor any + // frame sent, because a click that is on no pane is a click about no session. + page.Stopped.ShouldBe(0); + } + + /// + /// A right-click over a pane with a selection sends one frame and not two: the branch is exclusive, and a + /// page that asked for the clipboard as well would paste over the selection it had just copied. + /// + [Fact] + public void ARightClickOverASelection_DoesNotAlsoAskForTheClipboard() + { + var page = Attached(); + + page.Do("select(7, 'uname -a')"); + page.Do("rightClick(7)"); + + page.Sent.ShouldNotContain("6/7/"); + } + + /// + /// Two panes, because the session id on a clipboard frame is the whole of what puts the text where the + /// pointer was. They agree with the host's own idea of the active tab in every arrangement that exists + /// today; the page is what knows, so this is what proves the page says. + /// + [Fact] + public void ARightClick_IsAddressedToThePaneItHappenedIn() + { + var page = Attached(); + + page.Do("openSession(9)"); + page.Do("select(7, 'the first pane'); select(9, 'the second pane')"); + + page.Do("rightClick(9)"); + + page.Sent.ShouldContain("5/9/the second pane"); + page.Sent.ShouldNotContain("5/7/the first pane"); + } + + /// + /// The other half of the round trip. paste() rather than input(), because this text came + /// off the system clipboard and that is the case bracketed paste exists for: a shell that receives a + /// multi-line command inside the markers treats every newline as text rather than as "run this". + /// + [Fact] + public void TheClipboardComingBack_IsPastedRatherThanTyped() + { + var page = Attached(); + + page.Do("deliverClipboard(7, 'sudo systemctl restart sshd')"); + + page.Pasted(SessionId).ShouldBe("sudo systemctl restart sshd"); + page.Typed(SessionId).ShouldBeEmpty(); + } + + /// + /// No Enter, ever. A clipboard paste inserts and stops; pressing Enter for somebody is what the sidebar's + /// RUN does, and that is a thing they asked for by name. + /// + [Fact] + public void TheClipboardComingBack_DoesNotPressEnter() + { + var page = Attached(); + + page.Do("deliverClipboard(7, 'rm -rf /var/tmp/build')"); + + page.Typed(SessionId).ShouldBeEmpty(); + } + + /// The caret follows the paste, so the next keystroke lands where the text did. + [Fact] + public void TheClipboardComingBack_LeavesTheCaretInThatPane() + { + var page = Attached(); + + var before = page.Focuses(SessionId); + page.Do("deliverClipboard(7, 'whoami')"); + + page.Focuses(SessionId).ShouldBe(before + 1); + } + + /// + /// Multi-byte, because both directions run through the page's own TextEncoder and + /// TextDecoder and a character that straddles nothing is not a test of either. The harness's codec + /// is a real UTF-8 one for this reason; see its remark. + /// + [Fact] + public void TheTextSurvivesTheRoundTrip_WhenItIsNotAscii() + { + var page = Attached(); + + page.Do("select(7, 'échec — 90° · λ')"); + page.Do("rightClick(7)"); + page.Do("deliverClipboard(7, 'échec — 90° · λ')"); + + page.Sent.ShouldContain("5/7/échec — 90° · λ"); + page.Pasted(SessionId).ShouldBe("échec — 90° · λ"); + } + + /// + /// The distinction the new opcode exists to keep, asserted from the other side: a snippet off the sidebar + /// is still typed when it is a single line, because bash marks a bracketed paste as an active + /// region and a snippet should read as though it had been typed. Sharing one opcode with the clipboard + /// would have meant choosing one of these two behaviours for both. + /// + [Fact] + public void ASingleLineSnippet_IsStillTypedRatherThanPasted() + { + var page = Attached(); + + page.Do("deliverSnippet(7, 'tail -f /var/log/syslog', false)"); + + page.Typed(SessionId).ShouldBe("tail -f /var/log/syslog"); + page.Pasted(SessionId).ShouldBeEmpty(); + } + + /// + [Fact] + public void AMultiLineSnippet_IsStillPastedRatherThanTyped() + { + var page = Attached(); + + page.Do("deliverSnippet(7, 'cd /srv\\nmake\\nmake install', false)"); + + page.Pasted(SessionId).ShouldBe("cd /srv\nmake\nmake install"); + page.Typed(SessionId).ShouldBeEmpty(); + } + + /// + /// And the Enter goes through the typed route even for a multi-line snippet, deliberately outside the + /// bracketing: a \r inside the wrapper arrives at the shell as a literal carriage return and + /// nothing runs. + /// + [Fact] + public void ASnippetWithRun_PressesEnterOutsideTheBrackets() + { + var page = Attached(); + + page.Do("deliverSnippet(7, 'cd /srv\\nmake', true)"); + + page.Pasted(SessionId).ShouldBe("cd /srv\nmake"); + page.Typed(SessionId).ShouldBe("\r"); + } + + /// A page with an open socket and one pane, which is where every test here starts. + private static RendererPage Attached() + { + var page = RendererPage.Load(); + + // The socket has to be open before anything is sent: send() drops a frame when it is not, which + // would make every assertion here pass or fail on the transport rather than on the gesture. + page.Do("accept(0)"); + page.Do("openSession(7)"); + + return page; + } +} diff --git a/tests/DodoSSH.Client.Terminal.Tests/RendererPage.cs b/tests/DodoSSH.Client.Terminal.Tests/RendererPage.cs index dacd105..52b4a09 100644 --- a/tests/DodoSSH.Client.Terminal.Tests/RendererPage.cs +++ b/tests/DodoSSH.Client.Terminal.Tests/RendererPage.cs @@ -44,6 +44,60 @@ internal sealed class RendererPage /// What the status element says — the banner a user would be looking at. public string Banner => engine.Evaluate("banner()").AsString(); + /// + /// Every frame the page has sent, each as opcode/session/payload. + /// + /// + /// Materialised on each read rather than cached, because the point of it is what the page did in answer + /// to the last . A session's own resize frames are in here too and read as rubbish; see + /// the harness's sentFrame() for why they are not rendered some other way. + /// + public IReadOnlyList Sent + { + get + { + var count = (int)engine.Evaluate("sentFrames()").AsNumber(); + var frames = new List(count); + + for (var index = 0; index < count; index++) + { + frames.Add( + engine + .Evaluate(string.Create(CultureInfo.InvariantCulture, $"sentFrame({index})")) + .AsString()); + } + + return frames; + } + } + + /// How many times the last right-click called preventDefault, and stopPropagation. + /// + /// Both, because they answer different questions: the first is whether the WebView's own context menu + /// was suppressed, and the second is whether xterm's own right-click handling was — which is what keeps + /// a 20-pixel textarea from being parked under the pointer with the selection sitting in it. + /// + public int Prevented => (int)engine.Evaluate("prevented()").AsNumber(); + + /// + public int Stopped => (int)engine.Evaluate("stopped()").AsNumber(); + + /// What is selected in that session's pane. + public string Selection(uint sessionId) => + engine.Evaluate(string.Create(CultureInfo.InvariantCulture, $"selection({sessionId})")).AsString(); + + /// Everything term.paste was given for that session — the bracketed route. + public string Pasted(uint sessionId) => + engine.Evaluate(string.Create(CultureInfo.InvariantCulture, $"pasted({sessionId})")).AsString(); + + /// Everything term.input was given for that session — the typed route. + public string Typed(uint sessionId) => + engine.Evaluate(string.Create(CultureInfo.InvariantCulture, $"typed({sessionId})")).AsString(); + + /// How many times that session's pane has been asked for the caret. + public int Focuses(uint sessionId) => + (int)engine.Evaluate(string.Create(CultureInfo.InvariantCulture, $"focuses({sessionId})")).AsNumber(); + /// /// Loads the harness and then the page, leaving the page exactly as it is a moment after the WebView /// navigated to it: one socket opened and still connecting. diff --git a/tests/DodoSSH.Client.Terminal.Tests/TerminalDataPlaneTests.cs b/tests/DodoSSH.Client.Terminal.Tests/TerminalDataPlaneTests.cs index a9b40fa..46cfe09 100644 --- a/tests/DodoSSH.Client.Terminal.Tests/TerminalDataPlaneTests.cs +++ b/tests/DodoSSH.Client.Terminal.Tests/TerminalDataPlaneTests.cs @@ -356,6 +356,82 @@ public sealed class TerminalDataPlaneTests : IAsyncDisposable steps[0].ShouldBe(-1); } + /// + /// Dispatched before the session lookup for the reason the text-size chord is, and one of its own: a + /// clipboard is not a property of any terminal. The absence of a registered session is the point of the + /// test — copying the last thing a remote said before it hung up is an ordinary thing to want, and a + /// dead tab must not be the one place the gesture silently does nothing. + /// + [Fact] + public async Task ACopy_IsHeardWithNoSessionRegistered() + { + Start(); + + using var socket = await ConnectAsync(); + + var copied = new List(); + plane.ClipboardCopyRequested += (_, e) => copied.Add(e.Text); + + await SendAsync( + socket, + (byte)TerminalClientOpcode.ClipboardCopy, + Encoding.UTF8.GetBytes("error: nothing was written")); + + await WaitUntilAsync(() => copied.Count > 0); + + copied[0].ShouldBe("error: nothing was written"); + } + + /// + /// An empty copy is dropped rather than forwarded. The clipboard is shared with everything else on the + /// machine, so "copy nothing" would be a gesture that quietly destroys whatever was on it — and the page + /// does not send one, which makes this a guard against a frame from something that is not the page. + /// + [Fact] + public async Task AnEmptyCopy_IsDropped() + { + Start(); + + using var socket = await ConnectAsync(); + + var copies = 0; + plane.ClipboardCopyRequested += (_, _) => Interlocked.Increment(ref copies); + + await SendAsync(socket, (byte)TerminalClientOpcode.ClipboardCopy, []); + + // Followed by one that must be heard, so the assertion below is about ordering rather than about + // having waited long enough: the receive loop is sequential, so a copy arriving after this proves + // the empty one was seen and passed over. + var copied = new List(); + plane.ClipboardCopyRequested += (_, e) => copied.Add(e.Text); + + await SendAsync(socket, (byte)TerminalClientOpcode.ClipboardCopy, "uptime"u8.ToArray()); + await WaitUntilAsync(() => copied.Count > 0); + + copies.ShouldBe(1, "only the second frame carried anything"); + } + + /// + /// The session id is the whole payload of a paste request — it says which pane the pointer was over, and + /// therefore where the answer has to land. Heard with nothing registered for the reason a copy is. + /// + [Fact] + public async Task APasteRequest_CarriesTheSessionItWasMadeIn() + { + Start(); + + using var socket = await ConnectAsync(); + + var asked = new List(); + plane.ClipboardPasteRequested += (_, e) => asked.Add(e.SessionId); + + await SendAsync(socket, (byte)TerminalClientOpcode.ClipboardPaste, [], sessionId: 4242); + + await WaitUntilAsync(() => asked.Count > 0); + + asked[0].ShouldBe(4242u); + } + [Fact] public async Task AFrameForAnUnregisteredSession_IsIgnored() { diff --git a/tests/DodoSSH.Client.Terminal.Tests/TerminalWorkspaceTests.cs b/tests/DodoSSH.Client.Terminal.Tests/TerminalWorkspaceTests.cs index 8d0c214..0d069af 100644 --- a/tests/DodoSSH.Client.Terminal.Tests/TerminalWorkspaceTests.cs +++ b/tests/DodoSSH.Client.Terminal.Tests/TerminalWorkspaceTests.cs @@ -268,6 +268,69 @@ public sealed class TerminalWorkspaceTests .ShouldBeFalse("the tab it names is gone"); } + /// + /// + /// The right-click paste's answer, and it is a separate frame from the snippet one above rather than the + /// same frame with a flag: the page brackets this unconditionally and types a single-line snippet, which + /// is a difference of behaviour and not of degree. See . + /// + /// + /// The payload is the clipboard's text and nothing else — no leading flag byte, because there is no + /// Enter to ask for. A frame that grew one would put a stray character at the front of every paste, and + /// this is what would catch it. + /// + /// + [Fact] + public async Task TheClipboard_ReachesThePaneThatAskedForIt() + { + var connections = new FakeConnectionFactory(blockShellReads: true); + + await using var workspace = CreateWorkspace(connections); + workspace.Start(); + + using var renderer = await ConnectRendererAsync(workspace); + + var sessionId = await workspace.OpenSessionAsync( + Request(), TerminalSize.Default, progress: null, TestContext.Current.CancellationToken); + + // The session's opening frame, read and discarded so the clipboard frame below is the next one. + await ReceiveFrameAsync(renderer); + + (await workspace.SendClipboardAsync( + sessionId, "sudo systemctl restart sshd", TestContext.Current.CancellationToken)) + .ShouldBeTrue("the session is live"); + + var frame = await ReceiveFrameAsync(renderer); + + frame.Opcode.ShouldBe((byte)TerminalServerOpcode.Clipboard); + frame.SessionId.ShouldBe(sessionId); + Encoding.UTF8.GetString(frame.Payload).ShouldBe("sudo systemctl restart sshd"); + } + + /// + /// Refused rather than sent and dropped, for the reason PasteAsync is — with the difference that + /// nothing on screen would have shown it either way, so this answer exists to be honest rather than to + /// be reported. See . + /// + [Fact] + public async Task SendingTheClipboardToADeadSession_SaysSoRatherThanDroppingIt() + { + var connections = new FakeConnectionFactory(); + + await using var workspace = CreateWorkspace(connections); + + var sessionId = await workspace.OpenSessionAsync( + Request(), TerminalSize.Default, progress: null, TestContext.Current.CancellationToken); + + (await workspace.SendClipboardAsync(9999, "uptime", TestContext.Current.CancellationToken)) + .ShouldBeFalse("this workspace never issued that id"); + + await workspace.CloseSessionAsync(sessionId); + + (await workspace.SendClipboardAsync(sessionId, "uptime", TestContext.Current.CancellationToken)) + .ShouldBeFalse("the tab it names is gone"); + } + /// /// The event the tab strip and the phone's keep-alive listen to, so a dot can go out — and a foreground /// notification can come down — the moment a shell exits rather than at the next thing that happens to