mirror of
https://github.com/penpot/penpot.git
synced 2026-05-11 19:13:49 +00:00
b54fa2f11c
* 🐛 Reject clipboard helpers gracefully on insecure origins Closes #6514. Resolves the user-visible crash originally reported in #4478. `app.util.clipboard/to-clipboard` and `to-clipboard-promise` called `(unchecked-get js/navigator "clipboard")` and then immediately invoked `.writeText` / `.write` on the result, with no guard for the case where `navigator.clipboard` is `undefined`. The W3C Clipboard API spec requires a "secure context" (HTTPS or localhost), so a Penpot instance served over plain HTTP - which the SSDP/LAN self-hosted setup in #4478 was - throws TypeError: Cannot read properties of undefined (reading 'writeText') synchronously the moment the user clicks any copy button. The error escapes the consuming function before any error-handling rx/of arm runs, so the whole UI ends up on the error screen instead of just the affected control showing a "could not copy" message. A third helper (`to-clipboard-multi`) already guards `clipboard` and `clipboard.write`, but if both are missing it silently returns nil which is also surprising for callers expecting a Promise. ## Fix Add a small `get-clipboard` accessor and an `unavailable-error` factory that returns `Promise.reject(Error(...))` with a clear message ("Clipboard API is unavailable. This usually happens when the page is served over plain HTTP; serve Penpot over HTTPS to enable copy-to-clipboard."). Wire all three helpers through the same defensive contract: - `to-clipboard` - return the rejected Promise when `navigator.clipboard.writeText` is missing. - `to-clipboard-promise` - return the rejected Promise when `navigator.clipboard.write` is missing. - `to-clipboard-multi` - convert the existing `if` into a `cond` with three branches: prefer `clipboard.write` for true multi-MIME output, fall through to `writeText` with the text/plain payload when only the legacy text path is available, and finally reject with the unavailable error when neither path exists. Previously the no-API case fell off the `when-let` and silently returned nil. The contract is now consistent: every helper either resolves or rejects a Promise, never throws synchronously, and never returns nil. Callers (which are already structured around rx streams that call `rx/from` on the helper's return value) can chain `.catch` / `rx/catch` to surface a status-bar message instead of crashing. The two stale `;; FIXME` comments on `to-clipboard` (rename to `write-text`) and `to-clipboard-promise` (this API is very confuse) are removed - the rename remains an open follow-up across 13+ call sites and is intentionally out of scope, but the API is no longer "confuse" once the contract is documented and uniform. CHANGES.md entry added under the 2.17.0 Bugs-fixed section describing the user-visible behaviour change. * 🐛 Reject paste-from-navigator gracefully on insecure origins Symmetric companion to the to-clipboard / to-clipboard-promise / to-clipboard-multi guards added earlier in this PR. The paste path went through fromNavigator (frontend/src/app/util/clipboard.js) which called `navigator.clipboard.read()` with no nil-check; on insecure origins (plain HTTP / non-localhost) this raised an opaque `TypeError: Cannot read properties of undefined (reading 'read')` and the workspace surfaced a generic 'Something wrong has happened' toast instead of the descriptive 'serve Penpot over HTTPS …' message users get for the copy direction. Mirror the get-clipboard pattern from clipboard.cljs: - Read `navigator.clipboard` once into a local. - If it's missing the `.read` method, throw a descriptive Error that matches the wording the copy direction already uses (only the verb swaps: 'paste-from-clipboard' instead of 'copy-to-clipboard'). - Otherwise, dispatch through the local handle. The existing app.util.clipboard/from-navigator (clipboard.cljs:32) already wraps impl/fromNavigator in rx/from, so a rejected Promise from the async function propagates as an rx error event. Existing callers that subscribe with .catch / on-error see the structured Error and surface the toast, identical to how to-clipboard's unavailable-error already flows. Repro (matches niwinz's reproduction in the PR comment): Object.defineProperty(navigator, 'clipboard', { value: undefined }); // … then attempt a paste action in the workspace … Before: TypeError in console + 'Something wrong has happened' toast. After: descriptive Error caught by the rx subscription and rendered through the existing unavailable-Clipboard-API surface. Refs #6514, #4478 * 🐛 Show user-facing toast when clipboard API is unavailable Niwinz's review on penpot#9188 caught that the rejected Promise from to-clipboard / to-clipboard-promise / to-clipboard-multi / fromNavigator now surfaces the correct error to the console, but the workspace UI still falls through to the generic "Something wrong has happened" toast because the on-clipboard-permission-error and the paste error-handler in paste-from-clipboard only branched on clipboard-permission-error?. Apply the patch he suggested in the review: - Add clipboard-unavailable-error? predicate that matches the Promise.reject(Error("Clipboard API is unavailable. ...")) thrown by the get-clipboard / unavailable-error helpers added earlier in this PR. Uses str/starts-with? on the message prefix so the predicate stays stable even if the trailing "serve Penpot over HTTPS ..." advice text is reworded later. - Convert on-clipboard-permission-error from `if` to `cond` and add a third arm that fires errors.clipboard-api-unavailable as a warning toast. - Add the same arm in the second cond block inside paste-from-clipboard, before the :not-implemented and :else arms. - Add the matching errors.clipboard-api-unavailable entry to frontend/translations/en.po with the wording niwinz suggested: "Clipboard API is unavailable. Serve Penpot over HTTPS to enable clipboard access". Refs penpot#9188 review. --------- Signed-off-by: Andrey Antukh <niwi@niwi.nz> Co-authored-by: Andrey Antukh <niwi@niwi.nz>