Canvas read-only viewer: data-loss illusion (local edits vanish on refresh) #26

New issue

Closed

opened 2026-04-27 16:34:04 +00:00 by sameh-farouk · 0 comments

sameh-farouk commented

2026-04-27 16:34:04 +00:00

Member

Summary

Read-only canvas viewers (users with role viewer on a canvas, or workspace members viewing a canvas they're not a collaborator on) experienced a data-loss illusion: clicking format buttons (color, bold, etc.) on the bubble menu visibly changed the canvas locally, then those changes vanished on refresh. To the user this looks like "the system saved my edit and then lost it" — a worse failure mode than disabled buttons.

Server-side gating was always correct:

routes.rs::handle_canvas_ws drops viewer Yjs Update / SyncStep2 frames at line 1072 before broadcast/persist.
canvas.save_state RPC rejects via check_canvas_access(write_required: true) at canvas.rs:740.

The bug was purely client-side: Tiptap's setEditable(false) only blocks contenteditable typing; it does not block programmatic dispatch (editor.chain().toggleBold().run()) from toolbar buttons or keyboard shortcuts. The local Yjs Doc accepted these mutations, the y-websocket provider sent them upstream, the server dropped them silently, and refresh reverted to the (unchanged) authoritative state.

Reproduction (verified empirically)

As viewer_test (read-only on canvas 1):
Open the canvas; click any bubble-menu button (Bold, Highlight, color, etc.) over selected text.
Format change is visible immediately.
Hard-refresh the page.
Format change is gone — DB canvas_state.updated_at and canvases.updated_at never advanced.

Discussed in

Surfaced and analyzed in #10 — see comment #24493 for the empirical investigation that established this issue's framing.

Fix scope

Implemented in PR (linked below):

filterTransaction ProseMirror plugin registered on viewers' editors rejects any doc-changing transaction that isn't tagged with ySyncPluginKey (the meta @tiptap/y-tiptap sets on remote Yjs sync transactions). This is the structural choke point — catches programmatic dispatch, keyboard shortcuts, and any future code path.
editable: false set at editor construction (vs late toggle) for typing-input gating.
Bubble menu and table toolbar setup IIFEs skipped entirely for viewers — no positioning math, no DOM mutations, no event handlers attached.
CSS rules under body[data-canvas-mode="viewer"] hide the static top toolbar, share button, and the various popovers/pickers.
Visible "View only" badge in the topbar replaces the previous subtle status-text treatment.
Vendor bundle re-exports ySyncPluginKey (from @tiptap/y-tiptap) and Plugin (from @tiptap/pm/state) so the filterTransaction wiring is reachable from canvas-app.js.

Out of scope (deliberately)

Console-level evasion (window.editor.chain().toggleBold().run() from DevTools): can still mutate the local Doc, but the filterTransaction plugin still blocks it. Even if it didn't, the server is authoritative — the WS handler and canvas.save_state reject anyway. Honest UI for legitimate users; not a security boundary.
The loadChannels auto-create-#general bug surfaced in the same investigation thread is a separate concern (chat sidebar, not canvas) and should get its own issue.

### Summary Read-only canvas viewers (users with role `viewer` on a canvas, or workspace members viewing a canvas they're not a collaborator on) experienced a **data-loss illusion**: clicking format buttons (color, bold, etc.) on the bubble menu visibly changed the canvas locally, then those changes vanished on refresh. To the user this looks like "the system saved my edit and then lost it" — a worse failure mode than disabled buttons. Server-side gating was always correct: - `routes.rs::handle_canvas_ws` drops viewer Yjs `Update` / `SyncStep2` frames at line 1072 before broadcast/persist. - `canvas.save_state` RPC rejects via `check_canvas_access(write_required: true)` at canvas.rs:740. The bug was purely client-side: Tiptap's `setEditable(false)` only blocks contenteditable typing; it does **not** block programmatic dispatch (`editor.chain().toggleBold().run()`) from toolbar buttons or keyboard shortcuts. The local Yjs Doc accepted these mutations, the y-websocket provider sent them upstream, the server dropped them silently, and refresh reverted to the (unchanged) authoritative state. ### Reproduction (verified empirically) 1. As `viewer_test` (read-only on canvas 1): 2. Open the canvas; click any bubble-menu button (Bold, Highlight, color, etc.) over selected text. 3. Format change is visible immediately. 4. Hard-refresh the page. 5. Format change is gone — DB `canvas_state.updated_at` and `canvases.updated_at` never advanced. ### Discussed in Surfaced and analyzed in #10 — see [comment #24493](https://forge.ourworld.tf/lhumina_code/hero_collab/issues/10#issuecomment-24493) for the empirical investigation that established this issue's framing. ### Fix scope Implemented in PR (linked below): - `filterTransaction` ProseMirror plugin registered on viewers' editors rejects any doc-changing transaction that isn't tagged with `ySyncPluginKey` (the meta @tiptap/y-tiptap sets on remote Yjs sync transactions). This is the structural choke point — catches programmatic dispatch, keyboard shortcuts, and any future code path. - `editable: false` set at editor construction (vs late toggle) for typing-input gating. - Bubble menu and table toolbar setup IIFEs skipped entirely for viewers — no positioning math, no DOM mutations, no event handlers attached. - CSS rules under `body[data-canvas-mode="viewer"]` hide the static top toolbar, share button, and the various popovers/pickers. - Visible "View only" badge in the topbar replaces the previous subtle status-text treatment. - Vendor bundle re-exports `ySyncPluginKey` (from `@tiptap/y-tiptap`) and `Plugin` (from `@tiptap/pm/state`) so the filterTransaction wiring is reachable from canvas-app.js. ### Out of scope (deliberately) - Console-level evasion (`window.editor.chain().toggleBold().run()` from DevTools): can still mutate the local Doc, but the filterTransaction plugin still blocks it. Even if it didn't, the server is authoritative — the WS handler and `canvas.save_state` reject anyway. Honest UI for legitimate users; not a security boundary. - The `loadChannels` auto-create-`#general` bug surfaced in the same investigation thread is a separate concern (chat sidebar, not canvas) and should get its own issue.