From ef0a1bfb41e532d1a644aa769143536ea0952444 Mon Sep 17 00:00:00 2001 From: Aleksandr Chasnyk <69671996+ami3go@users.noreply.github.com> Date: Thu, 20 Aug 2026 13:17:52 +0300 Subject: [PATCH 1/2] Fix review findings across drawer, clipboard write-through, desktop and release automation Second review pass over the v0.11.6/v0.11.7 range, run as four sequential workers over the areas the first pass did not reach. Clipboard write-through (apps/web-client): - The four status headlines the Copy button depends on are now defined once in src/clipboard-status.ts and imported by both controller.ts and main.ts. bindAndroidCopyWriteThrough recognises the controller's reply by matching the status line against exact strings, so rewording one at a setStatus call site silently stopped the Copy button writing to the PC clipboard -- with a green suite, because the existing tests pinned only the main.ts side. - The execCommand fallback had no generation guard, so a superseded copy still stole focus and wrote its result over a newer request. It also copied the live textarea selection rather than the text it had validated, so the string placed on the clipboard could differ from the one the status claimed. - On timeout the write-through overwrote whatever terminal status the controller had produced. A disconnect ("Stream stopped") or an over-limit clipboard ("Clipboard skipped ... exceeds N KiB") was replaced with "Copy not confirmed", swapping an accurate diagnosis for a wrong one. It now only claims failure when nothing else has explained the silence. - Ctrl+C had no session guard, reinstating the "not confirmed" message that controller.ts deliberately suppresses when no device is attached. - The fallback now restores focus to wherever it was instead of assuming the canvas, since parking focus there sends the next keystroke to Android. - Added an event.repeat guard, and replaced a byte-for-byte copy of clipboardShortcut with an import of the tested original. Drawer: - openGroup forced focus to the mirrored canvas two frames after the Clipboard group opened. Its comment justified this by controller.ts focusing the fallback textarea -- a listener deleted when Ctrl+V moved to a document-level paste handler. The guard outlived its cause and now only steals focus from a user typing into the textarea, injecting their keystrokes into Android. - The @import of droidwebdisplay-main-drawer-core.css carried no ?v=, leaving 531 lines of drawer layout outside the cache-busting the entry sheet uses. A stale copy after an upgrade renders every drawer group stacked and visible. Desktop: - The Summary "Local URL" was a live hyperlink in every server state while the header Open button beside it was correctly gated, so stopping the server left a blue link that leads to ERR_CONNECTION_REFUSED. It is now plain text unless the server can serve it, and the colour comes from the palette instead of an inline literal that silently overrode the QLabel#summaryUrl rule. - _status_failed left the header Open button enabled indefinitely, because only _apply_snapshot gated it and that path does not run on a probe failure. - Added QPushButton#headerIconButton:disabled and :pressed. QSS specificity is CSS2.1, so the id selector outranked the bare pseudo-class rules and a disabled header button kept its enabled background with no press feedback. - Removed setFixedSize(38, 38), which fought the 36px box in theme.py. - Removed a no-op summary_layout.setAlignment(Qt.AlignTop): the one-argument form positions a layout within a parent layout item, and this one is installed directly on the card. addStretch(1) below is what actually works. Release automation: - pages.yml pinned four actions to mutable tags while every other workflow pins to a commit SHA, and it holds pages: write plus id-token: write. All four are now pinned. - release.yml granted contents: write and actions: write workflow-wide. Both are now job-scoped; actions: write reaches only publish, which needs it. It also permits deleting the workflow runs and artifacts that constitute the gate evidence. - The release request was selected with `head -n 1` over path-sorted output. Two request files in one push published only the alphabetically first, and a push touching an old request file would attempt a version downgrade. It now requires exactly one ADDED file and fails loudly otherwise. - Added workflow_dispatch. prepare self-aborts once VERSION already matches, so a failed publish previously had no re-entry point -- which is why v0.11.6 needed nine hand-written recovery commits. - Added timeout-minutes to both jobs; publish holds a global concurrency slot and could hold it for the six-hour default. Tests and docs: - Two new tests were vacuous. header-brand-layout used [\s\S]* spanning the whole file, so deleting the property under test was satisfied by a copy of it elsewhere; both are now anchored to the rule body. header-status-indicator asserted drawerSource.includes("back"), which is unconditionally true because the file contains callback, fallback and background. - test_layout_regression pinned the no-op setAlignment call and guarded a regression by exact local variable name, so reintroducing the footer button as `open_btn` would have passed. It now asserts the user-visible string. - Removed an assertion on a message that no longer exists anywhere in the repo. - docs/contracts/CLIPBOARD.md described a manual-Copy flow the code does not implement, and so failed to protect the one thing that keeps breaking. It now documents the real mechanism, why the write must stay inside the gesture, the status-string coupling, and the listener-ordering dependency. verify_static_client returned FAIL on the committed tree before this change: assets/main.js and assets/browser-support.js checksums were left stale by an earlier commit. CI did not catch it because the gate runs npm run build first, which rewrites both dist/ and the manifest. Regenerated, and the two new clipboard-status files added, so the manifest now covers 95 files and passes. NOT fixed, deliberately: release.yml still pushes the version bump to main before any gate runs, which is what left v0.11.5 as a changelog entry with no tag and no release. Fixing it means restructuring prepare/publish so the gate runs against the prepared commit before it reaches main. That is a real redesign of untestable-from-here automation that has already wedged main twice, and it should be done deliberately rather than blind. The workflow_dispatch added here at least provides the manual recovery path that was missing. No node/npm or pytest available. dist/ was hand-applied and the manifest regenerated. Executed directly and passing: 9 clipboard contract tests (including a new one pinning the sentinel values), plus the clipboard server-mode, toolbar layout, phase 9, desktop layout and Windows creation-kwargs suites -- 26 fixture-free tests, 0 failures. Not run: anything needing PyQt, node, or a browser. None of the UI changes have been seen rendered. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XfT5Z3Z24bjC8qtqERGpMQ --- .github/workflows/pages.yml | 8 +-- .github/workflows/release.yml | 37 ++++++++-- apps/web-client/dist-manifest.json | 30 +++++--- .../dist/assets/clipboard-status.d.ts | 6 ++ .../dist/assets/clipboard-status.js | 19 +++++ apps/web-client/dist/assets/controller.js | 9 +-- apps/web-client/dist/assets/main.js | 69 +++++++++++++++---- .../dist/droidwebdisplay-main-drawer.css | 5 +- .../dist/droidwebdisplay-main-drawer.js | 15 ++-- apps/web-client/src/clipboard-status.ts | 19 +++++ apps/web-client/src/controller.ts | 9 +-- apps/web-client/src/main.ts | 65 +++++++++++++---- .../static/droidwebdisplay-main-drawer.css | 5 +- .../static/droidwebdisplay-main-drawer.js | 15 ++-- .../tests/header-brand-layout.test.mjs | 4 +- .../tests/header-status-indicator.test.mjs | 2 +- docs/contracts/CLIPBOARD.md | 39 ++++++++++- droid_web_display/desktop/gui.py | 26 +++++-- droid_web_display/desktop/theme.py | 12 ++++ tests/desktop/test_layout_regression.py | 11 ++- .../test_clipboard_sync_regression.py | 37 ++++++++-- 21 files changed, 345 insertions(+), 97 deletions(-) create mode 100644 apps/web-client/dist/assets/clipboard-status.d.ts create mode 100644 apps/web-client/dist/assets/clipboard-status.js create mode 100644 apps/web-client/src/clipboard-status.ts diff --git a/.github/workflows/pages.yml b/.github/workflows/pages.yml index 0b1d673..e91261a 100644 --- a/.github/workflows/pages.yml +++ b/.github/workflows/pages.yml @@ -26,13 +26,13 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 - name: Setup Pages - uses: actions/configure-pages@v5 + uses: actions/configure-pages@983d7736d9b0ae728b81ab479565c72886d7745b # v5 - name: Upload site artifact - uses: actions/upload-pages-artifact@v3 + uses: actions/upload-pages-artifact@56afc609e74202658d3ffba0e8f6dda462b719fa # v3 with: path: ./site - name: Deploy to GitHub Pages id: deployment - uses: actions/deploy-pages@v5 + uses: actions/deploy-pages@cd2ce8fcbc39b97be8ca5fce6e763baed58fa128 # v5 diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 73df4bd..77a356a 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -5,10 +5,17 @@ on: branches: [main] paths: - '.github/releases/v*.md' + # Manual re-entry. `prepare` self-aborts once VERSION already matches, so + # without this a failed publish leaves no way to finish the release except + # hand-crafted recovery commits. + workflow_dispatch: + inputs: + version: + description: 'Version to publish, e.g. 0.11.8 (must have .github/releases/v.md)' + required: true permissions: - contents: write - actions: write + contents: read concurrency: group: droidwebdisplay-release @@ -17,6 +24,9 @@ concurrency: jobs: prepare: runs-on: ubuntu-24.04 + timeout-minutes: 15 + permissions: + contents: write outputs: version: ${{ steps.prepare.outputs.version }} sha: ${{ steps.prepare.outputs.sha }} @@ -30,8 +40,21 @@ jobs: shell: bash run: | set -euo pipefail - request=$(git diff --name-only HEAD^ HEAD -- '.github/releases/v*.md' | head -n 1) - test -n "$request" + if [ -n "${{ github.event.inputs.version }}" ]; then + request=".github/releases/v${{ github.event.inputs.version }}.md" + test -f "$request" + else + # head -n 1 silently published only the alphabetically-first file and + # could select an OLD request file touched by an unrelated edit, + # attempting a version downgrade. Require exactly one added file. + mapfile -t requests < <(git diff --name-only --diff-filter=A HEAD^ HEAD -- '.github/releases/v*.md') + if [ "${#requests[@]}" -ne 1 ]; then + echo "expected exactly one added release request, got ${#requests[@]}: ${requests[*]:-none}" >&2 + exit 1 + fi + request="${requests[0]}" + test -f "$request" + fi file=$(basename "$request") version=${file#v} version=${version%.md} @@ -121,6 +144,12 @@ jobs: publish: needs: prepare runs-on: ubuntu-24.04 + # Bounded so a hung smoke job cannot hold the droidwebdisplay-release + # concurrency slot for the six-hour default and block every later release. + timeout-minutes: 60 + permissions: + contents: write + actions: write env: GH_TOKEN: ${{ github.token }} RELEASE_VERSION: ${{ needs.prepare.outputs.version }} diff --git a/apps/web-client/dist-manifest.json b/apps/web-client/dist-manifest.json index 967d38c..dc3e329 100644 --- a/apps/web-client/dist-manifest.json +++ b/apps/web-client/dist-manifest.json @@ -70,14 +70,24 @@ }, { "path": "assets/browser-support.js", - "bytes": 671, - "sha256": "010a0b76cbe8bb6ae9e20ff5a60836b4fcfe94010678ee09fcaf349380675529" + "bytes": 1444, + "sha256": "1a67546561cd6236156c8282187e9324d176ae7922d896a24de822647abcd37a" }, { "path": "assets/browser-support.js.map", "bytes": 719, "sha256": "412b951eb2bbf8b3614024506c288498b5033e62c07a49385cd754101f0e8d74" }, + { + "path": "assets/clipboard-status.d.ts", + "bytes": 212, + "sha256": "3098fe0bb9f107c24d91673af1024d329553fa16922caf965f92f86b93b72b6d" + }, + { + "path": "assets/clipboard-status.js", + "bytes": 864, + "sha256": "509cb41e03eb47e68b55bed5f2aac9158b7a177b4d4da9209b9b7c6be8abba50" + }, { "path": "assets/controller.d.ts", "bytes": 4252, @@ -85,8 +95,8 @@ }, { "path": "assets/controller.js", - "bytes": 57121, - "sha256": "35577dbe1abb009a81530d785c6243c7f79947c8b5d0bc3b81d4c0f3cc7ee230" + "bytes": 57213, + "sha256": "9e2df97ba6f1876070322b09adec3284477b9983c466fa1a791a4c469ff56d63" }, { "path": "assets/controller.js.map", @@ -130,8 +140,8 @@ }, { "path": "assets/main.js", - "bytes": 14522, - "sha256": "746ee71599bdc6625c5eccaecef8869ebd3b17daf434a490e7830635f7b59b3f" + "bytes": 21120, + "sha256": "a2946a8f334091a636c56410183bcb45502c66408aac59ee8ede8e3ca9139af3" }, { "path": "assets/main.js.map", @@ -235,13 +245,13 @@ }, { "path": "droidwebdisplay-main-drawer.css", - "bytes": 5670, - "sha256": "fdf37a0bcb9ad994d2771f5ae8c6aa8f1871f401a206d28e76e62df1fa4bc0aa" + "bytes": 5917, + "sha256": "dd8ae94da10bcd2ef6596f81cbf49da938cde5e7403b0fdb4430a0ef5bdd57f2" }, { "path": "droidwebdisplay-main-drawer.js", - "bytes": 26484, - "sha256": "57291e2c349f95159d731dd90010f42bf0748fc019a4c32e922d6fed72af097d" + "bytes": 26302, + "sha256": "18214ff7a6bb60fb75e5b31aaa59e178cf47d5183c170ccdff0c2ae7abc45c6e" }, { "path": "favicon.svg", diff --git a/apps/web-client/dist/assets/clipboard-status.d.ts b/apps/web-client/dist/assets/clipboard-status.d.ts new file mode 100644 index 0000000..9f69f4a --- /dev/null +++ b/apps/web-client/dist/assets/clipboard-status.d.ts @@ -0,0 +1,6 @@ +export declare const CLIPBOARD_STATUS: { + readonly copying: "Copying"; + readonly copied: "Clipboard copied"; + readonly received: "Clipboard received"; + readonly notConfirmed: "Copy not confirmed"; +}; diff --git a/apps/web-client/dist/assets/clipboard-status.js b/apps/web-client/dist/assets/clipboard-status.js new file mode 100644 index 0000000..12b6a20 --- /dev/null +++ b/apps/web-client/dist/assets/clipboard-status.js @@ -0,0 +1,19 @@ +/** Status headlines shared between the controller and the copy write-through. + * + * `bindAndroidCopyWriteThrough` in main.ts has to perform + * `navigator.clipboard.writeText` inside the click or keypress that asked for + * the copy, because that write needs transient user activation. It therefore + * watches the status line for the controller's reply instead of awaiting a + * promise across the gesture boundary. + * + * That makes these four strings a contract between the two modules, not display + * copy: rewording one at a call site silently stops the Copy button writing to + * the PC clipboard, with no test failure and no console error. Change them here + * so both sides move together. + */ +export const CLIPBOARD_STATUS = { + copying: "Copying", + copied: "Clipboard copied", + received: "Clipboard received", + notConfirmed: "Copy not confirmed", +}; diff --git a/apps/web-client/dist/assets/controller.js b/apps/web-client/dist/assets/controller.js index 74652e6..b6d40f6 100644 --- a/apps/web-client/dist/assets/controller.js +++ b/apps/web-client/dist/assets/controller.js @@ -1,5 +1,6 @@ import { ControlMessageType, DeviceMessageType, ScrcpyV41Adapter, } from "@droid-web-display/scrcpy-protocol"; import { BridgeApi } from "./api.js"; +import { CLIPBOARD_STATUS } from "./clipboard-status.js"; import { alignedFlexSize, buildSessionRequest, validateDisplayForm, VIRTUAL_DISPLAY_PROFILES, } from "./display-config.js"; import { androidClipboardCopyMessage, androidKeyPress, clipboardMessage, clipboardShortcut, keyboardMessages, mapClientPoint, textInjectionMessages } from "./input.js"; import { WebCodecsVideoRenderer } from "./video-renderer.js"; @@ -687,11 +688,11 @@ export class DroidWebDisplayController { : "Android clipboard was copied to the PC clipboard."); } catch { - this.setStatus("Clipboard received", "Android clipboard is available in the clipboard panel; browser write permission was unavailable."); + this.setStatus(CLIPBOARD_STATUS.received, "Android clipboard is available in the clipboard panel; browser write permission was unavailable."); } } else { - this.setStatus("Clipboard received", "Android clipboard is available in the clipboard panel."); + this.setStatus(CLIPBOARD_STATUS.received, "Android clipboard is available in the clipboard panel."); } } } @@ -822,9 +823,9 @@ export class DroidWebDisplayController { this.#copyShortcutPending = false; if (!this.#protocolSession) return; - this.setStatus("Copy not confirmed", `Android did not report a clipboard update for ${source}. The previous PC clipboard was left unchanged.`); + this.setStatus(CLIPBOARD_STATUS.notConfirmed, `Android did not report a clipboard update for ${source}. The previous PC clipboard was left unchanged.`); }, 1_200); - this.setStatus("Copying", `Requesting ${source} from the focused Android selection…`); + this.setStatus(CLIPBOARD_STATUS.copying, `Requesting ${source} from the focused Android selection…`); } completeAndroidCopyRequest() { const pending = this.#copyShortcutPending; diff --git a/apps/web-client/dist/assets/main.js b/apps/web-client/dist/assets/main.js index ca3f66b..e55b285 100644 --- a/apps/web-client/dist/assets/main.js +++ b/apps/web-client/dist/assets/main.js @@ -1,5 +1,7 @@ import { inspectBrowserCapabilities } from "./browser-support.js"; import { DroidWebDisplayController } from "./controller.js"; +import { CLIPBOARD_STATUS } from "./clipboard-status.js"; +import { clipboardShortcut } from "./input.js"; import { AutoDownloadController } from "./auto-download-controller.js"; import { TransferController } from "./transfer-controller.js"; import { RunningAppController } from "./running-app-controller.js"; @@ -17,8 +19,25 @@ function bindAndroidCopyWriteThrough() { const clipboardText = required("#clipboard-text"); const status = required("#status"); const details = required("#details"); + const statusContainer = required("#connection-status"); let generation = 0; + /** Mirror the side effects of controller.setStatus for the fields that describe + the status line itself. data-state is deliberately untouched: a copy does + not change the connection state, but title and aria-label describe the + visible text and go stale if only textContent is written. */ + const writeStatus = (headline, detail) => { + status.textContent = headline; + details.textContent = detail; + statusContainer.title = detail; + statusContainer.setAttribute("aria-label", `${statusContainer.dataset.state ?? "unknown"}: ${headline}. ${detail}`); + }; const finishCopy = async () => { + // Ctrl+C reaches this listener whenever the canvas has focus, including with + // no device attached. The controller suppresses its own "not confirmed" + // message in that case, so claiming a failed copy here would reinstate the + // very message it takes care to avoid. + if (statusContainer.dataset.state !== "connected") + return; const request = ++generation; const initialText = clipboardText.value; const initialStatus = status.textContent?.trim() ?? ""; @@ -30,11 +49,11 @@ function bindAndroidCopyWriteThrough() { const currentStatus = status.textContent?.trim() ?? ""; const textChanged = clipboardText.value !== initialText; const statusChanged = currentStatus !== initialStatus; - if (statusChanged && currentStatus === "Copy not confirmed") + if (statusChanged && currentStatus === CLIPBOARD_STATUS.notConfirmed) return; - if ((textChanged || statusChanged) && currentStatus === "Clipboard copied") + if ((textChanged || statusChanged) && currentStatus === CLIPBOARD_STATUS.copied) return; - if (textChanged || (statusChanged && currentStatus === "Clipboard received")) { + if (textChanged || (statusChanged && currentStatus === CLIPBOARD_STATUS.received)) { responseObserved = true; break; } @@ -43,45 +62,65 @@ function bindAndroidCopyWriteThrough() { if (request !== generation) return; if (!responseObserved) { - status.textContent = "Copy not confirmed"; - details.textContent = "Android did not return a new clipboard value. The previous PC clipboard was left unchanged."; + // Only claim "not confirmed" when nothing else has explained the silence. + // A disconnect or an over-limit clipboard sets its own terminal status, + // and overwriting it replaces an accurate diagnosis with a wrong one. + const currentStatus = status.textContent?.trim() ?? ""; + if (currentStatus === initialStatus || currentStatus === CLIPBOARD_STATUS.copying) { + writeStatus(CLIPBOARD_STATUS.notConfirmed, "Android did not return a new clipboard value. The previous PC clipboard was left unchanged."); + } return; } const text = clipboardText.value; if (!text) { - status.textContent = "Copy not confirmed"; - details.textContent = "Android did not return clipboard text."; + writeStatus(CLIPBOARD_STATUS.notConfirmed, "Android did not return clipboard text."); return; } try { await navigator.clipboard.writeText(text); if (request !== generation) return; - status.textContent = "Clipboard copied"; - details.textContent = "Android selection was copied to the PC clipboard."; + writeStatus(CLIPBOARD_STATUS.copied, "Android selection was copied to the PC clipboard."); return; } catch { + // Every other exit re-checks the generation; without it here a superseded + // copy still yanks focus and writes its result over the newer request. + if (request !== generation) + return; + // execCommand copies the live selection, so it can only be trusted while + // the textarea still holds the text this request validated. + if (clipboardText.value !== text) + return; const selectionStart = clipboardText.selectionStart; const selectionEnd = clipboardText.selectionEnd; + const previouslyFocused = document.activeElement; clipboardText.focus(); clipboardText.select(); const copied = document.execCommand("copy"); clipboardText.setSelectionRange(selectionStart, selectionEnd); - canvas.focus(); + // Restore where focus actually was rather than assuming the canvas: this + // path runs on every copy in browsers without the async Clipboard API, and + // parking focus on the canvas sends the next keystroke to Android. + if (previouslyFocused instanceof HTMLElement) + previouslyFocused.focus(); + else + canvas.focus(); + if (request !== generation) + return; if (copied) { - status.textContent = "Clipboard copied"; - details.textContent = "Android selection was copied to the PC clipboard using the browser fallback."; + writeStatus(CLIPBOARD_STATUS.copied, "Android selection was copied to the PC clipboard using the browser fallback."); } else { - status.textContent = "Clipboard received"; - details.textContent = "Android clipboard reached the browser, but the browser blocked writing to the PC clipboard."; + writeStatus(CLIPBOARD_STATUS.received, "Android clipboard reached the browser, but the browser blocked writing to the PC clipboard."); } } }; button.addEventListener("click", () => void finishCopy()); canvas.addEventListener("keydown", (event) => { - if (event.altKey || (!event.ctrlKey && !event.metaKey) || event.key.toLowerCase() !== "c") + // Reuse the controller's predicate instead of restating it: a copy of this + // condition drifts out of step with the keydown handler it shadows. + if (event.repeat || clipboardShortcut(event) !== "copy") return; void finishCopy(); }); diff --git a/apps/web-client/dist/droidwebdisplay-main-drawer.css b/apps/web-client/dist/droidwebdisplay-main-drawer.css index e77b8b7..426d9b4 100644 --- a/apps/web-client/dist/droidwebdisplay-main-drawer.css +++ b/apps/web-client/dist/droidwebdisplay-main-drawer.css @@ -1,4 +1,7 @@ -@import url("./droidwebdisplay-main-drawer-core.css"); +/* Versioned to match the ?v= on the entry sheet in index.html. Without it the + 531 lines of drawer layout below sit outside cache-busting, and a stale + copy after an upgrade renders every drawer group stacked and visible. */ +@import url("./droidwebdisplay-main-drawer-core.css?v=0.11.2-native5"); /* Header brand/status lockup v2. Keep the brand on one header row, remove the legacy rail-style pseudo logo, diff --git a/apps/web-client/dist/droidwebdisplay-main-drawer.js b/apps/web-client/dist/droidwebdisplay-main-drawer.js index cdcf879..8314d8c 100644 --- a/apps/web-client/dist/droidwebdisplay-main-drawer.js +++ b/apps/web-client/dist/droidwebdisplay-main-drawer.js @@ -46,16 +46,11 @@ const label = root().querySelector(`[data-group="${group}"] .gb-rail-label`)?.textContent || 'Tools'; const title = root().querySelector('.gb-drawer-title'); if (title) title.textContent = label; drawer()?.classList.add('gb-open'); drawer()?.setAttribute('aria-hidden', 'false'); - if (group === 'clipboard') { - // controller.ts historically focused the fallback textarea one frame after - // opening Clipboard. That makes the global Ctrl+V route treat the paste as - // page editing instead of Android input. Return focus to the mirrored screen - // after that callback has run; users can still click the textarea explicitly. - window.requestAnimationFrame(() => window.requestAnimationFrame(() => { - const screen = document.getElementById('screen'); - if (screen instanceof HTMLCanvasElement) screen.focus(); - })); - } + // No focus handling here on purpose. This used to force focus back to the + // mirrored screen because controller.ts focused the fallback textarea when + // the Clipboard group opened; that listener was deleted when Ctrl+V moved to + // a document-level paste handler, so the guard now only steals focus from a + // user typing into the textarea and injects their keystrokes into Android. } function closeDrawer() { if (pinned || !root()) return; diff --git a/apps/web-client/src/clipboard-status.ts b/apps/web-client/src/clipboard-status.ts new file mode 100644 index 0000000..31aa817 --- /dev/null +++ b/apps/web-client/src/clipboard-status.ts @@ -0,0 +1,19 @@ +/** Status headlines shared between the controller and the copy write-through. + * + * `bindAndroidCopyWriteThrough` in main.ts has to perform + * `navigator.clipboard.writeText` inside the click or keypress that asked for + * the copy, because that write needs transient user activation. It therefore + * watches the status line for the controller's reply instead of awaiting a + * promise across the gesture boundary. + * + * That makes these four strings a contract between the two modules, not display + * copy: rewording one at a call site silently stops the Copy button writing to + * the PC clipboard, with no test failure and no console error. Change them here + * so both sides move together. + */ +export const CLIPBOARD_STATUS = { + copying: "Copying", + copied: "Clipboard copied", + received: "Clipboard received", + notConfirmed: "Copy not confirmed", +} as const; diff --git a/apps/web-client/src/controller.ts b/apps/web-client/src/controller.ts index 6b2aa94..7a359cc 100644 --- a/apps/web-client/src/controller.ts +++ b/apps/web-client/src/controller.ts @@ -6,6 +6,7 @@ import { type ScrcpyV41Session, } from "@droid-web-display/scrcpy-protocol"; import { BridgeApi, type StartSessionRequest } from "./api.js"; +import { CLIPBOARD_STATUS } from "./clipboard-status.js"; import { alignedFlexSize, buildSessionRequest, @@ -739,10 +740,10 @@ export class DroidWebDisplayController { ? "Ctrl+C copied the Android selection to the PC clipboard." : "Android clipboard was copied to the PC clipboard."); } catch { - this.setStatus("Clipboard received", "Android clipboard is available in the clipboard panel; browser write permission was unavailable."); + this.setStatus(CLIPBOARD_STATUS.received, "Android clipboard is available in the clipboard panel; browser write permission was unavailable."); } } else { - this.setStatus("Clipboard received", "Android clipboard is available in the clipboard panel."); + this.setStatus(CLIPBOARD_STATUS.received, "Android clipboard is available in the clipboard panel."); } } } @@ -873,9 +874,9 @@ export class DroidWebDisplayController { if (!this.#copyShortcutPending) return; this.#copyShortcutPending = false; if (!this.#protocolSession) return; - this.setStatus("Copy not confirmed", `Android did not report a clipboard update for ${source}. The previous PC clipboard was left unchanged.`); + this.setStatus(CLIPBOARD_STATUS.notConfirmed, `Android did not report a clipboard update for ${source}. The previous PC clipboard was left unchanged.`); }, 1_200); - this.setStatus("Copying", `Requesting ${source} from the focused Android selection…`); + this.setStatus(CLIPBOARD_STATUS.copying, `Requesting ${source} from the focused Android selection…`); } private completeAndroidCopyRequest(): boolean { diff --git a/apps/web-client/src/main.ts b/apps/web-client/src/main.ts index d146766..b459039 100644 --- a/apps/web-client/src/main.ts +++ b/apps/web-client/src/main.ts @@ -1,5 +1,7 @@ import { inspectBrowserCapabilities } from "./browser-support.js"; import { DroidWebDisplayController } from "./controller.js"; +import { CLIPBOARD_STATUS } from "./clipboard-status.js"; +import { clipboardShortcut } from "./input.js"; import { AutoDownloadController } from "./auto-download-controller.js"; import { TransferController } from "./transfer-controller.js"; import { RunningAppController } from "./running-app-controller.js"; @@ -18,9 +20,27 @@ function bindAndroidCopyWriteThrough(): void { const clipboardText = required("#clipboard-text"); const status = required("#status"); const details = required("#details"); + const statusContainer = required("#connection-status"); let generation = 0; + /** Mirror the side effects of controller.setStatus for the fields that describe + the status line itself. data-state is deliberately untouched: a copy does + not change the connection state, but title and aria-label describe the + visible text and go stale if only textContent is written. */ + const writeStatus = (headline: string, detail: string): void => { + status.textContent = headline; + details.textContent = detail; + statusContainer.title = detail; + statusContainer.setAttribute("aria-label", `${statusContainer.dataset.state ?? "unknown"}: ${headline}. ${detail}`); + }; + const finishCopy = async (): Promise => { + // Ctrl+C reaches this listener whenever the canvas has focus, including with + // no device attached. The controller suppresses its own "not confirmed" + // message in that case, so claiming a failed copy here would reinstate the + // very message it takes care to avoid. + if (statusContainer.dataset.state !== "connected") return; + const request = ++generation; const initialText = clipboardText.value; const initialStatus = status.textContent?.trim() ?? ""; @@ -32,9 +52,9 @@ function bindAndroidCopyWriteThrough(): void { const currentStatus = status.textContent?.trim() ?? ""; const textChanged = clipboardText.value !== initialText; const statusChanged = currentStatus !== initialStatus; - if (statusChanged && currentStatus === "Copy not confirmed") return; - if ((textChanged || statusChanged) && currentStatus === "Clipboard copied") return; - if (textChanged || (statusChanged && currentStatus === "Clipboard received")) { + if (statusChanged && currentStatus === CLIPBOARD_STATUS.notConfirmed) return; + if ((textChanged || statusChanged) && currentStatus === CLIPBOARD_STATUS.copied) return; + if (textChanged || (statusChanged && currentStatus === CLIPBOARD_STATUS.received)) { responseObserved = true; break; } @@ -43,44 +63,59 @@ function bindAndroidCopyWriteThrough(): void { if (request !== generation) return; if (!responseObserved) { - status.textContent = "Copy not confirmed"; - details.textContent = "Android did not return a new clipboard value. The previous PC clipboard was left unchanged."; + // Only claim "not confirmed" when nothing else has explained the silence. + // A disconnect or an over-limit clipboard sets its own terminal status, + // and overwriting it replaces an accurate diagnosis with a wrong one. + const currentStatus = status.textContent?.trim() ?? ""; + if (currentStatus === initialStatus || currentStatus === CLIPBOARD_STATUS.copying) { + writeStatus(CLIPBOARD_STATUS.notConfirmed, "Android did not return a new clipboard value. The previous PC clipboard was left unchanged."); + } return; } const text = clipboardText.value; if (!text) { - status.textContent = "Copy not confirmed"; - details.textContent = "Android did not return clipboard text."; + writeStatus(CLIPBOARD_STATUS.notConfirmed, "Android did not return clipboard text."); return; } try { await navigator.clipboard.writeText(text); if (request !== generation) return; - status.textContent = "Clipboard copied"; - details.textContent = "Android selection was copied to the PC clipboard."; + writeStatus(CLIPBOARD_STATUS.copied, "Android selection was copied to the PC clipboard."); return; } catch { + // Every other exit re-checks the generation; without it here a superseded + // copy still yanks focus and writes its result over the newer request. + if (request !== generation) return; + // execCommand copies the live selection, so it can only be trusted while + // the textarea still holds the text this request validated. + if (clipboardText.value !== text) return; const selectionStart = clipboardText.selectionStart; const selectionEnd = clipboardText.selectionEnd; + const previouslyFocused = document.activeElement; clipboardText.focus(); clipboardText.select(); const copied = document.execCommand("copy"); clipboardText.setSelectionRange(selectionStart, selectionEnd); - canvas.focus(); + // Restore where focus actually was rather than assuming the canvas: this + // path runs on every copy in browsers without the async Clipboard API, and + // parking focus on the canvas sends the next keystroke to Android. + if (previouslyFocused instanceof HTMLElement) previouslyFocused.focus(); + else canvas.focus(); + if (request !== generation) return; if (copied) { - status.textContent = "Clipboard copied"; - details.textContent = "Android selection was copied to the PC clipboard using the browser fallback."; + writeStatus(CLIPBOARD_STATUS.copied, "Android selection was copied to the PC clipboard using the browser fallback."); } else { - status.textContent = "Clipboard received"; - details.textContent = "Android clipboard reached the browser, but the browser blocked writing to the PC clipboard."; + writeStatus(CLIPBOARD_STATUS.received, "Android clipboard reached the browser, but the browser blocked writing to the PC clipboard."); } } }; button.addEventListener("click", () => void finishCopy()); canvas.addEventListener("keydown", (event) => { - if (event.altKey || (!event.ctrlKey && !event.metaKey) || event.key.toLowerCase() !== "c") return; + // Reuse the controller's predicate instead of restating it: a copy of this + // condition drifts out of step with the keydown handler it shadows. + if (event.repeat || clipboardShortcut(event) !== "copy") return; void finishCopy(); }); } diff --git a/apps/web-client/static/droidwebdisplay-main-drawer.css b/apps/web-client/static/droidwebdisplay-main-drawer.css index e77b8b7..426d9b4 100644 --- a/apps/web-client/static/droidwebdisplay-main-drawer.css +++ b/apps/web-client/static/droidwebdisplay-main-drawer.css @@ -1,4 +1,7 @@ -@import url("./droidwebdisplay-main-drawer-core.css"); +/* Versioned to match the ?v= on the entry sheet in index.html. Without it the + 531 lines of drawer layout below sit outside cache-busting, and a stale + copy after an upgrade renders every drawer group stacked and visible. */ +@import url("./droidwebdisplay-main-drawer-core.css?v=0.11.2-native5"); /* Header brand/status lockup v2. Keep the brand on one header row, remove the legacy rail-style pseudo logo, diff --git a/apps/web-client/static/droidwebdisplay-main-drawer.js b/apps/web-client/static/droidwebdisplay-main-drawer.js index cdcf879..8314d8c 100644 --- a/apps/web-client/static/droidwebdisplay-main-drawer.js +++ b/apps/web-client/static/droidwebdisplay-main-drawer.js @@ -46,16 +46,11 @@ const label = root().querySelector(`[data-group="${group}"] .gb-rail-label`)?.textContent || 'Tools'; const title = root().querySelector('.gb-drawer-title'); if (title) title.textContent = label; drawer()?.classList.add('gb-open'); drawer()?.setAttribute('aria-hidden', 'false'); - if (group === 'clipboard') { - // controller.ts historically focused the fallback textarea one frame after - // opening Clipboard. That makes the global Ctrl+V route treat the paste as - // page editing instead of Android input. Return focus to the mirrored screen - // after that callback has run; users can still click the textarea explicitly. - window.requestAnimationFrame(() => window.requestAnimationFrame(() => { - const screen = document.getElementById('screen'); - if (screen instanceof HTMLCanvasElement) screen.focus(); - })); - } + // No focus handling here on purpose. This used to force focus back to the + // mirrored screen because controller.ts focused the fallback textarea when + // the Clipboard group opened; that listener was deleted when Ctrl+V moved to + // a document-level paste handler, so the guard now only steals focus from a + // user typing into the textarea and injects their keystrokes into Android. } function closeDrawer() { if (pinned || !root()) return; diff --git a/apps/web-client/tests/header-brand-layout.test.mjs b/apps/web-client/tests/header-brand-layout.test.mjs index 565f852..3692f3a 100644 --- a/apps/web-client/tests/header-brand-layout.test.mjs +++ b/apps/web-client/tests/header-brand-layout.test.mjs @@ -12,8 +12,8 @@ test("header override removes legacy pseudo logo and keeps brand on one row", () assert.match(css, /content: none !important/); assert.match(css, /grid-template-columns: 54px minmax\(10\.6rem, auto\)/); assert.match(css, /grid-template-rows: 27px 27px/); - assert.match(css, /\.topbar \{[\s\S]*flex-wrap: nowrap !important/); - assert.match(css, /\.connection-row \{[\s\S]*flex-basis: auto !important/); + assert.match(css, /\.topbar \{[^}]*flex-wrap: nowrap !important/); + assert.match(css, /\.connection-row \{[^}]*flex-basis: auto !important/); }); test("header override exposes independent phone and display states while keeping wordmark white", () => { diff --git a/apps/web-client/tests/header-status-indicator.test.mjs b/apps/web-client/tests/header-status-indicator.test.mjs index aa7f563..fae857c 100644 --- a/apps/web-client/tests/header-status-indicator.test.mjs +++ b/apps/web-client/tests/header-status-indicator.test.mjs @@ -17,7 +17,7 @@ test("header status icon animates connection transitions and recent actions", () assert.match(drawerSource, new RegExp(`${action}:`)); } for (const control of ["back", "home", "recent", "power", "fullscreen", "running-app-icon", "running-app-select"]) { - assert.ok(drawerSource.includes(control)); + assert.match(drawerSource, new RegExp(`['"\`]${control}['"\`]`)); } assert.match(drawerSource, /status-action-glyph/); assert.match(drawerSource, /bindStatusActivityIndicator\(\);/); diff --git a/docs/contracts/CLIPBOARD.md b/docs/contracts/CLIPBOARD.md index a6f6adb..388ea9a 100644 --- a/docs/contracts/CLIPBOARD.md +++ b/docs/contracts/CLIPBOARD.md @@ -39,13 +39,39 @@ Control sessions intentionally keep scrcpy's native clipboard autosync enabled. ```text Copy button or Ctrl+C - -> explicit clipboard request / current Android clipboard value - -> browser-side write-through - -> Clipboard API or fallback copy path + -> controller.ts requests the Android clipboard (GetClipboard) + -> device reply lands in #clipboard-text and the status line + -> main.ts bindAndroidCopyWriteThrough observes that reply, still inside + the originating gesture, and performs the PC write + -> Clipboard API, or document.execCommand("copy") fallback ``` The manual path exists independently of automatic synchronization. Manual Copy must continue to work even when the Android clipboard text has not changed since the previous notification. +**Why the write lives in main.ts, not in the device-message loop.** +`navigator.clipboard.writeText` requires transient user activation. Writing +from `consumeDeviceMessages` would happen outside the click or keypress and be +refused. `bindAndroidCopyWriteThrough` therefore stays inside the gesture and +polls the DOM for the controller's reply for up to 1200 ms. + +**The coupling this creates.** That poll recognises the reply by matching the +status headline against exact strings. Those strings are therefore a contract +between the two modules, not display copy, and they live in +`apps/web-client/src/clipboard-status.ts` (`CLIPBOARD_STATUS`). Both +`controller.ts` and `main.ts` import them. + +Do not restate a sentinel as a literal at a `setStatus` call site. Doing so +breaks the Copy button silently: no exception, no console error, and the +existing tests stay green because they only pin the `main.ts` side. +`tests/packaging/test_clipboard_sync_regression.py::test_clipboard_status_sentinels_are_defined_once_and_shared` +guards this. + +One further ordering dependency worth knowing: `bindAndroidCopyWriteThrough` is +registered *after* the controller's own listeners, and the controller sets the +transient `Copying` status synchronously. That is what lets a repeated identical +Android clipboard value still register as a reply, since the textarea contents +do not change in that case. + ### PC -> Android ```text @@ -65,6 +91,13 @@ Do not solve a manual Copy problem by disabling scrcpy native clipboard autosync Do not solve browser permission problems by blocking keyboard input or repeatedly prompting/focusing a paste UI. +Exception, documented rather than silent: the `execCommand` fallback in +`main.ts` does focus `#clipboard-text`, select it, and restore focus, because +that is the only way to copy without the async Clipboard API. It runs only +when `writeText` has already been refused, and it restores focus to wherever +it was rather than parking it on the canvas. + + ## Regression history In v0.11.6, `clipboard_autosync=false` was added to control sessions to make direct `GetClipboard` behavior deterministic. That also disabled normal Android clipboard-change notifications and broke Android -> PC synchronization. The regression was fixed by restoring scrcpy native clipboard autosync while retaining the browser-side manual Copy/Ctrl+C fallback. diff --git a/droid_web_display/desktop/gui.py b/droid_web_display/desktop/gui.py index c23d216..df44e7f 100644 --- a/droid_web_display/desktop/gui.py +++ b/droid_web_display/desktop/gui.py @@ -99,7 +99,8 @@ def _make_header_icon_button( button.setObjectName("headerIconButton") button.setIcon(icon) button.setIconSize(QSize(20, 20)) - button.setFixedSize(38, 38) + # Size lives in theme.py (QPushButton#headerIconButton). Setting it here too + # gave the stylesheet a 36px box inside a 38px widget. button.setToolTip(tooltip) button.setAccessibleName(accessible_name) if danger: @@ -329,7 +330,9 @@ def _build_overview_tab(self) -> QWidget: overview_row.setSpacing(12) summary, summary_layout = _make_card("Summary") - summary_layout.setAlignment(Qt.AlignTop) + # Top alignment comes from the addStretch(1) below. The single-argument + # setAlignment positions a layout inside a PARENT layout item, and this + # one is installed directly on the card, so it was never read. summary_layout.setSpacing(8) self._url_value = _field_value() @@ -816,9 +819,18 @@ def _apply_snapshot(self, snapshot: ServerSnapshot) -> None: } self._set_status_visual(snapshot.state, labels[snapshot.state]) safe_url = escape(snapshot.url, quote=True) - self._url_value.setText( - f'{safe_url}' - ) + url_is_live = snapshot.state in {ServerState.RUNNING, ServerState.EXTERNAL} + # Only offer the link when the header Open button is offered too. A live + # hyperlink beside a greyed-out Open button just routes the user to + # ERR_CONNECTION_REFUSED. The colour comes from the palette so it tracks + # WEB_THEME["focus"]; an inline literal here silently overrides the + # QLabel#summaryUrl rule, because inline HTML style beats QSS. + if url_is_live: + self._url_value.setText( + f'{safe_url}' + ) + else: + self._url_value.setText(safe_url) self._network_value.setText(snapshot.network_mode) self._diag_url.setText(snapshot.url) if snapshot.device: @@ -900,6 +912,10 @@ def _status_failed(self, message: str) -> None: self._set_status_visual(ServerState.ERROR, "Status unavailable") self._error_value.setText(message) self._set_health(self._health_server, "error", "● Status unavailable") + # The probe failed, so the server state is unknown. _apply_snapshot is + # the only other place that gates this button, and it does not run on + # this path -- leaving Open enabled and blue indefinitely. + self._header_open_button.setEnabled(False) self._status_finished() def _status_finished(self) -> None: diff --git a/droid_web_display/desktop/theme.py b/droid_web_display/desktop/theme.py index b82f042..a3f3132 100644 --- a/droid_web_display/desktop/theme.py +++ b/droid_web_display/desktop/theme.py @@ -243,6 +243,18 @@ def desktop_stylesheet() -> str: border-color: {c['danger_border']}; }} +/* An id selector outranks the bare QPushButton:disabled and :pressed rules + below, so without these the header buttons keep their enabled background + when disabled and give no press feedback. */ +QPushButton#headerIconButton:disabled {{ + background-color: #171b23; + border-color: #242a37; +}} + +QPushButton#headerIconButton:pressed {{ + background-color: #1d2430; +}} + QPushButton:pressed {{ background-color: #1d2430; }} diff --git a/tests/desktop/test_layout_regression.py b/tests/desktop/test_layout_regression.py index 3011ff0..a391c9d 100644 --- a/tests/desktop/test_layout_regression.py +++ b/tests/desktop/test_layout_regression.py @@ -45,19 +45,24 @@ def test_primary_open_and_exit_actions_live_in_header_as_icons() -> None: assert "header_layout.addWidget(self._status_value" in gui assert "header_layout.addWidget(self._header_open_button" in gui assert "header_layout.addWidget(self._header_exit_button" in gui - assert 'footer_open = QPushButton("Open DroidWebDisplay")' not in gui - assert 'footer_exit = QPushButton("Exit")' not in gui + # Assert on the user-visible strings, not on a local variable name: the old + # form passed if the footer button came back as `open_btn = QPushButton(...)`. + assert gui.count('QPushButton("Open DroidWebDisplay")') == 0 + assert gui.count('QPushButton("Exit")') == 0 def test_summary_is_compact_and_local_url_is_clickable() -> None: gui = (ROOT / "droid_web_display/desktop/gui.py").read_text(encoding="utf-8") - assert "summary_layout.setAlignment(Qt.AlignTop)" in gui assert "summary_layout.addStretch(1)" in gui assert '("Local URL", self._url_value)' in gui assert 'self._url_value.setObjectName("summaryUrl")' in gui assert "Qt.TextBrowserInteraction" in gui assert "self._url_value.linkActivated.connect(self._open_summary_url)" in gui assert "QDesktopServices.openUrl(QUrl(href))" in gui + # The link must only be offered while the server can serve it, matching the + # gating on the header Open button. + assert "url_is_live = snapshot.state in {ServerState.RUNNING, ServerState.EXTERNAL}" in gui + assert 'style="color:#82a6ff' not in gui def test_settings_include_only_minimal_update_checker() -> None: diff --git a/tests/packaging/test_clipboard_sync_regression.py b/tests/packaging/test_clipboard_sync_regression.py index a524e9c..06b244b 100644 --- a/tests/packaging/test_clipboard_sync_regression.py +++ b/tests/packaging/test_clipboard_sync_regression.py @@ -42,7 +42,7 @@ def test_type_bypasses_clipboard_and_copy_requests_android_selection() -> None: assert "androidClipboardCopyMessage()" in block assert "beginAndroidCopyRequest" in block assert "completeAndroidCopyRequest" in block - assert "Copy not confirmed" in block + assert "CLIPBOARD_STATUS.notConfirmed" in block assert "previous PC clipboard was left unchanged" in block assert "writeText(this.#lastAndroidClipboard)" not in block @@ -55,11 +55,14 @@ def test_android_copy_write_through_never_copies_stale_android_text() -> None: assert '"#screen"' in block assert "navigator.clipboard.writeText(text)" in block assert 'document.execCommand("copy")' in block - assert 'currentStatus === "Clipboard received"' in block - assert 'currentStatus === "Copy not confirmed"' in block + # These sentinels are the contract between controller.ts and main.ts. + # They live in clipboard-status.ts precisely so that rewording one at a + # call site cannot silently break the Copy button; assert on the shared + # symbol, and pin the literal values in the dedicated test below. + assert "currentStatus === CLIPBOARD_STATUS.received" in block + assert "currentStatus === CLIPBOARD_STATUS.notConfirmed" in block assert "if (!responseObserved)" in block assert "previous PC clipboard was left unchanged" in block - assert "No new Android clipboard event arrived; copied the last Android clipboard value." not in block assert "bindAndroidCopyWriteThrough();" in block @@ -70,7 +73,7 @@ def test_manual_copy_timeout_clears_pending_and_late_clipboard_events_are_not_cl for block in (src_helpers, built_helpers): assert "#copyShortcutTimer" in block assert "#copyShortcutPending = false" in block - assert "Copy not confirmed" in block + assert "CLIPBOARD_STATUS.notConfirmed" in block assert "completeAndroidCopyRequest" in block for block in (source, dist): @@ -152,3 +155,27 @@ def test_oversized_text_is_synchronized_rather_than_typed_chunk_by_chunk() -> No for block in (src_type, built_type): assert "MAX_INJECTED_BYTES" in block assert "too large to type into Android" in block + + +def test_clipboard_status_sentinels_are_defined_once_and_shared() -> None: + """The copy write-through matches these strings against the status line. + + Rewording one at a call site used to break the Copy button with a green + suite, so the literals now live in one module and both sides import them. + """ + root = Path(__file__).resolve().parents[2] + contract = (root / "apps/web-client/src/clipboard-status.ts").read_text(encoding="utf-8") + built = (root / "apps/web-client/dist/assets/clipboard-status.js").read_text(encoding="utf-8") + for block in (contract, built): + assert 'copying: "Copying"' in block + assert 'copied: "Clipboard copied"' in block + assert 'received: "Clipboard received"' in block + assert 'notConfirmed: "Copy not confirmed"' in block + + controller = (root / "apps/web-client/src/controller.ts").read_text(encoding="utf-8") + main = (root / "apps/web-client/src/main.ts").read_text(encoding="utf-8") + for block in (controller, main): + assert 'from "./clipboard-status.js"' in block + # No call site may restate a sentinel as a literal. + for literal in ('setStatus("Clipboard received"', 'setStatus("Copy not confirmed"', 'setStatus("Copying"'): + assert literal not in controller From 79124267e847991636741d39846e8587c54decc2 Mon Sep 17 00:00:00 2001 From: Aleksandr Chasnyk <69671996+ami3go@users.noreply.github.com> Date: Thu, 20 Aug 2026 13:24:30 +0300 Subject: [PATCH 2/2] Scope the status-indicator assertions to the maps under test My own tightened assertion was wrong: the control names are unquoted object keys (`back: 'navigation'`), so requiring surrounding quotes failed for every key except 'running-app-icon'. Both loops now extract the object literal they are about -- `actionPaths` and `controlActions` -- and match a key within it. That fixes the failure and also closes the original hole properly: searching the whole 600-line file made `includes("back")` unconditionally true via callback/fallback/background, and a bare `${action}:` matched any object key anywhere. Verified in both directions: every real key matches, and `{ fallback: 1, callback: 2, background: 3 }` does not satisfy "back". running-app-select is asserted separately since it is bound directly rather than through the controlActions map. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XfT5Z3Z24bjC8qtqERGpMQ --- .../tests/header-status-indicator.test.mjs | 24 ++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/apps/web-client/tests/header-status-indicator.test.mjs b/apps/web-client/tests/header-status-indicator.test.mjs index fae857c..f6d72bc 100644 --- a/apps/web-client/tests/header-status-indicator.test.mjs +++ b/apps/web-client/tests/header-status-indicator.test.mjs @@ -8,17 +8,35 @@ const root = resolve(dirname(fileURLToPath(import.meta.url)), ".."); const drawerSource = await readFile(resolve(root, "static/droidwebdisplay-main-drawer.js"), "utf8"); const drawerCss = await readFile(resolve(root, "static/droidwebdisplay-main-drawer.css"), "utf8"); + +/** The object literal that follows `marker`, so an assertion about a map's keys + cannot be satisfied by an unrelated identifier elsewhere in the file. */ +function blockAfter(source, marker) { + const start = source.indexOf(marker); + assert.ok(start >= 0, `missing ${marker}`); + const end = source.indexOf("};", start); + assert.ok(end > start, `unterminated ${marker}`); + return source.slice(start, end); +} + test("header status icon animates connection transitions and recent actions", () => { assert.match(drawerSource, /function bindStatusActivityIndicator\(\)/); assert.match(drawerSource, /new MutationObserver/); assert.match(drawerSource, /attributeFilter: \['data-state'\]/); assert.match(drawerSource, /prefers-reduced-motion: reduce/); + // Scope to the maps under test. Searching the whole 600-line file made these + // vacuous: `includes("back")` is satisfied by callback/fallback/background, + // and a bare `${action}:` matches any object key anywhere in the file. + const actionPaths = blockAfter(drawerSource, "const actionPaths = {"); for (const action of ["navigation", "clipboard", "rotate", "resize", "power", "fullscreen", "apps", "warning"]) { - assert.match(drawerSource, new RegExp(`${action}:`)); + assert.match(actionPaths, new RegExp(`(^|[\\s{,])['"\`]?${action}['"\`]?\\s*:`, "m")); } - for (const control of ["back", "home", "recent", "power", "fullscreen", "running-app-icon", "running-app-select"]) { - assert.match(drawerSource, new RegExp(`['"\`]${control}['"\`]`)); + const controlActions = blockAfter(drawerSource, "const controlActions = {"); + for (const control of ["back", "home", "recent", "power", "fullscreen", "running-app-icon"]) { + assert.match(controlActions, new RegExp(`(^|[\\s{,])['"\`]?${control}['"\`]?\\s*:`, "m")); } + // running-app-select is bound separately, not through the controlActions map. + assert.match(drawerSource, /getElementById\('running-app-select'\)\?\.addEventListener\('change'/); assert.match(drawerSource, /status-action-glyph/); assert.match(drawerSource, /bindStatusActivityIndicator\(\);/); });