diff --git a/frontend/src/app/main/data/changes.cljs b/frontend/src/app/main/data/changes.cljs index 844fa1197c..ef18c15554 100644 --- a/frontend/src/app/main/data/changes.cljs +++ b/frontend/src/app/main/data/changes.cljs @@ -92,15 +92,17 @@ (effect [_ state _] (when (and wasm/context-initialized? (not @wasm/context-lost?)) - (let [objects (dsh/lookup-page-objects state)] - (doseq [{:keys [type id parent-id]} redo-changes - :when (contains? wasm-structural-change-types type) - :let [shape-id (case type - :add-obj id - :mov-objects parent-id) - shape (get objects shape-id)] - :when shape] - (wasm.api/process-object shape)) + (let [objects (dsh/lookup-page-objects state) + shapes + (into [] + (keep (fn [{:keys [type id parent-id]}] + (when (contains? wasm-structural-change-types type) + (get objects (case type + :add-obj id + :mov-objects parent-id))))) + redo-changes)] + + (wasm.api/process-objects shapes) (wasm.api/request-render "sync-wasm-structural-changes")))))) (defn- apply-changes-localy diff --git a/frontend/src/app/main/data/workspace/clipboard.cljs b/frontend/src/app/main/data/workspace/clipboard.cljs index baea2b8c3d..265ff3139b 100644 --- a/frontend/src/app/main/data/workspace/clipboard.cljs +++ b/frontend/src/app/main/data/workspace/clipboard.cljs @@ -743,7 +743,8 @@ (update :fills translate-fills) (update :strokes translate-strokes) (d/update-when :content #(txt/transform-nodes process-text-node %)) - (d/update-when :position-data #(mapv process-text-node %))))) + ;; Removes the position-data so it's regenerated + (dissoc :position-data)))) ;; Analyze the rchange and replace staled media and ;; references to the new uploaded media-objects. diff --git a/frontend/src/app/render_wasm/api.cljs b/frontend/src/app/render_wasm/api.cljs index 71402806b4..57748e48c4 100644 --- a/frontend/src/app/render_wasm/api.cljs +++ b/frontend/src/app/render_wasm/api.cljs @@ -1418,6 +1418,24 @@ (let [{:keys [thumbnails full]} (set-object shape)] (process-pending [shape] thumbnails full noop-fn))) +(defn process-objects + "Like process-object but for multiple shapes at once. Accumulates all + pending font/image callbacks before calling process-pending, so that + update-text-layouts fires for all text shapes after fonts load — not + just the first shape that triggered the fetch." + [shapes] + (let [total-shapes (count shapes) + {:keys [thumbnails full]} + (loop [index 0 thumbnails-acc (transient []) full-acc (transient [])] + (if (< index total-shapes) + (let [shape (nth shapes index) + {:keys [thumbnails full]} (set-object shape)] + (recur (inc index) + (reduce conj! thumbnails-acc thumbnails) + (reduce conj! full-acc full))) + {:thumbnails (persistent! thumbnails-acc) :full (persistent! full-acc)}))] + (process-pending shapes thumbnails full noop-fn))) + (defn- process-shapes-chunk "Process shapes starting at `start-index` until the time budget is exhausted. Returns {:thumbnails [...] :full [...] :next-index n}" diff --git a/frontend/test/frontend_tests/logic/copying_and_duplicating_test.cljs b/frontend/test/frontend_tests/logic/copying_and_duplicating_test.cljs index 186fa3071c..b1c19573c7 100644 --- a/frontend/test/frontend_tests/logic/copying_and_duplicating_test.cljs +++ b/frontend/test/frontend_tests/logic/copying_and_duplicating_test.cljs @@ -562,3 +562,74 @@ ;; Total = 1 + 2 + 3 + 4 + 2 + 3 + 4 + 4 + 3 + 2 + 1 = 29 (t/is (= (count (:objects page')) 29))))))))) +;; --------------------------------------------------------------------------- +;; Tests for issue-14302 +;; Pasting a board with text shapes must clear stale position-data so the +;; workspace can remeasure the text and avoid incorrect line breaks. +;; --------------------------------------------------------------------------- + +(defn- setup-board-with-texts + "Two-page file. Page 1 has a board with two text shapes that carry stale + position-data (simulating a shape that was already laid out on a + different page). Page 2 is empty. Current page is page-1." + [] + (let [stale-pd [{:x 999 :y 999 :width 999 :height 20 :fills [] :text "stale"}]] + (-> (cthf/sample-file :file1 :page-label :page-1) + (ctho/add-frame :frame-1 :name "board-with-texts") + (cths/add-sample-shape :text-a :type :text :parent-label :frame-1 :name "text-a") + (cths/update-shape :text-a :position-data stale-pd) + (cths/add-sample-shape :text-b :type :text :parent-label :frame-1 :name "text-b") + (cths/update-shape :text-b :position-data stale-pd) + (cthf/add-sample-page :page-2) + (cthf/switch-to-page :page-1)))) + +(t/deftest paste-to-empty-page-clears-text-position-data + "Regression for issue-14302: copying a board with text shapes and pasting + onto an empty page produced incorrect line breaks because stale + position-data from the source page was preserved." + (t/async done + (with-redefs [uuid/next cthi/next-uuid] + (let [file (setup-board-with-texts) + store (ths/setup-store file) + ;; copy-paste-shape handles initialize-page + select-shape (needed + ;; for calculate-paste-position) + paste-shapes + return to page-1 + events (copy-paste-shape :frame-1 file + :target-page-label :page-2 + :target-container-id uuid/zero)] + + (ths/run-store + store done events + (fn [new-state] + (let [file' (ths/get-file-from-state new-state) + page-2 (cthf/get-page file' :page-2) + pasted-texts (->> (vals (:objects page-2)) + (filter #(= :text (:type %))))] + (t/is (= 2 (count pasted-texts)) + "Both text shapes are pasted onto the empty page") + (t/is (every? #(nil? (:position-data %)) pasted-texts) + "Pasted text shapes have nil position-data so they get remeasured")))))))) + +(t/deftest paste-to-same-page-clears-text-position-data + "Position-data is stripped on paste regardless of destination page." + (t/async done + (with-redefs [uuid/next cthi/next-uuid] + (let [file (setup-board-with-texts) + store (ths/setup-store file) + events (copy-paste-shape :frame-1 file + :target-container-id uuid/zero)] + + (ths/run-store + store done events + (fn [new-state] + (let [file' (ths/get-file-from-state new-state) + page-1' (cthf/get-page file' :page-1) + ;; Pasted copies have IDs not registered in idmap — they show + ;; up as "" strings from cthi/label. + pasted-texts (->> (vals (:objects page-1')) + (filter #(= :text (:type %))) + (remove #(keyword? (cthi/label (:id %)))))] + (t/is (= 2 (count pasted-texts)) + "Two new text shapes are pasted onto the same page") + (t/is (every? #(nil? (:position-data %)) pasted-texts) + "Pasted text shapes have nil position-data")))))))) + diff --git a/frontend/test/frontend_tests/render_wasm/process_objects_test.cljs b/frontend/test/frontend_tests/render_wasm/process_objects_test.cljs new file mode 100644 index 0000000000..e20aa2e1bc --- /dev/null +++ b/frontend/test/frontend_tests/render_wasm/process_objects_test.cljs @@ -0,0 +1,110 @@ +;; 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.render-wasm.process-objects-test + "Unit tests for wasm.api/process-objects. + + The key invariant: process-objects must call set-object for every shape + and forward ALL shapes to a single process-pending invocation. + + Without this batching the async font-load callback only covers the shape + that triggered the font fetch. Subsequent text shapes that share the same + font URL get no callback (fetch-font returns nil when the URL is already + in :fetching) and are permanently stuck with fallback-font layout metrics." + (:require + [app.render-wasm.api :as wasm.api] + [beicon.v2.core :as rx] + [cljs.test :as t :include-macros true])) + +;; --------------------------------------------------------------------------- +;; Helpers +;; --------------------------------------------------------------------------- + +(defn- make-shape [type] + {:id (random-uuid) :type type}) + +(defn- with-mocks* + "Temporarily replaces wasm.api/set-object and wasm.api/process-pending, + calls (thunk), then restores the originals." + [mock-set-object mock-process-pending thunk] + (let [orig-set-object wasm.api/set-object + orig-process-pending wasm.api/process-pending] + (set! wasm.api/set-object mock-set-object) + (set! wasm.api/process-pending mock-process-pending) + (try + (thunk) + (finally + (set! wasm.api/set-object orig-set-object) + (set! wasm.api/process-pending orig-process-pending))))) + +;; --------------------------------------------------------------------------- +;; Tests +;; --------------------------------------------------------------------------- + +(t/deftest process-objects-calls-set-object-for-every-shape + "Each shape in the input must go through set-object exactly once." + (let [shapes [(make-shape :text) (make-shape :text) (make-shape :rect)] + visited-ids (atom []) + mock-set (fn [s] (swap! visited-ids conj (:id s)) {:thumbnails [] :full []}) + mock-pend (fn [_sh _t _f _cb] nil)] + + (with-mocks* mock-set mock-pend #(wasm.api/process-objects shapes)) + + (t/is (= (mapv :id shapes) @visited-ids) + "set-object called once per shape in order"))) + +(t/deftest process-objects-calls-process-pending-once-with-all-shapes + "process-pending must receive ALL shapes in a single call. + This is the invariant that makes the async font-load callback update text + layouts for every text shape, not just the first one that triggered the + font fetch." + (let [shapes [(make-shape :text) (make-shape :text)] + captured (atom nil) + mock-set (fn [_s] {:thumbnails [] :full []}) + mock-pend (fn [sh t f cb] (reset! captured {:shapes sh :thumbnails t :full f :cb cb}))] + + (with-mocks* mock-set mock-pend #(wasm.api/process-objects shapes)) + + (t/is (some? @captured) + "process-pending was called") + (t/is (= 2 (count (:shapes @captured))) + "process-pending received all shapes") + (t/is (= (set (map :id shapes)) + (set (map :id (:shapes @captured)))) + "process-pending received the correct shape objects"))) + +(t/deftest process-objects-accumulates-callbacks-across-shapes + "Pending font/image callbacks from all shapes must be merged into the single + process-pending call. This covers the deduplication scenario: when a second + text shape shares the same font (fetch-font returns nil because the URL is + already in :fetching), the callback from the first shape still covers the + second shape because both are in the :shapes list passed to process-pending." + (let [shape-a (make-shape :text) + shape-b (make-shape :text) + shapes [shape-a shape-b] + font-cb (fn [] (rx/of true)) + captured (atom nil) + + ;; Simulate deduplication: shape-a triggers a real font fetch, + ;; shape-b gets an empty pending list (same URL already in :fetching). + mock-set + (fn [s] + (if (= (:id s) (:id shape-a)) + {:thumbnails [{:key "font-url" :callback font-cb}] :full []} + {:thumbnails [] :full []})) + + mock-pend + (fn [sh t f _cb] (reset! captured {:shapes sh :thumbnails t :full f}))] + + (with-mocks* mock-set mock-pend #(wasm.api/process-objects shapes)) + + ;; The single font callback from shape-a is present... + (t/is (= 1 (count (:thumbnails @captured))) + "The font callback from shape-a is forwarded") + ;; ...and BOTH shapes are in the list, so when the font loads and + ;; process-pending fires update-text-layouts, it covers shape-b too. + (t/is (= 2 (count (:shapes @captured))) + "Both shapes are in process-pending so font-load covers all of them"))) diff --git a/frontend/test/frontend_tests/runner.cljs b/frontend/test/frontend_tests/runner.cljs index d7048409a8..e96ceece01 100644 --- a/frontend/test/frontend_tests/runner.cljs +++ b/frontend/test/frontend_tests/runner.cljs @@ -28,6 +28,7 @@ [frontend-tests.plugins.parser-test] [frontend-tests.plugins.tokens-test] [frontend-tests.plugins.utils-test] + [frontend-tests.render-wasm.process-objects-test] [frontend-tests.svg-fills-test] [frontend-tests.tokens.import-export-test] [frontend-tests.tokens.logic.token-actions-test] @@ -85,6 +86,7 @@ frontend-tests.tokens.token-errors-test frontend-tests.tokens.workspace-tokens-remap-test frontend-tests.ui.ds-controls-numeric-input-test + frontend-tests.render-wasm.process-objects-test frontend-tests.util-object-test frontend-tests.util-range-tree-test frontend-tests.util-simple-math-test