From 01363be3a8310a6ce42fbddd29b206aa03afcd54 Mon Sep 17 00:00:00 2001 From: Eva Marco Date: Tue, 22 Sep 2026 12:59:42 +0200 Subject: [PATCH] :bug: Fix text variant changing text content on variant switch (#11815) --- common/src/app/common/types/text.cljc | 8 ++- common/test/common_tests/types/text_test.cljc | 60 +++++++++++++++++ .../app/main/data/workspace/libraries.cljs | 2 +- .../workspace_variant_switch_wasm_test.cljs | 67 +++++++++++++++++++ 4 files changed, 134 insertions(+), 3 deletions(-) create mode 100644 frontend/test/frontend_tests/data/workspace_variant_switch_wasm_test.cljs diff --git a/common/src/app/common/types/text.cljc b/common/src/app/common/types/text.cljc index f7aeb37664..04db3bb313 100644 --- a/common/src/app/common/types/text.cljc +++ b/common/src/app/common/types/text.cljc @@ -257,8 +257,12 @@ (= a b) #{} - ;; If types are different, the structure is different - (not= (type a) (type b)) + ;; If one is a map and the other isn't, the structure is different. + ;; Compare by category (map?), not by `type`: a map's underlying + ;; PersistentArrayMap/PersistentHashMap representation depends on its + ;; key count, not on the shape of the content tree, so two maps that + ;; only differ in size can otherwise report a false structure diff. + (not= (map? a) (map? b)) #{:text-content-structure} ;; If they are maps, check the keys diff --git a/common/test/common_tests/types/text_test.cljc b/common/test/common_tests/types/text_test.cljc index 8165558bed..ace36f0c4d 100644 --- a/common/test/common_tests/types/text_test.cljc +++ b/common/test/common_tests/types/text_test.cljc @@ -125,6 +125,58 @@ (assoc-in content-base [:children 0 :children 0 :children 0 :fills] [{:fill-color "#ff0000" :fill-opacity 1}])) +;; The newer text editor duplicates typography attrs onto the paragraph +;; node (not just the leaf) once a non-default style is applied. That +;; pushes the paragraph map's key count from 4 to 9, crossing the +;; PersistentArrayMap/PersistentHashMap promotion threshold, which +;; changes its `type` even though it's still just a map. These fixtures +;; are built as raw content, matching real editor output exactly +;; (content-base's paragraph already carries every default attr, so it +;; can't reproduce the 4-key vs 9-key gap seen in practice). +(defn- leaf-text-node + [font-weight font-variant-id] + {:line-height "1.2" + :font-style "normal" + :typography-ref-id nil + :text-transform "none" + :font-id "sourcesanspro" + :font-size "14" + :font-weight font-weight + :typography-ref-file nil + :font-variant-id font-variant-id + :text-decoration "none" + :letter-spacing "0" + :fills [{:fill-color "#000000" :fill-opacity 1}] + :font-family "sourcesanspro" + :text "hello world"}) + +(defn- text-content + [paragraph-extra-attrs font-weight font-variant-id] + {:type "root" + :vertical-align "top" + :children + [{:type "paragraph-set" + :children + [(merge {:type "paragraph" + :text-align "left" + :text-direction "ltr" + :children [(leaf-text-node font-weight font-variant-id)]} + paragraph-extra-attrs)]}]}) + +;; A plain, unstyled paragraph: only paragraph-specific attrs, 4 keys. +(def content-plain-compact-paragraph + (text-content {} "400" "regular")) + +;; Same bold text/attrs, but the paragraph node also carries the +;; duplicated attrs (9 keys), like the newer text editor produces. +(def content-bold-duplicated-paragraph + (text-content {:font-id "sourcesanspro" + :font-weight "700" + :font-variant-id "bold" + :font-family "sourcesanspro" + :font-style "normal"} + "700" "bold")) + (t/deftest test-get-diff-type (let [diff-text (cttx/get-diff-type content-base content-changed-text) @@ -189,6 +241,14 @@ (t/is (= #{:text-content-attribute} diff-typography-ref)) (t/is (= #{:text-content-attribute} diff-fills)))) +(t/deftest test-get-diff-type-paragraph-key-count + ;; A real attribute difference (400 vs 700), where only one side's + ;; paragraph node has the duplicated keys (9, crossing the map + ;; promotion threshold): must be reported as :text-content-attribute, + ;; not misclassified as :text-content-structure. + (t/is (= #{:text-content-attribute} (cttx/get-diff-type content-plain-compact-paragraph + content-bold-duplicated-paragraph)))) + (t/deftest test-get-diff-attrs (let [attrs-text (cttx/get-diff-attrs content-base content-changed-text) diff --git a/frontend/src/app/main/data/workspace/libraries.cljs b/frontend/src/app/main/data/workspace/libraries.cljs index da2b59b341..ae4928cfc4 100644 --- a/frontend/src/app/main/data/workspace/libraries.cljs +++ b/frontend/src/app/main/data/workspace/libraries.cljs @@ -1106,7 +1106,7 @@ (dwtp/propagate-workspace-tokens) ;; Make the new instance get the token values from the current file, not from the component's library (when (and (features/active-feature? state "render-wasm/v1") (seq new-text-ids)) - (dwwt/resize-wasm-text-all new-text-ids)) + (dwwt/resize-wasm-text-all new-text-ids {:skip-component-sync? true})) (ptk/data-event :layout/update {:ids update-layout-ids :undo-group undo-group}) (dwu/commit-undo-transaction undo-id) (dws/select-shape (:id new-shape) false)))))) diff --git a/frontend/test/frontend_tests/data/workspace_variant_switch_wasm_test.cljs b/frontend/test/frontend_tests/data/workspace_variant_switch_wasm_test.cljs new file mode 100644 index 0000000000..a27ec9b8d7 --- /dev/null +++ b/frontend/test/frontend_tests/data/workspace_variant_switch_wasm_test.cljs @@ -0,0 +1,67 @@ +;; 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 SUBSIDIARY SL + +(ns frontend-tests.data.workspace-variant-switch-wasm-test + "Regression test for #10588: switching a component's variant while + render-wasm/v1 is active must not let the derived WASM text-resize + commit trigger a component sync that discards the switch's merged + overrides. See commit fb22c1547c for the same class of bug." + (:require + [app.common.test-helpers.components :as cthc] + [app.common.test-helpers.compositions :as ctho] + [app.common.test-helpers.files :as cthf] + [app.common.test-helpers.ids-map :as cthi] + [app.common.test-helpers.shapes :as cths] + [app.main.data.workspace.libraries :as dwl] + [app.main.data.workspace.wasm-text :as dwwt] + [cljs.test :as t :include-macros true] + [frontend-tests.helpers.state :as ths])) + +(def ^:private orig-resize-wasm-text-all dwwt/resize-wasm-text-all) + +(def ^:private resize-calls (atom [])) + +(defn- mock-resize-wasm-text-all + ([ids] (mock-resize-wasm-text-all ids nil)) + ([ids opts] + (swap! resize-calls conj {:ids ids :opts opts}) + nil)) + +(t/use-fixtures :each + {:before (fn [] + (reset! resize-calls []) + (set! dwwt/resize-wasm-text-all mock-resize-wasm-text-all)) + :after (fn [] + (set! dwwt/resize-wasm-text-all orig-resize-wasm-text-all))}) + +(defn- setup-file + [] + (-> (cthf/sample-file :file1) + (ctho/add-frame :root-a) + (cthc/make-component :comp-a :root-a) + (ctho/add-frame-with-text :root-b :text-b "hello") + (cthc/make-component :comp-b :root-b) + (cthc/instantiate-component :comp-a :copy1))) + +(t/deftest test-variant-switch-marks-wasm-resize-as-skip-component-sync + (t/async + done + (let [file (setup-file) + store (ths/setup-store file) + copy1 (cths/get-shape file :copy1) + comp-b-id (cthi/id :comp-b) + + events + [(dwl/component-swap copy1 (:id file) comp-b-id true)]] + + (ths/run-store + store done events + (fn [_new-state] + (t/is (= 1 (count @resize-calls)) + "resize-wasm-text-all should be called once for the swapped-in text") + (let [{:keys [opts]} (first @resize-calls)] + (t/is (:skip-component-sync? opts) + "the derived WASM resize commit must be tagged :skip-component-sync?, or watch-component-changes will treat it as a real edit and clobber the switch's merged overrides")))))))