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..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.ok(drawerSource.includes(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\(\);/); }); 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