From 4bf70de73caec6d097a0ccf4bc7e99d20dd7afe8 Mon Sep 17 00:00:00 2001 From: Luis de Dios Date: Fri, 23 Oct 2026 13:32:39 +0200 Subject: [PATCH] :bug: Restrict per-side stroke width tokens to boards and rects in plugin API (#12332) * :bug: Fix restrict per-side stroke width tokens to rectangles in plugins API * :recycle: Reuse per-side stroke restriction check across plugin API Move the per-side stroke width restriction into the public `per-side-stroke-attrs-restricted?` helper, now taking `page-id`, and reuse it from both `shape.applyToken` and the token application path. Keep the error message in a single `per-side-stroke-widths-message` def used by all three rejection sites. Wait for token resolution in the uniform stroke width regression test instead of relying on a bare `settle`, which could assert before the applied token lands. AI-assisted-by: deepseek-v4.1-flash --------- Co-authored-by: Luis de Dios --- frontend/src/app/plugins/shape.cljs | 9 +- frontend/src/app/plugins/tokens.cljs | 47 ++++++++-- .../frontend_tests/plugins/tokens_test.cljs | 87 +++++++++++++++++++ plugins/CHANGELOG.md | 1 + .../src/tests/tokens.test.ts | 49 +++++++++++ plugins/libs/plugin-types/index.d.ts | 9 ++ 6 files changed, 193 insertions(+), 9 deletions(-) diff --git a/frontend/src/app/plugins/shape.cljs b/frontend/src/app/plugins/shape.cljs index b34b329598..817492536c 100644 --- a/frontend/src/app/plugins/shape.cljs +++ b/frontend/src/app/plugins/shape.cljs @@ -66,7 +66,7 @@ [app.plugins.strokes :as strokes] [app.plugins.system-events :as se] [app.plugins.text :as text] - [app.plugins.tokens :refer [applied-tokens-plugin->applied-tokens expand-token-attrs token-attr-plugin->token-attr token-attr? valid-token-resolution?]] + [app.plugins.tokens :refer [applied-tokens-plugin->applied-tokens expand-token-attrs per-side-stroke-attrs-restricted? per-side-stroke-widths-message token-attr-plugin->token-attr token-attr? valid-token-resolution?]] [app.plugins.utils :as u] [app.util.http :as http] [app.util.object :as obj] @@ -292,7 +292,7 @@ (and (not (ctt/per-side-stroke-shape? (:type (u/proxy->shape self)))) (some #(= :multiple (types.stroke/width-type %)) value)) - (u/not-valid plugin-id :strokes "Per-side stroke widths require a board or rectangle") + (u/not-valid plugin-id :strokes per-side-stroke-widths-message) :else (st/emit! (dwsh/update-shapes [id] #(assoc % :strokes value) options)))))) @@ -1840,6 +1840,11 @@ (some #(not (token-attr? %)) kw-attrs) (u/not-valid plugin-id :applyToken attrs) + ;; The uniform `strokeWidth` alias is exempt: it expands + ;; to a uniform width on every side. + (per-side-stroke-attrs-restricted? file-id page-id [id] attrs) + (u/not-valid plugin-id :applyToken per-side-stroke-widths-message) + :else (st/emit! (-> (dwta/toggle-token {:token token diff --git a/frontend/src/app/plugins/tokens.cljs b/frontend/src/app/plugins/tokens.cljs index f1895d7efc..4e21174bab 100644 --- a/frontend/src/app/plugins/tokens.cljs +++ b/frontend/src/app/plugins/tokens.cljs @@ -111,6 +111,20 @@ (or (= attr stroke-width-alias) (cto/token-attr? attr)))) +(def per-side-stroke-widths-message + "Per-side stroke widths require a board or rectangle") + +(defn per-side-stroke-attrs? + "Returns true when the plugin attrs name at least one explicit per-side stroke + width attribute (`strokeWidthTop/Right/Bottom/Left`). The uniform + `strokeWidth` alias is not considered per-side: it expands to all four sides + with the same token and always resolves to a uniform width." + [attrs] + (boolean + (some #(contains? cto/per-side-stroke-width-keys + (token-attr-plugin->token-attr %)) + attrs))) + (defn- token-name-schema [file-id set-id token] (let [tokens-lib (u/locate-tokens-lib file-id) @@ -181,23 +195,41 @@ (get (:name token)) (resolved-without-errors?)))) +(defn per-side-stroke-attrs-restricted? + "Returns true when `attrs` name an explicit per-side stroke width while some + target shape on `page-id` does not support independent sides. Only boards and + rectangles accept them. Shapes absent from the page are ignored, because + token application only touches the active page." + [file-id page-id shape-ids attrs] + (when (per-side-stroke-attrs? attrs) + (boolean + (some (fn [id] + (let [shape (u/locate-shape file-id page-id id)] + (and (some? shape) + (not (cto/per-side-stroke-shape? (:type shape)))))) + shape-ids)))) + (defn- apply-token-to-shapes - [plugin-id file-id set-id id shape-ids attrs] + [plugin-id code file-id set-id id shape-ids attrs] (cond (not (r/check-permission plugin-id "content:write")) - (u/not-valid plugin-id :applyToken "Plugin doesn't have 'content:write' permission") + (u/not-valid plugin-id code "Plugin doesn't have 'content:write' permission") :else (let [token (u/locate-token file-id set-id id)] (cond (nil? token) - (u/not-valid plugin-id :applyToSelected id) + (u/not-valid plugin-id code id) (not (valid-token-resolution? file-id set-id id)) - (u/not-valid plugin-id :applyToSelected (:value token)) + (u/not-valid plugin-id code (:value token)) (some #(not (token-attr? %)) attrs) - (u/not-valid plugin-id :applyToSelected attrs) + (u/not-valid plugin-id code attrs) + + (per-side-stroke-attrs-restricted? file-id (:current-page-id @st/state) + shape-ids attrs) + (u/not-valid plugin-id code per-side-stroke-widths-message) :else (st/emit! @@ -443,14 +475,15 @@ [:vector [:fn shape-proxy?]] [:maybe [::sm/set [:and ::sm/keyword [:fn token-attr?]]]]] :fn (fn [shapes attrs] - (apply-token-to-shapes plugin-id file-id set-id id (map #(obj/get % "$id") shapes) attrs))} + (apply-token-to-shapes plugin-id :applyToShapes file-id set-id id + (map #(obj/get % "$id") shapes) attrs))} :applyToSelected {:enumerable false :schema [:tuple [:maybe [::sm/set [:and ::sm/keyword [:fn token-attr?]]]]] :fn (fn [attrs] (let [selected (get-in @st/state [:workspace-local :selected])] - (apply-token-to-shapes plugin-id file-id set-id id selected attrs)))})) + (apply-token-to-shapes plugin-id :applyToSelected file-id set-id id selected attrs)))})) ;; === Token Set diff --git a/frontend/test/frontend_tests/plugins/tokens_test.cljs b/frontend/test/frontend_tests/plugins/tokens_test.cljs index f731c98910..5f47fb92b3 100644 --- a/frontend/test/frontend_tests/plugins/tokens_test.cljs +++ b/frontend/test/frontend_tests/plugins/tokens_test.cljs @@ -545,6 +545,23 @@ (t/is (= #{:fill} (ptok/expand-token-attrs #{:fill}))) (t/is (= #{} (ptok/expand-token-attrs nil))))) +(t/deftest per-side-stroke-attrs?-distinguishes-sides-from-the-uniform-alias + (t/testing "the uniform alias is not per-side" + (t/is (false? (ptok/per-side-stroke-attrs? #{:stroke-width}))) + (t/is (false? (ptok/per-side-stroke-attrs? #{"stroke-width"})))) + + (t/testing "each explicit side is per-side, as keyword or string" + (t/is (true? (ptok/per-side-stroke-attrs? #{:stroke-width-top}))) + (t/is (true? (ptok/per-side-stroke-attrs? #{:stroke-width-right}))) + (t/is (true? (ptok/per-side-stroke-attrs? #{:stroke-width-bottom}))) + (t/is (true? (ptok/per-side-stroke-attrs? #{:stroke-width-left}))) + (t/is (true? (ptok/per-side-stroke-attrs? #{"stroke-width-left"})))) + + (t/testing "unrelated and empty attrs are not per-side" + (t/is (false? (ptok/per-side-stroke-attrs? #{:fill}))) + (t/is (false? (ptok/per-side-stroke-attrs? #{}))) + (t/is (false? (ptok/per-side-stroke-attrs? nil))))) + (t/deftest shape-apply-token-accepts-padding-top (t/async done @@ -646,6 +663,76 @@ (done))) 0)))) +;; Regression coverage for issue #12319. +;; +;; Per-side stroke widths are only supported on boards and rectangles, and the +;; `strokes` setter already rejects them elsewhere. The token path did not: +;; `shape.applyToken` / `token.applyToShapes` / `token.applyToSelected` bound a +;; per-side width token to an ellipse and left the stroke in the per-side state +;; the strokes setter refuses. The token path now rejects the four explicit side +;; properties on unsupported shapes while keeping the uniform `strokeWidth` +;; alias working. + +(t/deftest ^:async apply-token-rejects-per-side-stroke-width-on-unsupported-shape + (let [plugin-id "00000000-0000-0000-0000-000000000000" + set-id (cthi/new-id! :token-set) + token-id (cthi/new-id! :stroke-width-token) + theme-id (cthi/new-id! :theme) + file (-> (ctht/sample-file-with-tokens + :file-id :file1 + :lib-fn #(-> % + (ctob/add-set + (ctob/make-token-set :id set-id + :name "widths")) + (ctob/add-theme + (ctob/make-token-theme :id theme-id + :name "theme" + :sets #{"widths"})) + (ctob/add-token + set-id + (ctob/make-token :id token-id + :name "width.sm" + :type :stroke-width + :value 10))) + :status-fn #(ctos/set-tokens-status % #{theme-id} #{set-id}))) + store (ths/setup-store file) + _ (set! st/state store) + _ (set! st/stream (ptk/input-stream store)) + ^js context (api/create-context plugin-id) + ^js ellipse (.createEllipse context) + id (obj/get ellipse "$id") + ^js library (.-library context) + ^js local (.-local library) + ^js catalog (.-tokens local) + ^js token-set (.getSetById catalog (str set-id)) + ^js token (.getTokenById token-set (str token-id)) + path [:files (:id file) :data :pages-index (cthf/current-page-id file) + :objects id :applied-tokens]] + + (swap! store assoc-in [:plugins :flags plugin-id :throw-validation-errors] true) + + (t/testing "shape.applyToken rejects an explicit per-side width" + (t/is (thrown? js/Error (.applyToken ellipse token #js ["strokeWidthTop"]))) + (t/is (nil? (get-in @store path)))) + + (t/testing "token.applyToShapes rejects an explicit per-side width" + (t/is (thrown? js/Error (.applyToShapes token #js [ellipse] #js ["strokeWidthLeft"]))) + (t/is (nil? (get-in @store path)))) + + (t/testing "token.applyToSelected rejects an explicit per-side width" + (swap! store assoc-in [:workspace-local :selected] #{id}) + (t/is (thrown? js/Error (.applyToSelected token #js ["strokeWidthRight"]))) + (t/is (nil? (get-in @store path)))) + + ;; The uniform alias stays valid: it expands to the four sides with the same + ;; token and resolves to a uniform width, which the strokes setter accepts. + (t/testing "the uniform strokeWidth alias is still accepted" + (.applyToken ellipse token #js ["strokeWidth"]) + (await (async/wait-for + #(= "width.sm" (get-in @store (conj path :stroke-width-top))) + "uniform stroke width token applied")) + (t/is (= "width.sm" (get-in @store (conj path :stroke-width-top))))))) + (t/deftest token-attr?-rejects-unknown-input (t/is (false? (boolean (ptok/token-attr? :not-a-real-attr)))) (t/is (false? (boolean (ptok/token-attr? "not-a-real-attr")))) diff --git a/plugins/CHANGELOG.md b/plugins/CHANGELOG.md index fbd46deec9..3ea8fd12c9 100644 --- a/plugins/CHANGELOG.md +++ b/plugins/CHANGELOG.md @@ -22,6 +22,7 @@ - **plugins-runtime**: The plugin modal no longer shows two resize grips in Firefox. Firefox now shows only its native grip. Closes #11795. - **plugins-runtime**: Fixed the `fontFamilies` token property mapping so `Shape.applyToken(token, ["fontFamilies"])` resolves to the canonical `:font-family` attribute and applied-token readback exposes the documented `fontFamilies` key instead of the undocumented singular `fontFamily`. Closes #11405. - **plugins-runtime**: Opening a plugin no longer drops running background plugins (`allowBackground`, such as the MCP plugin) from the runtime registry. They kept running, but their UI messages were no longer delivered to the plugin and they could no longer be unloaded, so MCP tasks timed out after another plugin was opened. +- **plugins-runtime**: `Shape.applyToken()`, `Token.applyToShapes()` and `Token.applyToSelected()` now reject an explicit per-side stroke width property (`strokeWidthTop`, `strokeWidthRight`, `strokeWidthBottom`, `strokeWidthLeft`) on shapes other than boards and rectangles, matching the `strokes` setter. The uniform `strokeWidth` property keeps working on every shape. Closes #12319. ## 1.5.0 (2026-07-08) diff --git a/plugins/apps/plugin-api-test-suite/src/tests/tokens.test.ts b/plugins/apps/plugin-api-test-suite/src/tests/tokens.test.ts index 095a86ac17..2614617c77 100644 --- a/plugins/apps/plugin-api-test-suite/src/tests/tokens.test.ts +++ b/plugins/apps/plugin-api-test-suite/src/tests/tokens.test.ts @@ -456,6 +456,55 @@ describe('Tokens', () => { expect(keys).not.toContain('strokeWidthRight'); }); + // Issue #12319: per-side widths are only supported on boards and + // rectangles. `applyToken` used to bind them on any shape, leaving an + // ellipse in the per-side state the `strokes` setter rejects. + test('applyToken rejects an explicit per-side width on an ellipse', (ctx) => { + const set = activeSet(ctx, unique('set')); + const token = set.addToken({ + type: 'borderWidth', + name: unique('width.'), + value: '4', + }); + const ellipse = ctx.penpot.createEllipse(); + ctx.board.appendChild(ellipse); + + expect(() => ellipse.applyToken(token, ['strokeWidthTop'])).toThrow(); + expect(ellipse.tokens.strokeWidthTop).toBeUndefined(); + }); + + test('applyToShapes rejects an explicit per-side width on an ellipse', (ctx) => { + const set = activeSet(ctx, unique('set')); + const token = set.addToken({ + type: 'borderWidth', + name: unique('width.'), + value: '4', + }); + const ellipse = ctx.penpot.createEllipse(); + ctx.board.appendChild(ellipse); + + expect(() => + token.applyToShapes([ellipse], ['strokeWidthLeft']), + ).toThrow(); + expect(ellipse.tokens.strokeWidthLeft).toBeUndefined(); + }); + + test('applyToken keeps accepting the uniform strokeWidth alias on an ellipse', async (ctx) => { + const set = activeSet(ctx, unique('set')); + const token = set.addToken({ + type: 'borderWidth', + name: unique('width.'), + value: '4', + }); + const ellipse = ctx.penpot.createEllipse(); + ctx.board.appendChild(ellipse); + + ellipse.applyToken(token, ['strokeWidth']); + + await waitFor(() => ellipse.tokens.strokeWidth === token.name); + expect(ellipse.tokens.strokeWidth).toBe(token.name); + }); + for (const side of [ 'strokeWidthTop', 'strokeWidthRight', diff --git a/plugins/libs/plugin-types/index.d.ts b/plugins/libs/plugin-types/index.d.ts index aa7008dc13..6d5e8c5408 100644 --- a/plugins/libs/plugin-types/index.d.ts +++ b/plugins/libs/plugin-types/index.d.ts @@ -4174,6 +4174,11 @@ export interface ShapeBase extends PluginData { * several tokens with the same name in different sets, the actual token applied * and the value set to the attributes will depend on which sets are active * (and will change if different sets or themes are activated later). + * + * NOTE that the per-side stroke width properties (`strokeWidthTop`, + * `strokeWidthRight`, `strokeWidthBottom`, `strokeWidthLeft`) are only valid on + * boards and rectangles. Applying one to any other shape raises a validation + * error. The uniform `strokeWidth` property is accepted on every shape. */ applyToken(token: Token, properties?: TokenProperty[]): void; @@ -4662,6 +4667,10 @@ export interface TokenBase { * several tokens with the same name in different sets, the actual token applied * and the value set to the attributes will depend on which sets are active * (and will change if different sets or themes are activated later). + * + * NOTE that the per-side stroke width properties are only valid on boards and + * rectangles; applying one to any other shape raises a validation error. The + * uniform `strokeWidth` property is accepted on every shape. */ applyToShapes(shapes: Shape[], properties?: TokenProperty[]): void;