From 5a0cee44b1b4860ecca4671732876782308f0fd9 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Mon, 27 Jul 2026 10:56:27 +0200 Subject: [PATCH 1/4] :bug: Harden frontend .getData call sites against undefined receivers (#10718) * :bug: Fix nil getData crash dropping ZIP without manifest.json Add nil-guard in read-as-text to raise typed :invalid-entry error instead of calling (.getData nil writer) which produced a raw TypeError. Made read-zip-manifest public (was defn-) with explicit detection of missing manifest.json, raising typed :invalid-penpot-file validation error. The existing catch path surfaces this hint as a friendly user error instead of the raw TypeError text. Add regression tests for both paths. 374 users were affected, 704 occurrences across 2.17.0-RC2/RC3/RC4. Fixes #10709. AI-assisted-by: minimax-m3 * :bug: Harden dnd/get-data against missing dataTransfer When the sortable hook or any caller passes a synthetic event without a dataTransfer property (e.g. a dragend fired after a drop that has already cleared the transfer), the previous implementation called .getData directly on the nil/undefined result and threw "Cannot read properties of undefined (reading 'getData')". Wrap the body in when-let so get-data returns nil cleanly when dataTransfer is missing. All three current callers (hooks.cljs:164, viewport/actions.cljs:531 and :562) already treat the return value as optional via when-let / when, so no caller breaks. Add a regression test covering both the missing-dataTransfer case and a real dataTransfer roundtrip. AI-assisted-by: minimax-m3 * :bug: Harden paste handler against missing clipboardData in forms When a paste event arrives without a clipboardData property (e.g. a programmatically dispatched ClipboardEvent in some browsers, or edge cases like dragging a file with no text content), the previous implementation called .getData directly on the nil/undefined clipboardData and threw "Cannot read properties of undefined (reading 'getData')". Wrap the body in when-let so the paste logic is skipped entirely when clipboardData is missing. The existing (string? paste-data) guard in the inner when already tolerates nil; no other caller behavior changes. AI-assisted-by: minimax-m3 * :bug: Harden paste handler against missing clipboardData in components/forms Same defensive pattern as the main/ui/forms.cljs paste handler: wrap the body in when-let so the .getData call is skipped when the clipboardData property is missing on the paste event. Prevents the raw "Cannot read properties of undefined (reading 'getData')" TypeError for programmatic / edge-case paste events. AI-assisted-by: minimax-m3 * :bug: Harden v3 text editor paste and styles-fn against undefined receivers Two related fixes for the "Cannot read properties of undefined (reading 'getData')" family of bugs in the workspace text editor: - v3_editor.cljs: wrap the paste body in when-let on clipboardData so .getData("text/plain") is never called on a nil receiver. The existing (when (and text (seq text))) guard already tolerates nil text; only the outer .getData call was unprotected. - editor.cljs: add (and content ...) to the if branch in styles-fn so .getText and .getData are never called on a nil content. The else branch (legacy.txt/styles-to-attrs) is already the correct fallback for missing content. Add a regression test that mirrors the fixed patterns and verifies they no longer throw on synthetic events with no clipboardData or nil content. AI-assisted-by: minimax-m3 * :bug: Harden get-editor-block-data and get-editor-block-type against nil block getCurrentBlock from Draft.js can return undefined for an empty selection (e.g. before any block is created). The previous implementations called .getData / .getType directly on the result and threw "Cannot read properties of undefined (reading 'getData')" / "...reading 'getType')". Wrap both functions in (when (some? block) ...) so they return nil cleanly. Callers in editor.cljs and text_editor.cljs already handle nil results (render-block short-circuits via the case on type; the text-data caller in text_editor.cljs lets nil flow up), so no upstream change is required. Add a regression test covering both functions with nil and js/undefined input. AI-assisted-by: minimax-m3 * :bug: Harden draft-js block-data helpers against nil block Three related fixes in the vendored draft-js package: - mergeBlockData: early-return undefined when block is falsy. Without this, the first line (block.getData()) throws for callers that pass a nil block. - splitBlockPreservingData: guard the blockMap.get(...) lookup. If the start key is stale (e.g. after a Modifier.splitBlock that doesn't actually produce the expected key), .get() returns undefined and the subsequent .getData() throws. Fall back to an empty Immutable Map for the block data. - updateBlockData: short-circuit (return state unchanged) when mergeBlockData returns undefined. Without this, the chain newBlock.getData() would throw on the same nil-block case that mergeBlockData now guards. These match the defensive nil-handling pattern used elsewhere in the frontend (.getData callers) and protect against stale selection keys in the Draft.js content state. AI-assisted-by: minimax-m3 --- frontend/packages/draft-js/index.js | 10 ++- .../src/app/main/ui/components/forms.cljs | 39 ++++++------ frontend/src/app/main/ui/forms.cljs | 39 ++++++------ .../main/ui/workspace/shapes/text/editor.cljs | 2 +- .../ui/workspace/shapes/text/v3_editor.cljs | 16 ++--- frontend/src/app/util/dom/dnd.cljs | 14 ++--- frontend/src/app/util/text_editor.cljs | 8 ++- frontend/src/app/util/zip.cljs | 4 ++ frontend/src/app/worker/import.cljs | 10 ++- frontend/test/frontend_tests/runner.cljs | 8 +++ .../text_editor_paste_guard_test.cljs | 62 +++++++++++++++++++ .../frontend_tests/util/dom/dnd_test.cljs | 23 +++++++ .../frontend_tests/util_text_editor_test.cljs | 18 ++++++ .../test/frontend_tests/util_zip_test.cljs | 44 +++++++++++++ 14 files changed, 235 insertions(+), 62 deletions(-) create mode 100644 frontend/test/frontend_tests/text_editor_paste_guard_test.cljs create mode 100644 frontend/test/frontend_tests/util/dom/dnd_test.cljs create mode 100644 frontend/test/frontend_tests/util_text_editor_test.cljs create mode 100644 frontend/test/frontend_tests/util_zip_test.cljs diff --git a/frontend/packages/draft-js/index.js b/frontend/packages/draft-js/index.js index 400636c5dd..f02109f971 100644 --- a/frontend/packages/draft-js/index.js +++ b/frontend/packages/draft-js/index.js @@ -29,6 +29,7 @@ function isDefined(v) { } function mergeBlockData(block, newData) { + if (!block) return undefined; let data = block.getData(); for (let key of Object.keys(newData)) { @@ -176,10 +177,12 @@ export function splitBlockPreservingData(state) { content = Modifier.splitBlock(content, selection); - const blockData = content.blockMap.get(content.selectionBefore.getStartKey()).getData(); + const startKey = content.selectionBefore.getStartKey(); + const block = content.blockMap.get(startKey); + const blockData = (block && block.getData()) || new Map(); const blockKey = content.selectionAfter.getStartKey(); - const blockMap = content.blockMap.update(blockKey, (block) => { - return block.set("data", blockData); + const blockMap = content.blockMap.update(blockKey, (b) => { + return b.set("data", blockData); }); content = content.set("blockMap", blockMap); @@ -325,6 +328,7 @@ export function updateBlockData(state, blockKey, data) { const content = state.getCurrentContent(); const block = content.getBlockForKey(blockKey); const newBlock = mergeBlockData(block, data); + if (!newBlock) return state; const blockData = newBlock.getData(); diff --git a/frontend/src/app/main/ui/components/forms.cljs b/frontend/src/app/main/ui/components/forms.cljs index 89a1bcd42d..88585a753e 100644 --- a/frontend/src/app/main/ui/components/forms.cljs +++ b/frontend/src/app/main/ui/components/forms.cljs @@ -560,28 +560,29 @@ on-paste (mf/use-fn (fn [event] - (let [paste-data (-> event .-clipboardData (.getData "text"))] - (when (and (string? paste-data) - (re-find #"[,\s]" paste-data)) - (dom/prevent-default event) - (dom/stop-propagation event) + (when-let [clipboard-data (.-clipboardData event)] + (let [paste-data (.getData clipboard-data "text")] + (when (and (string? paste-data) + (re-find #"[,\s]" paste-data)) + (dom/prevent-default event) + (dom/stop-propagation event) - ;; Mark as touched - (swap! form assoc-in [:touched input-name] true) + ;; Mark as touched + (swap! form assoc-in [:touched input-name] true) - ;; Split pasted text by commas and/or whitespace, add each valid part - (let [parts (->> (str/split paste-data #",|\s+") - (map str/trim) - (remove str/empty?))] - (doseq [part parts] - (when (valid-item-fn part) - (swap! items conj-dedup {:text part - :valid true - :caution (caution-item-fn part)}))) + ;; Split pasted text by commas and/or whitespace, add each valid part + (let [parts (->> (str/split paste-data #",|\s+") + (map str/trim) + (remove str/empty?))] + (doseq [part parts] + (when (valid-item-fn part) + (swap! items conj-dedup {:text part + :valid true + :caution (caution-item-fn part)}))) - ;; Reset input value and mark as untouched after successful paste - (reset! value "") - (swap! form assoc-in [:touched input-name] false)))))) + ;; Reset input value and mark as untouched after successful paste + (reset! value "") + (swap! form assoc-in [:touched input-name] false))))))) on-blur (mf/use-fn diff --git a/frontend/src/app/main/ui/forms.cljs b/frontend/src/app/main/ui/forms.cljs index 82b0335008..9115761290 100644 --- a/frontend/src/app/main/ui/forms.cljs +++ b/frontend/src/app/main/ui/forms.cljs @@ -121,28 +121,29 @@ on-paste (mf/use-fn (fn [event] - (let [paste-data (-> event .-clipboardData (.getData "text"))] - (when (and (string? paste-data) - (re-find #"[,\s]" paste-data)) - (dom/prevent-default event) - (dom/stop-propagation event) + (when-let [clipboard-data (.-clipboardData event)] + (let [paste-data (.getData clipboard-data "text")] + (when (and (string? paste-data) + (re-find #"[,\s]" paste-data)) + (dom/prevent-default event) + (dom/stop-propagation event) - ;; Mark as touched - (swap! form assoc-in [:touched name] true) + ;; Mark as touched + (swap! form assoc-in [:touched name] true) - ;; Split pasted text by commas and/or whitespace, add each valid part - (let [parts (->> (str/split paste-data #",|\s+") - (map str/trim) - (remove str/empty?))] - (doseq [part parts] - (when (valid-item-fn part) - (swap! items conj-dedup {:text part - :valid true - :caution (caution-item-fn part)}))) + ;; Split pasted text by commas and/or whitespace, add each valid part + (let [parts (->> (str/split paste-data #",|\s+") + (map str/trim) + (remove str/empty?))] + (doseq [part parts] + (when (valid-item-fn part) + (swap! items conj-dedup {:text part + :valid true + :caution (caution-item-fn part)}))) - ;; Reset input value and mark as untouched after successful paste - (reset! value "") - (swap! form assoc-in [:touched name] false)))))) + ;; Reset input value and mark as untouched after successful paste + (reset! value "") + (swap! form assoc-in [:touched name] false))))))) on-blur (mf/use-fn diff --git a/frontend/src/app/main/ui/workspace/shapes/text/editor.cljs b/frontend/src/app/main/ui/workspace/shapes/text/editor.cljs index 766acb2430..060ee0bada 100644 --- a/frontend/src/app/main/ui/workspace/shapes/text/editor.cljs +++ b/frontend/src/app/main/ui/workspace/shapes/text/editor.cljs @@ -61,7 +61,7 @@ nil))) (defn- styles-fn [shape styles content] - (let [data (if (= (.getText ^js content) "") + (let [data (if (and content (= (.getText ^js content) "")) (-> ^js (.getData content) (.toJS) (js->clj :keywordize-keys true)) diff --git a/frontend/src/app/main/ui/workspace/shapes/text/v3_editor.cljs b/frontend/src/app/main/ui/workspace/shapes/text/v3_editor.cljs index 749c5af69f..8db3275dad 100644 --- a/frontend/src/app/main/ui/workspace/shapes/text/v3_editor.cljs +++ b/frontend/src/app/main/ui/workspace/shapes/text/v3_editor.cljs @@ -105,14 +105,14 @@ (mf/use-fn (fn [^js event] (dom/prevent-default event) - (let [clipboard-data (.-clipboardData event) - text (.getData clipboard-data "text/plain")] - (when (and text (seq text)) - (text-editor/text-editor-insert-text text) - (sync-wasm-text-editor-content!) - (wasm.api/request-render "text-paste")) - (when-let [node (mf/ref-val contenteditable-ref)] - (set! (.-textContent node) ""))))) + (when-let [clipboard-data (.-clipboardData event)] + (let [text (.getData clipboard-data "text/plain")] + (when (and text (seq text)) + (text-editor/text-editor-insert-text text) + (sync-wasm-text-editor-content!) + (wasm.api/request-render "text-paste")))) + (when-let [node (mf/ref-val contenteditable-ref)] + (set! (.-textContent node) "")))) on-copy (mf/use-fn diff --git a/frontend/src/app/util/dom/dnd.cljs b/frontend/src/app/util/dom/dnd.cljs index 2a1732d6c2..bb647db785 100644 --- a/frontend/src/app/util/dom/dnd.cljs +++ b/frontend/src/app/util/dom/dnd.cljs @@ -115,13 +115,13 @@ ([e] (get-data e "penpot/data")) ([e data-type] - (let [dt (.-dataTransfer e) - data (.getData dt data-type)] - (cond-> data - (and (some? data) (not= data "") - (or (str/starts-with? data-type "penpot") - (= data-type "application/json"))) - (t/decode-str))))) + (when-let [dt (.-dataTransfer e)] + (let [data (.getData dt data-type)] + (cond-> data + (and (some? data) (not= data "") + (or (str/starts-with? data-type "penpot") + (= data-type "application/json"))) + (t/decode-str)))))) (defn get-files [e] diff --git a/frontend/src/app/util/text_editor.cljs b/frontend/src/app/util/text_editor.cljs index f46ed9aa35..4e91ee80ef 100644 --- a/frontend/src/app/util/text_editor.cljs +++ b/frontend/src/app/util/text_editor.cljs @@ -60,12 +60,14 @@ (defn get-editor-block-data [block] - (-> (.getData ^js block) - (immutable-map->map))) + (when (some? block) + (-> (.getData ^js block) + (immutable-map->map)))) (defn get-editor-block-type [block] - (.getType ^js block)) + (when (some? block) + (.getType ^js block))) (defn get-editor-current-block-data [state] diff --git a/frontend/src/app/util/zip.cljs b/frontend/src/app/util/zip.cljs index 72571f2e79..2288cbdaae 100644 --- a/frontend/src/app/util/zip.cljs +++ b/frontend/src/app/util/zip.cljs @@ -82,6 +82,10 @@ (defn read-as-text [entry] + (when (nil? entry) + (ex/raise :type :assertion + :code :invalid-entry + :hint "cannot read zip entry: entry is nil")) (let [writer (new zip/TextWriter)] (.getData entry writer))) diff --git a/frontend/src/app/worker/import.cljs b/frontend/src/app/worker/import.cljs index 91ce6b92c8..ba6a49ce1e 100644 --- a/frontend/src/app/worker/import.cljs +++ b/frontend/src/app/worker/import.cljs @@ -7,6 +7,7 @@ (ns app.worker.import (:refer-clojure :exclude [resolve]) (:require + [app.common.exceptions :as ex] [app.common.json :as json] [app.common.logging :as log] [app.common.schema :as sm] @@ -44,10 +45,15 @@ (def conjv (fnil conj [])) -(defn- read-zip-manifest +(defn read-zip-manifest [zip-reader] (->> (rx/from (uz/get-entry zip-reader "manifest.json")) - (rx/mapcat uz/read-as-text) + (rx/mapcat (fn [entry] + (if (nil? entry) + (rx/throw (ex/error :type :validation + :code :invalid-penpot-file + :hint "Not a valid Penpot file: manifest.json is missing")) + (uz/read-as-text entry)))) (rx/map json/decode))) (defn slurp-uri diff --git a/frontend/test/frontend_tests/runner.cljs b/frontend/test/frontend_tests/runner.cljs index 128ce0c216..cda245671d 100644 --- a/frontend/test/frontend_tests/runner.cljs +++ b/frontend/test/frontend_tests/runner.cljs @@ -47,6 +47,7 @@ [frontend-tests.plugins.value-objects-test] [frontend-tests.render-wasm.process-objects-test] [frontend-tests.svg-fills-test] + [frontend-tests.text-editor-paste-guard-test] [frontend-tests.tokens.import-export-test] [frontend-tests.tokens.logic.token-actions-test] [frontend-tests.tokens.logic.token-data-test] @@ -60,7 +61,10 @@ [frontend-tests.util-object-test] [frontend-tests.util-range-tree-test] [frontend-tests.util-simple-math-test] + [frontend-tests.util-text-editor-test] [frontend-tests.util-webapi-test] + [frontend-tests.util-zip-test] + [frontend-tests.util.dom.dnd-test] [frontend-tests.worker-snap-test] [goog.object :as gobj])) @@ -132,10 +136,14 @@ 'frontend-tests.ui.ds-controls-numeric-input-test 'frontend-tests.ui.measures-menu-props-test 'frontend-tests.render-wasm.process-objects-test + 'frontend-tests.text-editor-paste-guard-test 'frontend-tests.util-object-test 'frontend-tests.util-range-tree-test 'frontend-tests.util-simple-math-test + 'frontend-tests.util-text-editor-test 'frontend-tests.util-webapi-test + 'frontend-tests.util.dom.dnd-test + 'frontend-tests.util-zip-test 'frontend-tests.worker-snap-test]) (assert (every? find-ns-obj test-namespaces) diff --git a/frontend/test/frontend_tests/text_editor_paste_guard_test.cljs b/frontend/test/frontend_tests/text_editor_paste_guard_test.cljs new file mode 100644 index 0000000000..3a7829f450 --- /dev/null +++ b/frontend/test/frontend_tests/text_editor_paste_guard_test.cljs @@ -0,0 +1,62 @@ +;; This Source Code Form is subject to the terms of the Mozilla Public +;; License, v. 2.0. If a copy of the MPL was not distributed with this +;; file, You can obtain one at http://mozilla.org/MPL/2.0/. +;; +;; Copyright (c) KALEIDOS INC + +(ns frontend-tests.text-editor-paste-guard-test + "Regression tests for the Cannot read properties of undefined + (reading getData) family of bugs. Each test verifies that the inner + getData call is now guarded by an outer when-let on clipboardData + so that a synthetic event with no clipboardData no longer throws." + (:require + [cljs.test :as t :include-macros true])) + +(defn- guarded-get-data-text + "Mirrors the body of the fixed paste handlers in main/ui/forms.cljs and + main/ui/components/forms.cljs." + [event] + (when-let [clipboard-data (.-clipboardData event)] + (.getData clipboard-data "text"))) + +(defn- guarded-get-data-text-plain + "Mirrors the body of the fixed paste handler in + main/ui/workspace/shapes/text/v3_editor.cljs." + [event] + (when-let [clipboard-data (.-clipboardData event)] + (.getData clipboard-data "text/plain"))) + +(t/deftest guarded-paste-handlers-do-not-throw-on-missing-clipboardData + (t/testing "event without clipboardData returns nil (no throw)" + (t/is (nil? (guarded-get-data-text #js {}))) + (t/is (nil? (guarded-get-data-text-plain #js {})))) + (t/testing "event with explicit nil clipboardData returns nil" + (t/is (nil? (guarded-get-data-text #js {:clipboardData nil}))) + (t/is (nil? (guarded-get-data-text-plain #js {:clipboardData nil})))) + (t/testing "event with valid clipboardData returns the text" + (let [text-cb (fn [t] (if (= t "text") "hello" nil)) + text-plain-cb (fn [t] (if (= t "text/plain") "hello" nil)) + cb #js {:getData (fn [t] (if (= t "text") "hello" nil))} + cbp #js {:getData (fn [t] (if (= t "text/plain") "hello" nil))}] + (t/is (= "hello" (guarded-get-data-text #js {:clipboardData cb}))) + (t/is (= "hello" (guarded-get-data-text-plain #js {:clipboardData cbp})))))) + +;; Mirrors the fixed styles-fn body in main/ui/workspace/shapes/text/editor.cljs. +;; The fix adds an (and content ...) guard so getText and getData are never +;; called on a nil content object. +(defn- guarded-styles-fn-branch + "Returns the data that styles-fn would use for a given content. When content + is nil, the function falls back to the styles-only branch and returns :fallback + (the real function calls legacy.txt/styles-to-attrs which we don't exercise + here — we only verify the guard itself prevents the getText/getData throws)." + [content] + (if (and content (= (.getText ^js content) "")) + (-> ^js (.getData content) + (.toJS) + (js->clj :keywordize-keys true)) + :fallback)) + +(t/deftest guarded-styles-fn-branch-does-not-throw-on-nil-content + (t/testing "nil content falls back to the styles branch (no throw)" + (t/is (= :fallback (guarded-styles-fn-branch nil))) + (t/is (= :fallback (guarded-styles-fn-branch js/undefined))))) diff --git a/frontend/test/frontend_tests/util/dom/dnd_test.cljs b/frontend/test/frontend_tests/util/dom/dnd_test.cljs new file mode 100644 index 0000000000..60c7ad725d --- /dev/null +++ b/frontend/test/frontend_tests/util/dom/dnd_test.cljs @@ -0,0 +1,23 @@ +;; This Source Code Form is subject to the terms of the Mozilla Public +;; License, v. 2.0. If a copy of the MPL was not distributed with this +;; file, You can obtain one at http://mozilla.org/MPL/2.0/. +;; +;; Copyright (c) KALEIDOS INC + +(ns frontend-tests.util.dom.dnd-test + (:require + [app.util.dom.dnd :as dnd] + [cljs.test :as t :include-macros true])) + +(t/deftest get-data-returns-nil-when-event-has-no-dataTransfer + (t/testing "event without dataTransfer" + (t/is (nil? (dnd/get-data #js {})))) + (t/testing "event with explicit nil dataTransfer" + (t/is (nil? (dnd/get-data #js {:dataTransfer nil})))) + (t/testing "explicit data-type also returns nil for missing dataTransfer" + (t/is (nil? (dnd/get-data #js {} "penpot/data"))))) + +(t/deftest get-data-reads-from-dataTransfer + (t/testing "dataTransfer with matching key returns the value (non-decoded type)" + (let [dt #js {:getData (fn [_type] "hello")}] + (t/is (= "hello" (dnd/get-data #js {:dataTransfer dt} "text/plain")))))) diff --git a/frontend/test/frontend_tests/util_text_editor_test.cljs b/frontend/test/frontend_tests/util_text_editor_test.cljs new file mode 100644 index 0000000000..0823a77d10 --- /dev/null +++ b/frontend/test/frontend_tests/util_text_editor_test.cljs @@ -0,0 +1,18 @@ +;; This Source Code Form is subject to the terms of the Mozilla Public +;; License, v. 2.0. If a copy of the MPL was not distributed with this +;; file, You can obtain one at http://mozilla.org/MPL/2.0/. +;; +;; Copyright (c) KALEIDOS INC + +(ns frontend-tests.util-text-editor-test + (:require + [app.util.text-editor :as te] + [cljs.test :as t :include-macros true])) + +(t/deftest get-editor-block-data-returns-nil-for-nil-block + (t/is (nil? (te/get-editor-block-data nil))) + (t/is (nil? (te/get-editor-block-data js/undefined)))) + +(t/deftest get-editor-block-type-returns-nil-for-nil-block + (t/is (nil? (te/get-editor-block-type nil))) + (t/is (nil? (te/get-editor-block-type js/undefined)))) diff --git a/frontend/test/frontend_tests/util_zip_test.cljs b/frontend/test/frontend_tests/util_zip_test.cljs new file mode 100644 index 0000000000..7cf2d6f609 --- /dev/null +++ b/frontend/test/frontend_tests/util_zip_test.cljs @@ -0,0 +1,44 @@ +;; This Source Code Form is subject to the terms of the Mozilla Public +;; License, v. 2.0. If a copy of the MPL was not distributed with this +;; file, You can obtain one at http://mozilla.org/MPL/2.0/. +;; +;; Copyright (c) KALEIDOS INC + +(ns frontend-tests.util-zip-test + (:require + [app.util.zip :as-alias uz] + [app.worker.import :as worker.import] + [beicon.v2.core :as rx] + [cljs.test :as t :include-macros true] + [frontend-tests.helpers.mock :as mock] + [promesa.core :as p])) + +(t/deftest read-as-text-nil-entry-raises-typed-error + (t/testing "read-as-text guards against nil entry" + (t/is (thrown-with-msg? js/Error #"nil" + (app.util.zip/read-as-text nil))))) + +(t/deftest read-zip-manifest-missing-throws-validation-error + (t/async + done + (t/testing "read-zip-manifest rejects ZIPs without manifest.json" + (mock/with-mocks + {app.util.zip/get-entry (mock/stub (fn [_ _] (p/resolved nil)))} + (fn [done'] + (->> (worker.import/read-zip-manifest #js {}) + (rx/subs! + (fn [_] + (t/is false "expected validation error to be thrown") + (done')) + (fn [err] + (let [data (ex-data err)] + (t/is (= :invalid-penpot-file (:code data)) + "missing manifest.json raises typed :invalid-penpot-file error") + (t/is (string? (:hint data)) + "missing manifest.json error carries a :hint") + (t/is (re-find #"manifest\.json" (:hint data)) + "missing manifest.json :hint mentions manifest.json") + (t/is (nil? (re-find #"getData" (:hint data))) + "missing manifest.json :hint does not leak the raw TypeError text") + (done')))))) + done)))) \ No newline at end of file From af120feb1f86bffeb1abcea2f9b0717e3c338e82 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Mon, 27 Jul 2026 12:11:12 +0200 Subject: [PATCH 2/4] :bug: Fix workspace crash and cleanup viewport_ref event/resize handling (#10721) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace `globals/document` and `globals/window` with `js/document` and `js/window` in workspace.cljs, removing the unused `app.util.globals` import. This avoids "can't access dead object" errors in Firefox when navigating between pages/files, matching the existing pattern used in viewport/hooks.cljs. Fix a leaked MOUSELEAVE listener in viewport_ref.cljs — the ref callback added a new listener on every mount but never unregistered the previous one. Now uses standard .addEventListener/.removeEventListener with a React ref to track the handler for proper cleanup. Fix ResizeObserver cleanup in viewport_ref.cljs — `init-observer` is now a private function that only creates an observer when a node is provided, and cleanup is handled via the ref callback on unmount. AI-assisted-by: mimo-v2.5-pro --- frontend/src/app/main/ui/workspace.cljs | 5 +- .../ui/workspace/viewport/viewport_ref.cljs | 95 +++++++++++-------- 2 files changed, 56 insertions(+), 44 deletions(-) diff --git a/frontend/src/app/main/ui/workspace.cljs b/frontend/src/app/main/ui/workspace.cljs index e7a3e7b119..124eee9b1b 100644 --- a/frontend/src/app/main/ui/workspace.cljs +++ b/frontend/src/app/main/ui/workspace.cljs @@ -44,7 +44,6 @@ [app.main.ui.workspace.webgl-unavailable-modal] [app.util.debug :as dbg] [app.util.dom :as dom] - [app.util.globals :as globals] [app.util.i18n :as i18n :refer [tr]] [goog.events :as events] [okulary.core :as l] @@ -177,7 +176,7 @@ (mf/with-effect [] (let [focus-out #(st/emit! (dw/workspace-focus-lost)) - key (events/listen globals/window "blur" focus-out)] + key (events/listen js/window "blur" focus-out)] (partial events/unlistenByKey key))) (mf/with-effect [file-id page-id] @@ -252,7 +251,7 @@ (let [handle-wasm-render (fn [_] (reset! first-frame-rendered? true)) - listener-key (events/listen globals/document "penpot:wasm:render" handle-wasm-render)] + listener-key (events/listen js/document "penpot:wasm:render" handle-wasm-render)] (fn [] (events/unlistenByKey listener-key)))) diff --git a/frontend/src/app/main/ui/workspace/viewport/viewport_ref.cljs b/frontend/src/app/main/ui/workspace/viewport/viewport_ref.cljs index 46e1356596..38349f8f24 100644 --- a/frontend/src/app/main/ui/workspace/viewport/viewport_ref.cljs +++ b/frontend/src/app/main/ui/workspace/viewport/viewport_ref.cljs @@ -7,62 +7,75 @@ (ns app.main.ui.workspace.viewport.viewport-ref (:require [app.common.data :as d] - [app.common.data.macros :as dm] [app.common.geom.point :as gpt] + [app.main.refs :as refs] [app.main.store :as st] [app.util.dom :as dom] [app.util.mouse :as mse] - [goog.events :as events] - [rumext.v2 :as mf]) - (:import goog.events.EventType)) + [rumext.v2 :as mf])) (defonce viewport-ref (atom nil)) -(defonce current-observer (atom nil)) (defonce viewport-brect (atom nil)) -(defn init-observer - [node on-change-bounds] +(defn- init-observer + [node] + (let [on-change-bounds + (fn [_] + (let [brect (dom/get-bounding-rect node) + brect (gpt/point (d/parse-integer (:left brect)) + (d/parse-integer (:top brect)))] + (reset! viewport-brect brect))) - (let [observer (js/ResizeObserver. on-change-bounds)] - (when (some? @current-observer) - (.disconnect @current-observer)) + observer + (js/ResizeObserver. on-change-bounds)] - (reset! current-observer observer) - - (when (some? node) - (.observe observer node)))) - -(defn on-change-bounds - [_] - (when @viewport-ref - (let [brect (dom/get-bounding-rect @viewport-ref) - brect (gpt/point (d/parse-integer (:left brect)) - (d/parse-integer (:top brect)))] - (reset! viewport-brect brect)))) + (.observe observer node) + observer)) (defn create-viewport-ref [] - (let [ref (mf/use-ref nil)] - [ref - (mf/use-memo - #(fn [node] - (mf/set-ref-val! ref node) - (reset! viewport-ref node) - (when (some? node) - (events/listen node EventType.MOUSELEAVE (fn [] (st/emit! (mse/->BlurEvent))))) - (init-observer node on-change-bounds)))])) + (let [node-ref (mf/use-ref nil) + handler-ref (mf/use-ref nil) + observer-ref (mf/use-ref nil) + callback (mf/use-fn + (fn [node] + ;; Dispose all previous resources + (when-let [observer (mf/ref-val observer-ref)] + (.disconnect ^js observer) + (mf/set-ref-val! observer-ref nil)) + + + (when-let [handler (mf/ref-val handler-ref)] + (when-let [node (mf/ref-val node-ref)] + (.removeEventListener ^js node "mouseleave" handler) + (mf/set-ref-val! handler-ref nil))) + + ;; Reset the ref values to the current node (can be nil) + (mf/set-ref-val! node-ref node) + (reset! viewport-ref node) + + (when (some? node) + (let [handler (fn [] (st/emit! (mse/->BlurEvent))) + observer (init-observer node)] + (.addEventListener ^js node "mouseleave" handler) + + (mf/set-ref-val! handler-ref handler) + (mf/set-ref-val! observer-ref observer)))))] + [node-ref callback])) (defn point->viewport [pt] - (let [zoom (dm/get-in @st/state [:workspace-local :zoom] 1)] - (when (and (some? @viewport-ref) - (some? @viewport-brect)) - (let [vbox (.. ^js @viewport-ref -viewBox -baseVal) - brect @viewport-brect - box (gpt/point (.-x vbox) (.-y vbox)) - zoom (gpt/point zoom)] + (let [zoom (d/nilv @refs/selected-zoom 1) + viewport-node @viewport-ref + viewport-brect @viewport-brect] - (-> (gpt/subtract pt brect) + (when (and (some? viewport-brect) + (some? viewport-node)) + (let [vbox (.. ^js viewport-node -viewBox -baseVal) + box (gpt/point (.-x vbox) (.-y vbox)) + zoom (gpt/point zoom)] + + (-> (gpt/subtract pt viewport-brect) (gpt/divide zoom) (gpt/add box)))))) @@ -71,8 +84,8 @@ Unlike point->viewport, this does NOT convert to canvas coordinates - it just subtracts the viewport's bounding rect offset." [pt] - (when (some? @viewport-brect) - (gpt/subtract pt @viewport-brect))) + (when-let [brect @viewport-brect] + (gpt/subtract pt brect))) (defn inside-viewport? [target] From 63e0c536f0caed806e9087094dbb1281caaf44a5 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Mon, 27 Jul 2026 13:24:11 +0000 Subject: [PATCH 3/4] :sparkles: Align event names column in format-last-events output The third column (event name) in error report "last events" now starts at a consistent position regardless of the delta value, by right-padding the delta string to 10 characters. The first event always shows (+0ms). Adds tests for empty, single, multi-event, and column alignment cases. AI-assisted-by: deepseek-v4-flash --- frontend/src/app/main/store.cljs | 15 +++--- .../test/frontend_tests/data/store_test.cljs | 49 +++++++++++++++++++ frontend/test/frontend_tests/runner.cljs | 2 + 3 files changed, 59 insertions(+), 7 deletions(-) create mode 100644 frontend/test/frontend_tests/data/store_test.cljs diff --git a/frontend/src/app/main/store.cljs b/frontend/src/app/main/store.cljs index 9ecfc39284..dc389033ae 100644 --- a/frontend/src/app/main/store.cljs +++ b/frontend/src/app/main/store.cljs @@ -107,8 +107,8 @@ (defn format-last-events "Render the `last-events` buffer as a multi-line string with the wall-clock time of each event and the delta (ms) since the previous - entry. The first entry has no delta. Useful for embedding in error - reports." + entry. The delta column is right-padded to 10 chars so the event + names align. Useful for embedding in error reports." ([] (format-last-events @last-events)) ([events] (let [lines @@ -117,13 +117,14 @@ out (transient [])] (if xs (let [{:keys [name t]} (first xs) - iso (ct/format-inst t :iso) - tail (if prev-t - (str " (+" (ct/diff-ms prev-t t) "ms)") - "")] + iso (ct/format-inst t :iso) + delta (if prev-t + (str "(+" (ct/diff-ms prev-t t) "ms)") + "(+0ms)") + delta-pad (str/pad delta {:length 10 :type :right})] (recur t (next xs) - (conj! out (str iso tail " " name)))) + (conj! out (str iso " " delta-pad " " name)))) (persistent! out)))] (str/join "\n" lines)))) diff --git a/frontend/test/frontend_tests/data/store_test.cljs b/frontend/test/frontend_tests/data/store_test.cljs new file mode 100644 index 0000000000..56d29fec85 --- /dev/null +++ b/frontend/test/frontend_tests/data/store_test.cljs @@ -0,0 +1,49 @@ +;; This Source Code Form is subject to the terms of the Mozilla Public +;; License, v. 2.0. If a copy of the MPL was not distributed with this +;; file, You can obtain one at http://mozilla.org/MPL/2.0/. +;; +;; Copyright (c) KALEIDOS INC Sucursal en España SL + +(ns frontend-tests.data.store-test + "Unit tests for app.main.store. + Tests cover: + - format-last-events – empty, single, multi, column alignment" + (:require + [app.main.store :as st] + [cljs.test :as t :include-macros true] + [cuerdas.core :as str])) + +(t/deftest format-last-events-empty + (t/testing "empty events produce empty string" + (t/is (= "" (st/format-last-events []))))) + +(t/deftest format-last-events-single + (t/testing "a single event shows (+0ms) and its name" + (let [result (st/format-last-events [{:name ":test/event" :t (js/Date. 1000)}])] + (t/is (str/includes? result "(+0ms)")) + (t/is (str/includes? result ":test/event")) + (t/is (= 1 (count (str/split result "\n"))))))) + +(t/deftest format-last-events-multi + (t/testing "multiple events show correct deltas and event names" + (let [events [{:name ":event/a" :t (js/Date. 0)} + {:name ":event/b" :t (js/Date. 500)} + {:name ":event/c" :t (js/Date. 2500)}] + lines (str/split (st/format-last-events events) "\n")] + (t/is (= 3 (count lines))) + (t/is (some #(str/includes? % ":event/a") lines)) + (t/is (some #(str/includes? % ":event/b") lines)) + (t/is (some #(str/includes? % ":event/c") lines)) + (t/is (str/includes? (nth lines 0) "(+0ms)")) + (t/is (str/includes? (nth lines 1) "(+500ms)")) + (t/is (str/includes? (nth lines 2) "(+2000ms)"))))) + +(t/deftest format-last-events-alignment + (t/testing "event names start at the same column across all lines" + (let [events [{:name ":evt-a" :t (js/Date. 0)} + {:name ":evt-b" :t (js/Date. 500)}] + lines (str/split (st/format-last-events events) "\n") + col-a (.indexOf (nth lines 0) ":evt-a") + col-b (.indexOf (nth lines 1) ":evt-b")] + (t/is (pos? col-a)) + (t/is (= col-a col-b))))) diff --git a/frontend/test/frontend_tests/runner.cljs b/frontend/test/frontend_tests/runner.cljs index cda245671d..450f6978bd 100644 --- a/frontend/test/frontend_tests/runner.cljs +++ b/frontend/test/frontend_tests/runner.cljs @@ -9,6 +9,7 @@ [frontend-tests.copy-as-svg-test] [frontend-tests.data.nitrate-test] [frontend-tests.data.repo-test] + [frontend-tests.data.store-test] [frontend-tests.data.uploads-test] [frontend-tests.data.viewer-test] [frontend-tests.data.workspace-colors-test] @@ -86,6 +87,7 @@ 'frontend-tests.copy-as-svg-test 'frontend-tests.data.nitrate-test 'frontend-tests.data.repo-test + 'frontend-tests.data.store-test 'frontend-tests.errors-test 'frontend-tests.main-errors-test 'frontend-tests.data.uploads-test From 6063a45c3ca12a67acd73482174657151950c602 Mon Sep 17 00:00:00 2001 From: Alejandro Alonso Date: Tue, 28 Jul 2026 10:26:31 +0200 Subject: [PATCH 4/4] :bug: Fix area selection aborted by select-shapes interrupt (#10870) * :bug: Fix area selection aborted by select-shapes interrupt Only emit :interrupt from select-shapes when edition mode is active. Unconditional :interrupt (from #10798) made drag-stopper cancel the marquee mid-drag. * :wrench: Fix text editor v2 fill e2e test on develop --- frontend/playwright/ui/specs/text-editor-v2.spec.js | 1 - frontend/src/app/main/data/workspace/selection.cljs | 6 +++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/frontend/playwright/ui/specs/text-editor-v2.spec.js b/frontend/playwright/ui/specs/text-editor-v2.spec.js index 2219a4ea8c..a58c71dba4 100644 --- a/frontend/playwright/ui/specs/text-editor-v2.spec.js +++ b/frontend/playwright/ui/specs/text-editor-v2.spec.js @@ -344,7 +344,6 @@ test("Preserves empty fill after editing text without changes", async ({ page }) await workspace.createTextShape(190, 150, 300, 200, initialText); await workspace.textEditor.stopEditing(); - await workspace.page.getByRole('button', { name: 'Fill', exact: true }).click(); const fillColorButton = workspace.page.getByRole("button", { name: "#000000", diff --git a/frontend/src/app/main/data/workspace/selection.cljs b/frontend/src/app/main/data/workspace/selection.cljs index 3eab8b2d6b..36039f230b 100644 --- a/frontend/src/app/main/data/workspace/selection.cljs +++ b/frontend/src/app/main/data/workspace/selection.cljs @@ -275,7 +275,11 @@ ;; the event loop expand-s (->> (rx/of (dwc/expand-all-parents ids objects)) (rx/observe-on :async)) - interrupt-s (rx/of :interrupt ::dwsp/interrupt)] + ;; :interrupt aborts drag-stopper; only emit it when clearing edition + ;; (unconditional emit broke marquee selection after #10798). + interrupt-s (if (some? (dm/get-in state [:workspace-local :edition])) + (rx/of :interrupt ::dwsp/interrupt) + (rx/of ::dwsp/interrupt))] (rx/merge expand-s interrupt-s))))) (defn select-all