Stop the terminal's accessory keys taking the keyboard off it

Ctrl, Esc, Tab, the arrows and the two text-size keys were ordinary Avalonia
buttons sitting over a NativeWebView. An ordinary button takes focus on tap,
which takes it off the WebView — and the package's own OnLostFocus then calls the
adapter's ResignFocus(). So pressing Tab handed the terminal one byte and took
the keyboard away from it, and everything typed afterwards went nowhere.

What makes it worth more than a one-line fix is the symptom. The row goes on
working, because its keys are pressed rather than typed into, so what you see is
a terminal that answers the buttons and ignores the keyboard — which reads as the
session having died rather than as anything to do with focus.

Focusable = false is what a toolbar button is: these keys are an extension of the
keyboard, not a place it should go. The focused element then never changes, so
nothing resigns and nothing has to be handed back — which matters, because the
hand-back is the direction platform-flags already records as the hard one.

The flags file gains the phone's half of that entry, and check 11.10 is the
measurement: this needs a paired hardware keyboard and there is no test on this
head that could stand in for one.

.github/workflows/ci.yml

@@ -103,12 +103,38 @@ jobs:
         with:
           fetch-depth: 0
 
+      # cache: true is actions/cache underneath, keyed on the lock files. That needs a cache server,
+      # which act_runner ships and can be turned off — and when it is off, this does nothing at all and
+      # says nothing about it. See the step below, which is how you find out.
       - uses: actions/setup-dotnet@a98b56852c35b8e3190ac28c8c2271da59106c68 # v6.0.0
         with:
           global-json-file: global.json
           cache: true
           cache-dependency-path: '**/packages.lock.json'
 
+      # ◆ ONE LINE, AND IT EXISTS BECAUSE THE ALTERNATIVE IS ASSUMING.
+      #
+      # A restore that is cold every single run and a cache working perfectly look identical from
+      # outside: both end in a green build, and the only difference is minutes. This job's whole
+      # package cache is the step above, so whether that step works is worth knowing as a fact rather
+      # than as a belief — and it is one `find` before anything has written to the folder.
+      #
+      # It does not fail the build. A runner with no cache server is slow rather than wrong, and a
+      # pipeline that refused to run on one would be a worse trade than the minutes.
+      - name: what the package cache brought
+        run: |
+          set -eu
+
+          root="${NUGET_PACKAGES:-$HOME/.nuget/packages}"
+
+          if [ -d "$root" ]; then
+            echo "$root holds $(find "$root" -mindepth 1 -maxdepth 1 -type d | wc -l) package(s) already."
+          else
+            echo "$root is not there, so this restore is cold."
+            echo "If every run says that, the cache above is doing nothing: check that this runner's"
+            echo "config.yaml has the cache server enabled, and that ACTIONS_CACHE_URL reaches it."
+          fi
+
       # Locked mode fails if packages.lock.json does not match the project files, so a
       # dependency cannot change without the lock file change being reviewed.
       - name: restore
@@ -381,12 +407,31 @@ jobs:
       # does and never otherwise. This runner is persistent, so the first build pays for a JDK, an
       # Android SDK and the android workload — several minutes — and every run after it is a cache hit
       # that prints two lines. A change to that Dockerfile is what buys a new one.
+      # ◆ BUILT ONCE PER DOCKERFILE, NOT ONCE PER RUN.
+      #
+      # The tag is the Dockerfile's own digest, so a tag that exists is by construction an image built
+      # from exactly this Dockerfile — and docker applies a tag only when the build succeeded, so it
+      # cannot be a half-built one. That makes `docker image inspect` a sound cache check rather than a
+      # guess, and it skips the context upload and the layer walk that a no-op `docker build` still does.
+      #
+      # The staleness this trades away is real and bounded. Everything that decides what is *in* the
+      # image — the command-line tools zip, the build-tools version, the workload — is pinned in the
+      # Dockerfile, so changing any of them changes the digest and rebuilds. What drifts is the JDK from
+      # apt. This is a build tool rather than something shipped, so that is the right way round; the
+      # image job takes the opposite trade with --pull, because what it builds is what users run.
+      #
+      # Deleting the image, or editing the Dockerfile, is how you force it.
       - name: the android toolchain image
         run: |
           set -eu
 
           tag="dodossh-android-build:$(sha256sum build/android-build.Dockerfile | cut -c1-16)"
-          docker build -t "$tag" -f build/android-build.Dockerfile build
+
+          if docker image inspect "$tag" >/dev/null 2>&1; then
+            echo "$tag is already on this runner"
+          else
+            docker build -t "$tag" -f build/android-build.Dockerfile build
+          fi
 
           echo "ANDROID_BUILD_IMAGE=$tag" >> "$GITHUB_ENV"
           echo "$tag"

docs/manual-checks.md

@@ -1334,6 +1334,23 @@ by gesture or by the arrow, returns to Settings and not to HOSTS; a second back
 **Failure means:** `ShellScreen.Vault` is missing from `IsMoreSurface` or from the back gesture's first
 case, and those two have to move together — the switch mirrors that property by construction.
 
+### 11.10 The accessory keys do not cost the terminal its keyboard · **needs a hardware keyboard**
+
+With a shell open and a Bluetooth or USB keyboard paired, type into the terminal and confirm it arrives.
+Now tap **Tab** on the accessory row, or **A+**, and type again.
+
+**Pass:** the byte the key sent arrives, and so does everything typed after it. The terminal still has the
+keyboard.
+
+**Failure means:** the buttons on that row have become focusable again. An ordinary Avalonia button takes
+focus on tap, which takes it off the `NativeWebView` — and the package's own `OnLostFocus` then resigns the
+page's focus. The row goes on working, because its keys are pressed rather than typed into, so the symptom
+is a terminal that answers the buttons and ignores the keyboard: it reads as the session having died. See
+`Focusable = false` in `TerminalScreen.axaml.cs` and the focus entry in `docs/platform-flags.md`.
+
+Worth doing on the software keyboard too, where the same fault shows as the keyboard closing on the first
+tap of an arrow key.
+
 ---
 
 ## Phase 12 — Shared vaults: the operations that span two accounts

docs/platform-flags.md

@@ -168,6 +168,18 @@ that reports `GetFocus()`, the class name of the window holding it, and the page
   could fire. Not Escape, and not a bare F6: both are keys a TUI legitimately binds, and Ctrl+Shift is the
   range terminal emulators conventionally keep for themselves.
 
+**On the phone the same asymmetry arrives through a button, and the fix is one property.** The terminal's
+accessory row — Ctrl, Esc, Tab, the arrows, and the two text-size keys — is a set of ordinary Avalonia
+buttons over a `NativeWebView`. An ordinary button takes focus on tap, which takes it off the WebView, and
+`OnLostFocus` then calls the adapter's `ResignFocus()`. So pressing Tab handed the terminal one byte and
+took the keyboard away from it: everything typed afterwards on a hardware keyboard went nowhere.
+
+The symptom is what makes it worth an entry. The row goes on working — its keys are *pressed* rather than
+typed into — so what a user sees is a terminal that answers the buttons and ignores the keyboard, which
+reads as the session having died rather than as a focus problem. `Focusable = false` is what a toolbar
+button is, and it means the focused element never changes, so nothing resigns and nothing has to be handed
+back. Every button on that row carries it; check 11.10 is the measurement.
+
 None of this is covered by a test, and cannot be here: headless Avalonia has no native window, so a
 headless test renders and focuses correctly and would confirm the wrong belief. What the suite covers is
 the plumbing that drives it — that connecting asks for focus once per session, that a failed connect does

scripts/ci-android.sh

@@ -26,14 +26,37 @@ echo "==> restore"
 # Locked mode, the same gate the solution restore gives every other project. This head is not in
 # DodoSSH.slnx — it needs a workload the other jobs have no reason to install — so this is the only
 # place its lock file is ever checked. It has silently gone stale before.
+#
+# Restored without a channel and built with one, which is safe and worth one line: the two channel
+# PropertyGroups change the application id, the version and some assembly metadata, and none of those
+# is in the restore graph. If a channel ever gains a PackageReference of its own, this splits and the
+# restore has to move under the same -p: as the build.
+#
+# Into /root/.nuget/packages, which the job mounts as a named volume — see the workflow. That is the
+# whole of this job's package caching and it is what took a run from eight minutes to two and a half.
 dotnet restore "$project" --locked-mode
 
-echo "==> build"
-dotnet build "$project" \
-  --no-restore --configuration Release \
-  "-p:Aapt2ToolPath=$build_tools"
-
 echo "==> package the nightly"
+# ◆ ONE BUILD, WHERE THERE WERE TWO, AND THE FIRST WAS NOT A CHECK.
+#
+# There used to be a plain `dotnet build` above this. It looked like a cheap compile before the
+# expensive packaging and it was neither: SignAndroidPackage depends on Build, so this line compiles
+# everything anyway, and the build above ran with no -p:DodoChannel — which means it ran as the
+# *release* channel. Different ApplicationId, different ApplicationVersion, different assembly
+# metadata; MSBuild treats a different set of global properties as a different project instance, so
+# not one output was reused. It was a complete second compile of the whole reference closure,
+# producing an APK for a channel this job must never build, thrown away unread.
+#
+# Removing it does not lose a check. Anything that fails in a compile fails in this one, and the
+# link-time failures below are the ones only packaging finds.
+#
+# Measured in the toolchain image with a warm package volume, same commit, same APK out of both:
+#
+#   two builds   2m52 + 2m26   5m21 total
+#   one build            3m01   3m04 total
+#
+# versionCode 203, versionName 0.0.0-alpha.0.136, dev.dodotech.dodossh.nightly, either way.
+#
 # Packaging rather than only compiling, because the two failures this head is most exposed to are both
 # link-time: a native library with no android ABI, and a managed assembly that resolves for net10.0 but
 # has nothing to dex. Neither shows up in a compile.

src/DodoSSH.Client.Android/Views/TerminalScreen.axaml

@@ -347,6 +347,11 @@
           Disabled at the ends rather than clamping silently. A button that keeps accepting taps and does
           nothing reads as the terminal having stopped responding, which is the one thing this screen must
           never look like.
+
+          ◆ Focusable="False", for the reason the accessory keys carry in code — see
+          TerminalScreen.axaml.cs. Every button on this row sits over a terminal somebody is typing into,
+          and an ordinary one takes Avalonia's focus off the NativeWebView on tap, which makes the package
+          resign the page's focus. Making the text smaller must not cost the keyboard.
         -->
         <StackPanel Grid.Column="1" Orientation="Horizontal" Spacing="5" Margin="8,0,12,0"
                     VerticalAlignment="Center">
@@ -356,7 +361,7 @@
           <Button Classes="row" MinHeight="30" Height="30" MinWidth="40" Padding="0" CornerRadius="9"
                   HorizontalContentAlignment="Center" VerticalContentAlignment="Center"
                   Background="{StaticResource Panel}" BorderBrush="{StaticResource BorderMid}"
-                  BorderThickness="1"
+                  BorderThickness="1" Focusable="False"
                   Command="{Binding ShrinkTerminalFontCommand}"
                   IsEnabled="{Binding CanShrinkTerminalFont}">
             <TextBlock Classes="mono" FontSize="13" Text="A−" />
@@ -365,7 +370,7 @@
           <Button Classes="row" MinHeight="30" Height="30" MinWidth="40" Padding="0" CornerRadius="9"
                   HorizontalContentAlignment="Center" VerticalContentAlignment="Center"
                   Background="{StaticResource Panel}" BorderBrush="{StaticResource BorderMid}"
-                  BorderThickness="1"
+                  BorderThickness="1" Focusable="False"
                   Command="{Binding EnlargeTerminalFontCommand}"
                   IsEnabled="{Binding CanEnlargeTerminalFont}">
             <TextBlock Classes="mono" FontSize="15" Text="A+" />

src/DodoSSH.Client.Android/Views/TerminalScreen.axaml.cs

@@ -166,6 +166,20 @@ internal sealed partial class TerminalScreen : UserControl
                 BorderThickness = new Thickness(1),
                 Foreground = Palette("TextDim"),
                 HorizontalContentAlignment = HorizontalAlignment.Center,
+
+                // ◆ NOT FOCUSABLE, AND THAT IS THE WHOLE CONTROL RATHER THAN A DETAIL.
+                //
+                // These keys are an extension of the keyboard, not a place the keyboard should go. As
+                // ordinary buttons they took Avalonia's focus on tap, which takes it off the NativeWebView
+                // — and the package's own OnLostFocus then calls the adapter's ResignFocus(). So pressing
+                // Tab or an arrow handed the terminal one byte and took the keyboard away from it: the next
+                // thing typed on a hardware keyboard went nowhere, and the row went on working because its
+                // buttons are pressed rather than typed into, which is what makes it look like the terminal
+                // had died instead.
+                //
+                // Focusable=false is what a toolbar button is, and it means the focused element never
+                // changes: the WebView is still it, so nothing resigns and nothing has to be handed back.
+                Focusable = false,
             };
 
             if (latches)