Skip to content

fix(ui): restore Firefox text selection for room public key and contract ID - #536

Merged
sanity merged 5 commits into
mainfrom
worktree-fix-firefox-room-info-selection
Jul 29, 2026
Merged

fix(ui): restore Firefox text selection for room public key and contract ID#536
sanity merged 5 commits into
mainfrom
worktree-fix-firefox-room-info-selection

Conversation

@sanity

@sanity sanity commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Problem

In the room-details panel (the (i) icon beside the room title), the Room
Public Key
and Contract ID could not be selected or copied in Firefox.
Double-click selected nothing, click-drag selected nothing. The same fields
worked in Chrome, Brave and Safari.

Root cause: all three read-only fields in that panel carried Tailwind's
select-all, which compiles to -webkit-user-select: all; user-select: all.
Firefox parses the rule — the computed value really is all — but selecting
inside an <input> under it yields a zero-length selection, so there is
nothing for Ctrl+C to copy. Chromium and WebKit select the whole value under
the same rule, which is why the bug presented as Firefox-specific.

Measured with a standalone repro (no River code), counting characters selected
by a click-drag across the field and then by a double-click:

input rule Firefox Chromium WebKit
user-select: all 0 44 44
user-select: text 44 44 44

Approach

  • Declare user-select: text explicitly (select-text) on the three
    read-only fields — Room Public Key, Contract ID, and Secret Version — rather
    than relying on the auto default. The Secret Version field had the
    identical defect and is fixed in the same pass.
  • Add a Copy button beside each of the two long values, reusing the
    existing crate::util::copy_to_clipboard helper (which uses
    execCommand('copy') so it works in the gateway's sandboxed iframe).
    Manual text selection works independently of the button.
  • The inputs move into a flex row; flex-1 min-w-0 keeps a long base58
    value from widening the modal.

One deliberate behaviour change in Chromium/WebKit

Worth calling out against the issue's "Chrome/Brave/Safari behavior remains
unchanged" criterion. Under user-select: all, a single click in these
fields selected the entire value in Chromium/WebKit. Under user-select: text
a single click places a caret instead; drag, double-click, Ctrl+A and Ctrl+C
all behave normally. That is inherent to the fix the issue asks for in
requirement 4, and the new copy button more than restores the one-action copy —
this time in Firefox too, where it never worked.

There is no way to keep click-selects-all and fix Firefox: re-selecting the
whole value from a click/focus handler would clobber a drag selection, which is
the exact behaviour being restored.

Why not just drop the class and let auto apply? The issue asks for the value
to be explicitly selectable, and an explicit declaration is what stops the
next person reintroducing select-all. A rustdoc block on CopyButton records
the measurement table and says not to restore it.

Testing

New ui/tests/room-info-key-selection.spec.ts, which runs on all five
Playwright projects (CI installs chromium + firefox + webkit).

The Firefox gate is verified, not assumed. With the fix reverted and the
UI rebuilt, the 4 mouse-selection tests fail on Firefox; with the fix they
pass. The loaded build was confirmed by reading the input's computed
user-select out of the live DOM before each run, per the AGENTS.md warning
about stale builds.

Coverage per field: click-drag selects, double-click selects, Ctrl+C copies
a mouse selection, Ctrl+A+Ctrl+C copies the whole value, selecting
does not close the modal, the copy button reports "Copied!", and the panel
does not overflow horizontally.

The Ctrl+C assertions are deliberately mouse-driven. An earlier Ctrl+A
version passed even on the broken build (keyboard select-all is not blocked by
user-select: all), i.e. it was non-discriminating.

The two keyboard-copy tests are skipped on WebKit. Isolated to a bare page
with two inputs and no River code, Playwright's WebKit will not deliver a
keyboard copy to a readonly input at all:

engine readonly Ctrl+A readonly mouse-select readonly Ctrl+C editable
firefox selects selects copy fires all work
chromium selects selects copy fires all work
webkit selects nothing selects no copy event all work

These fields were already readonly before this PR, so that is a harness
property, not a regression. Mouse selection — the behaviour this PR is about —
is still asserted on webkit and mobile-safari.

Results: new spec 57 passed / 8 skipped across all projects. Full existing
suite 661 passed / 23 skipped / 1 flaky, plus cargo test -p river-ui --bins
780 passed and cargo fmt --check clean. The one flaky test
(invitation-sandbox.spec.ts on mobile-safari) is unrelated to this change and
passes 6/6 standalone; filed as #538.

Also in this PR

Two small fixes found while getting a clean checkout to build. Both are
independent of the bug fix and easy to drop if you'd rather they went
separately.

  • Makefile.tomlui/assets/styles.css is gitignored (a build
    product), but only build-ui and dev-example depended on
    build-tailwind. build-ui-example, build-ui-no-sync,
    build-ui-example-no-sync and dev did not, so on a clean checkout they
    fail with Asset at /assets/styles.css doesn't exist.
    build-ui-example-no-sync is the task AGENTS.md tells you to run before the
    Playwright suite. CI is unaffected either way because build.yml runs
    npm run build:css as its own explicit step, which is why this went
    unnoticed. Verified by deleting styles.css and rebuilding.
  • ui/Cargo.toml — declared dioxus = "0.7.3" while Cargo.lock has
    resolved 0.7.9 for some time. AGENTS.md documents that stale declaration as a
    debugging trap, and build.yml keys its dx cache on
    hashFiles('ui/Cargo.toml') expecting a dioxus bump to invalidate the cached
    CLI — which only works if the declared version tracks the real one. 0.7.9 is
    the latest stable (0.8.0-alpha.0 is a pre-release and is deliberately not
    adopted). Cargo.lock is byte-identical before and after, so resolution and
    every WASM artifact are unchanged.

No delegate/contract WASM was rebuilt or committed.

Closes #537

[AI-assisted - Claude]

sanity and others added 4 commits July 29, 2026 09:38
The Room Public Key, Contract ID and Secret Version fields in the
room-details panel carried Tailwind's `select-all` (`user-select: all`).
Firefox parses that rule — the computed value really is `all` — but
selecting inside an <input> under it yields a ZERO-length selection, so
click-drag and double-click both did nothing and Ctrl+C copied nothing.
Chromium and WebKit select the whole value under the same rule, which is
why the bug presented as Firefox-specific.

Declare `user-select: text` explicitly instead of relying on the `auto`
default, and add a copy button beside the two long values.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QaMA3cqXTBiTNmPYcMeZsi
`ui/assets/styles.css` is gitignored (it is a build product), but only
`build-ui` and `dev-example` depended on `build-tailwind`. The other four
dx-driven tasks — `build-ui-example`, `build-ui-no-sync`,
`build-ui-example-no-sync` and `dev` — did not, so on a clean checkout they
fail with:

    error: Asset at /assets/styles.css doesn't exist

`build-ui-example-no-sync` is the task AGENTS.md tells you to run before the
Playwright suite, so this bites anyone following the documented workflow. CI
is unaffected either way because build.yml runs `npm run build:css` as its
own explicit step, which is why this went unnoticed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QaMA3cqXTBiTNmPYcMeZsi
The manifest declared `dioxus = "0.7.3"` while Cargo.lock has resolved
0.7.9 for some time. Two concrete consequences:

- AGENTS.md documents ui/Cargo.toml as a debugging trap: someone checking a
  stale `dx` CLI against the declared version would wrongly conclude a 0.7.3
  CLI was correct.
- build.yml keys its `dx` cache on `hashFiles('ui/Cargo.toml')`, with a
  comment saying a dioxus bump should invalidate the cached CLI. That only
  works if the declared version tracks the real one.

0.7.9 is the latest stable dioxus (0.8.0-alpha.0 is a pre-release and is
deliberately not adopted here). Cargo.lock is byte-identical before and
after, so resolution and every WASM artifact are unchanged.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QaMA3cqXTBiTNmPYcMeZsi
… 320px

Review pass on the new spec found three gaps:

- The copy-button tests only asserted the label flips to "Copied!", which is
  a side effect that is identical whether or not the two buttons are wired to
  the right values. Capture what the app actually writes to the clipboard (by
  patching document.execCommand, since copy_to_clipboard deliberately uses
  execCommand so it works in the sandboxed iframe) and assert each button
  copies ITS field. Mutation-tested: swapping the two values in the component
  makes exactly these two tests fail.
- No coverage that the "Copied!" feedback resets when the panel is closed and
  reopened, even though copy-clipboard-feedback.spec.ts holds the sibling
  Export Identity button to that same contract.
- The overflow check pinned the viewport to 1280px, so the narrow-width risk
  that adding a button beside each value actually introduces was untested.
  Add a 320px check (the smallest width responsive-layout.spec.ts covers)
  asserting both buttons stay visible and neither the panel nor the document
  overflows.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QaMA3cqXTBiTNmPYcMeZsi
@sanity

sanity commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

Review (Full tier)

Tier: Full, because the diff touches Makefile.toml, which
~/.claude/rules/multi-model-review.md classifies as build/release
configuration ("always Full regardless of diff size"). The app change itself
would otherwise have been Light.

Lenses run sequentially in-session (no pr-review skill available in this
session, and subagents were not requested): code-first, testing,
skeptical/adversarial, big-picture, and the change-specific lens for
this diff, cross-browser CSS behaviour. External non-Claude reviewers were
not invoked — they are opt-in per the rule, not a required pass.

Findings, all fixed in f58f0f9

1. Copy-button tests were non-discriminating (testing lens). They asserted
only that the label flips to "Copied!" — a side effect that is identical
whether or not the two buttons are wired to the right values. With two
hand-wired buttons carrying two different values, a prop swap was exactly the
plausible defect and it would have passed.
Fixed by capturing what the app actually writes to the clipboard and asserting
each button copies ITS own field. Mutation-tested: swapping the two values
in the component makes precisely these two tests fail.

2. No reset-on-reopen coverage (big-picture lens).
copy-clipboard-feedback.spec.ts holds the sibling Export Identity copy button
to a "reopening resets the label" contract; the new buttons had no equivalent.
Behaviour was correct (the child unmounts with the panel) but unpinned. Test
added.

3. Narrow-viewport overflow untested (skeptical lens). The overflow check
pinned the viewport to 1280px, so the risk that adding a button beside each
value
actually introduces — a cramped small screen — was never exercised.
Added a 320px check asserting both buttons stay visible and neither the panel
nor the document overflows.

Considered and deliberately not changed

  • copied.set(true) in an onclick without crate::util::defer().
    .claude/rules/dioxus-signal-safety.md requires defer() for signal
    mutations in event handlers, but its rationale is GlobalSignal subscriber
    notification re-entrancy. copied is a component-local use_signal with no
    external subscribers, and both existing copy buttons in this codebase
    (not_member_notification.rs, invite_member_modal.rs) set local state
    directly in onclick. Following that precedent.
  • Single click no longer selects the whole value in Chromium/WebKit. This
    is inherent to user-select: text and cannot be kept alongside working
    drag-selection. Documented in the PR description rather than worked around.
  • Double base58 encode per render (once for the input, once for the button
    prop). 32 bytes, negligible, and hoisting it would mean restructuring the
    if let block.

Verification notes

Every browser claim in this PR is measured, not inferred. The Firefox gate was
mutation-tested (revert the fix → the 4 mouse-selection tests fail), and the
loaded build was confirmed each time by reading the input's computed
user-select out of the live DOM, per the AGENTS.md warning that a stale build
can silently invert a mutation check. The WebKit keyboard-copy skip is backed by
an isolated repro containing no River code.

[AI-assisted - Claude]

The reset test flaked once on mobile-safari in a full-suite run (it passed on
retry). Investigated rather than retried:

- Standalone it passes 10/10 on mobile-safari.
- An instrumented 25-cycle open/copy/close/reopen loop showed a stale label
  0 times in BOTH WebKit and Firefox, so the reset behaviour itself is sound.

The sensitivity is actionability, not correctness: the panel is a `fixed
inset-0` overlay, so the room-header (i) button only becomes clickable again
once it has fully unmounted, and under the full suite's parallel load (five
projects each booting a WASM app) that settle can outrun a 5s budget. Wait for
the reopen affordance explicitly and give the reopen assertions the same 15s
budget other app-level waits in this suite use.

Still non-vacuous: making the copy state persist across remount (the exact
regression this guards) fails it with `Received string: "Copied!"`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QaMA3cqXTBiTNmPYcMeZsi
@sanity

sanity commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

Verification update

A flake in my own new test, found and fixed. The reset-on-reopen test
flaked once on mobile-safari in a full-suite run (passed on retry). Rather than
retry it:

  • Standalone: 10/10 on mobile-safari.
  • An instrumented 25-cycle open/copy/close/reopen loop showed a stale label
    0 times in both WebKit and Firefox — so the reset behaviour is sound.

The sensitivity is actionability, not correctness: the panel is a fixed inset-0 overlay, so the room-header (i) button only becomes clickable again
once it has fully unmounted, and under the suite's parallel load (five projects
each booting a WASM app) that settle can outrun a 5s budget. Fixed by waiting on
the reopen affordance explicitly and using the 15s budget other app-level waits
in this suite already use. Re-confirmed non-vacuous: making the copy state
persist across remount fails it with Received string: "Copied!".

Local full suite, final run: 682 passed, 23 skipped, 0 flaky.

Filed while working on this

[AI-assisted - Claude]

@sanity
sanity merged commit e2ec477 into main Jul 29, 2026
6 checks passed
@sanity
sanity deleted the worktree-fix-firefox-room-info-selection branch July 29, 2026 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Room public key and contract ID cannot be selected or copied in Firefox

1 participant