From d04cbf175e0a28ae74f7b960d31d8ece5e2b971c Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Fri, 31 Jul 2026 12:51:35 +0200 Subject: [PATCH] :bug: Fix nil dereference crash during flex layout drag operations (#10845) Production crash where @(get bounds id) threw "No protocol method IDeref.-deref defined for type null" when a shape ID had no corresponding entry in the bounds map during layout calculations. Added defensive nil guards (when-let / when) to all unprotected bounds dereference sites: - flex_layout/bounds.cljc: layout-content-points (parent + child) and layout-content-bounds - grid_layout/bounds.cljc: layout-content-points and layout-content-bounds - min_size_layout.cljc: child-min-width grid branch (3 sites) and child-min-height grid branch Added 7 new tests in geom_bounds_layout_nil_test.cljc covering all nil-bounds edge cases for flex, grid, and min-size layout paths. Registered in runner.cljc. Closes #10843 AI-assisted-by: qwen3.7-plus --- .../geom/shapes/flex_layout/bounds.cljc | 117 +++++----- .../geom/shapes/grid_layout/bounds.cljc | 54 ++--- .../common/geom/shapes/min_size_layout.cljc | 28 ++- .../geom_bounds_layout_nil_test.cljc | 213 ++++++++++++++++++ common/test/common_tests/runner.cljc | 2 + 5 files changed, 321 insertions(+), 93 deletions(-) create mode 100644 common/test/common_tests/geom_bounds_layout_nil_test.cljc diff --git a/common/src/app/common/geom/shapes/flex_layout/bounds.cljc b/common/src/app/common/geom/shapes/flex_layout/bounds.cljc index e6aef398f0..b434d99fc9 100644 --- a/common/src/app/common/geom/shapes/flex_layout/bounds.cljc +++ b/common/src/app/common/geom/shapes/flex_layout/bounds.cljc @@ -121,74 +121,79 @@ (defn layout-content-points [bounds parent children objects] - (let [parent-id (dm/get-prop parent :id) - parent-bounds @(get bounds parent-id) - reverse? (ctl/reverse? parent) - children (cond->> children (not reverse?) reverse)] + (let [parent-id (dm/get-prop parent :id) + parent-bounds (get bounds parent-id)] + (when-let [parent-bounds (some-> parent-bounds deref)] + (let [reverse? (ctl/reverse? parent) + children (cond->> children (not reverse?) reverse)] - (loop [children (seq children) - result (transient []) - correct-v (gpt/point 0)] + (loop [children (seq children) + result (transient []) + correct-v (gpt/point 0)] - (if (not children) - (persistent! result) + (if (not children) + (persistent! result) - (let [child (first children) - child-id (dm/get-prop child :id) - child-bounds @(get bounds child-id) - [margin-top margin-right margin-bottom margin-left] (ctl/child-margins child) + (let [child (first children) + child-id (dm/get-prop child :id) + child-bounds-ref (get bounds child-id) + child-bounds (some-> child-bounds-ref deref) + [margin-top margin-right margin-bottom margin-left] (ctl/child-margins child) - [child-bounds correct-v] - (if (or (ctl/fill-width? child) (ctl/fill-height? child)) - (child-layout-bound-points parent child parent-bounds child-bounds correct-v bounds objects) - [(->> child-bounds (map #(gpt/add % correct-v))) correct-v]) + [child-bounds correct-v] + (if (and child-bounds + (or (ctl/fill-width? child) (ctl/fill-height? child))) + (child-layout-bound-points parent child parent-bounds child-bounds correct-v bounds objects) + [(when child-bounds + (->> child-bounds (map #(gpt/add % correct-v)))) + correct-v]) - child-bounds - (when (d/not-empty? child-bounds) - (-> (gpo/parent-coords-bounds child-bounds parent-bounds) - (gpo/pad-points (- margin-top) (- margin-right) (- margin-bottom) (- margin-left))))] + child-bounds + (when (d/not-empty? child-bounds) + (-> (gpo/parent-coords-bounds child-bounds parent-bounds) + (gpo/pad-points (- margin-top) (- margin-right) (- margin-bottom) (- margin-left))))] - (recur (next children) - (cond-> result (some? child-bounds) (conj! child-bounds)) - correct-v)))))) + (recur (next children) + (cond-> result (some? child-bounds) (conj! child-bounds)) + correct-v)))))))) (defn layout-content-bounds [bounds {:keys [layout-padding] :as parent} children objects] - (let [parent-id (:id parent) - parent-bounds @(get bounds parent-id) + (let [parent-id (:id parent) + parent-bounds (get bounds parent-id)] + (when-let [parent-bounds (some-> parent-bounds deref)] + (let [row? (ctl/row? parent) + col? (ctl/col? parent) + space-around? (ctl/space-around? parent) + space-evenly? (ctl/space-evenly? parent) + content-evenly? (ctl/content-evenly? parent) + [layout-gap-row layout-gap-col] (ctl/gaps parent) - row? (ctl/row? parent) - col? (ctl/col? parent) - space-around? (ctl/space-around? parent) - space-evenly? (ctl/space-evenly? parent) - content-evenly? (ctl/content-evenly? parent) - [layout-gap-row layout-gap-col] (ctl/gaps parent) + row-pad (if (or (and col? space-evenly?) + (and col? space-around?) + (and row? content-evenly?)) + layout-gap-row + 0) - row-pad (if (or (and col? space-evenly?) - (and col? space-around?) - (and row? content-evenly?)) - layout-gap-row - 0) + col-pad (if (or (and row? space-evenly?) + (and row? space-around?) + (and col? content-evenly?)) + layout-gap-col + 0) - col-pad (if (or (and row? space-evenly?) - (and row? space-around?) - (and col? content-evenly?)) - layout-gap-col - 0) + {pad-top :p1 pad-right :p2 pad-bottom :p3 pad-left :p4} layout-padding + pad-top (+ (or pad-top 0) row-pad) + pad-right (+ (or pad-right 0) col-pad) + pad-bottom (+ (or pad-bottom 0) row-pad) + pad-left (+ (or pad-left 0) col-pad) - {pad-top :p1 pad-right :p2 pad-bottom :p3 pad-left :p4} layout-padding - pad-top (+ (or pad-top 0) row-pad) - pad-right (+ (or pad-right 0) col-pad) - pad-bottom (+ (or pad-bottom 0) row-pad) - pad-left (+ (or pad-left 0) col-pad) + layout-points + (layout-content-points bounds parent children objects)] - layout-points - (layout-content-points bounds parent children objects)] - - (if (d/not-empty? layout-points) - (-> layout-points - (gpo/merge-parent-coords-bounds parent-bounds) - (gpo/pad-points (- pad-top) (- pad-right) (- pad-bottom) (- pad-left))) - ;; Cannot create some bounds from the children so we return the parent's - parent-bounds))) + (if (d/not-empty? layout-points) + (-> layout-points + (gpo/merge-parent-coords-bounds parent-bounds) + (gpo/pad-points (- pad-top) (- pad-right) (- pad-bottom) (- pad-left))) + ;; Cannot create some bounds from the children so we return the parent's + parent-bounds))))) diff --git a/common/src/app/common/geom/shapes/grid_layout/bounds.cljc b/common/src/app/common/geom/shapes/grid_layout/bounds.cljc index 0628cf2060..caadff4766 100644 --- a/common/src/app/common/geom/shapes/grid_layout/bounds.cljc +++ b/common/src/app/common/geom/shapes/grid_layout/bounds.cljc @@ -12,36 +12,36 @@ (defn layout-content-points [bounds parent {:keys [row-tracks column-tracks]}] - (let [parent-id (:id parent) - parent-bounds @(get bounds parent-id) - - hv #(gpo/start-hv parent-bounds %) - vv #(gpo/start-vv parent-bounds %)] - (d/concat-vec - (->> row-tracks - (mapcat #(vector (:start-p %) - (gpt/add (:start-p %) (vv (:size %)))))) - (->> column-tracks - (mapcat #(vector (:start-p %) - (gpt/add (:start-p %) (hv (:size %))))))))) + (let [parent-id (:id parent) + parent-bounds (get bounds parent-id)] + (when-let [parent-bounds (some-> parent-bounds deref)] + (let [hv #(gpo/start-hv parent-bounds %) + vv #(gpo/start-vv parent-bounds %)] + (d/concat-vec + (->> row-tracks + (mapcat #(vector (:start-p %) + (gpt/add (:start-p %) (vv (:size %)))))) + (->> column-tracks + (mapcat #(vector (:start-p %) + (gpt/add (:start-p %) (hv (:size %))))))))))) (defn layout-content-bounds [bounds {:keys [layout-padding] :as parent} layout-data] - (let [parent-id (:id parent) - parent-bounds @(get bounds parent-id) + (let [parent-id (:id parent) + parent-bounds (get bounds parent-id)] + (when-let [parent-bounds (some-> parent-bounds deref)] + (let [{pad-top :p1 pad-right :p2 pad-bottom :p3 pad-left :p4} layout-padding + pad-top (or pad-top 0) + pad-right (or pad-right 0) + pad-bottom (or pad-bottom 0) + pad-left (or pad-left 0) - {pad-top :p1 pad-right :p2 pad-bottom :p3 pad-left :p4} layout-padding - pad-top (or pad-top 0) - pad-right (or pad-right 0) - pad-bottom (or pad-bottom 0) - pad-left (or pad-left 0) + layout-points (layout-content-points bounds parent layout-data)] - layout-points (layout-content-points bounds parent layout-data)] - - (if (d/not-empty? layout-points) - (-> layout-points - (gpo/merge-parent-coords-bounds parent-bounds) - (gpo/pad-points (- pad-top) (- pad-right) (- pad-bottom) (- pad-left))) - ;; Cannot create some bounds from the children so we return the parent's - parent-bounds))) + (if (d/not-empty? layout-points) + (-> layout-points + (gpo/merge-parent-coords-bounds parent-bounds) + (gpo/pad-points (- pad-top) (- pad-right) (- pad-bottom) (- pad-left))) + ;; Cannot create some bounds from the children so we return the parent's + parent-bounds))))) diff --git a/common/src/app/common/geom/shapes/min_size_layout.cljc b/common/src/app/common/geom/shapes/min_size_layout.cljc index 89f17871e3..57375098e9 100644 --- a/common/src/app/common/geom/shapes/min_size_layout.cljc +++ b/common/src/app/common/geom/shapes/min_size_layout.cljc @@ -31,13 +31,17 @@ (and (ctl/fill-width? child) (ctl/grid-layout? child)) - (let [children - (->> (cfh/get-immediate-children objects (:id child)) - (remove ctl/position-absolute?) - (map #(vector @(get bounds (:id %)) %))) - layout-data (gd/calc-layout-data child @(get bounds (:id child)) children bounds objects true)] - (max (ctl/child-min-width child) - (gpo/width-points (gb/layout-content-bounds bounds child layout-data)))) + (let [child-bounds-ref (get bounds (:id child))] + (if child-bounds-ref + (let [children + (->> (cfh/get-immediate-children objects (:id child)) + (remove ctl/position-absolute?) + (keep #(when-let [b (get bounds (:id %))] + [@b %]))) + layout-data (gd/calc-layout-data child @child-bounds-ref children bounds objects true)] + (max (ctl/child-min-width child) + (gpo/width-points (gb/layout-content-bounds bounds child layout-data)))) + (ctl/child-min-width child))) (ctl/fill-width? child) (ctl/child-min-width child) @@ -63,11 +67,15 @@ (let [children (->> (cfh/get-immediate-children objects (dm/get-prop child :id)) (remove ctl/position-absolute?) - (map (fn [child] [@(get bounds (:id child)) child]))) + (keep (fn [c] + (when-let [b (get bounds (:id c))] + [@b c])))) layout-data (gd/calc-layout-data child (:points child) children bounds objects true) auto-bounds (gb/layout-content-bounds bounds child layout-data)] - (max (ctl/child-min-height child) - (gpo/height-points auto-bounds))) + (if auto-bounds + (max (ctl/child-min-height child) + (gpo/height-points auto-bounds)) + (ctl/child-min-height child))) (ctl/fill-height? child) (ctl/child-min-height child) diff --git a/common/test/common_tests/geom_bounds_layout_nil_test.cljc b/common/test/common_tests/geom_bounds_layout_nil_test.cljc new file mode 100644 index 0000000000..db070ce4f4 --- /dev/null +++ b/common/test/common_tests/geom_bounds_layout_nil_test.cljc @@ -0,0 +1,213 @@ +;; 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 common-tests.geom-bounds-layout-nil-test + (:require + [app.common.data :as d] + [app.common.geom.bounds-map :as gbm] + [app.common.geom.point :as gpt] + [app.common.geom.rect :as grc] + [app.common.geom.shapes.flex-layout.bounds :as fb] + [app.common.geom.shapes.grid-layout.bounds :as gb] + [app.common.geom.shapes.min-size-layout :as msl] + [app.common.types.shape :as cts] + [app.common.types.shape.layout :as ctl] + [app.common.uuid :as uuid] + [clojure.test :as t])) + +;; ---- Helpers ---- + +(defn- make-rect + [id x y w h] + (-> (cts/setup-shape {:id id + :type :rect + :name (str "rect-" id) + :x x :y y :width w :height h}) + (assoc :parent-id uuid/zero + :frame-id uuid/zero))) + +(defn- make-flex-frame + [id child-ids & {:keys [x y w h dir] + :or {x 0 y 0 w 200 h 200 dir :row}}] + (-> (cts/setup-shape {:id id + :type :frame + :name (str "flex-" id) + :layout :flex + :layout-flex-dir dir + :x x :y y :width w :height h}) + (assoc :parent-id uuid/zero + :frame-id uuid/zero + :shapes (vec child-ids)))) + +(defn- make-grid-frame + [id child-ids & {:keys [x y w h dir] + :or {x 0 y 0 w 200 h 200 dir :row}}] + (let [cell-id (uuid/next)] + (-> (cts/setup-shape {:id id + :type :frame + :name (str "grid-" id) + :layout :grid + :layout-grid-dir dir + :layout-grid-columns [{:type :flex :value 1}] + :layout-grid-rows [{:type :flex :value 1}] + :layout-grid-cells + {cell-id {:id cell-id + :row 1 + :row-span 1 + :column 1 + :column-span 1 + :shapes (vec child-ids)}} + :layout-padding-type :multiple + :layout-padding {:p1 0 :p2 0 :p3 0 :p4 0} + :layout-gap {:column-gap 0 :row-gap 0} + :x x :y y :width w :height h}) + (assoc :parent-id uuid/zero + :frame-id uuid/zero + :shapes (vec child-ids))))) + +(defn- make-objects + [shapes] + (let [shape-map (into {} (map (fn [s] [(:id s) s]) shapes))] + (reduce-kv (fn [m _id shape] + (if (contains? shape :shapes) + (reduce (fn [m' child-id] + (assoc-in m' [child-id :parent-id] (:id shape))) + m + (:shapes shape)) + m)) + shape-map + shape-map))) + +(defn- bounds-map-from-objects + "Build a bounds map from objects, optionally excluding some IDs." + [objects & {:keys [exclude-ids]}] + (let [full (gbm/objects->bounds-map objects)] + (if (seq exclude-ids) + (apply dissoc full exclude-ids) + full))) + +;; ---- Tests for flex layout bounds with nil bounds ---- + +(t/deftest layout-content-points-with-missing-parent-bounds + (t/testing "layout-content-points returns nil when parent is not in bounds map" + (let [child-id (uuid/next) + parent-id (uuid/next) + child (make-rect child-id 10 10 50 50) + parent (make-flex-frame parent-id [child-id]) + objects (make-objects [parent child]) + bounds (bounds-map-from-objects objects :exclude-ids #{parent-id})] + + (t/is (nil? (fb/layout-content-points bounds parent [child] objects)))))) + +(t/deftest layout-content-points-with-missing-child-bounds + (t/testing "layout-content-points skips children with missing bounds" + (let [child1-id (uuid/next) + child2-id (uuid/next) + parent-id (uuid/next) + child1 (make-rect child1-id 10 10 50 50) + child2 (make-rect child2-id 70 10 50 50) + parent (make-flex-frame parent-id [child1-id child2-id]) + objects (make-objects [parent child1 child2]) + bounds (bounds-map-from-objects objects :exclude-ids #{child1-id})] + + (let [result (fb/layout-content-points bounds parent [child1 child2] objects)] + (t/is (some? result)) + ;; Only child2's bounds should be in the result + (t/is (pos? (count result))))))) + +(t/deftest layout-content-bounds-with-missing-parent-bounds + (t/testing "layout-content-bounds returns nil when parent is not in bounds map" + (let [child-id (uuid/next) + parent-id (uuid/next) + child (make-rect child-id 10 10 50 50) + parent (make-flex-frame parent-id [child-id]) + objects (make-objects [parent child]) + bounds (bounds-map-from-objects objects :exclude-ids #{parent-id})] + + (t/is (nil? (fb/layout-content-bounds bounds parent [child] objects)))))) + +;; ---- Tests for grid layout bounds with nil bounds ---- + +(t/deftest grid-layout-content-points-with-missing-parent-bounds + (t/testing "grid layout-content-points returns nil when parent is not in bounds map" + (let [parent-id (uuid/next) + parent (make-grid-frame parent-id []) + objects (make-objects [parent]) + bounds (bounds-map-from-objects objects :exclude-ids #{parent-id}) + layout-data {:row-tracks [{:start-p (gpt/point 0 0) :size 100}] + :column-tracks [{:start-p (gpt/point 0 0) :size 100}]}] + + (t/is (nil? (gb/layout-content-points bounds parent layout-data)))))) + +(t/deftest grid-layout-content-bounds-with-missing-parent-bounds + (t/testing "grid layout-content-bounds returns nil when parent is not in bounds map" + (let [parent-id (uuid/next) + parent (make-grid-frame parent-id []) + objects (make-objects [parent]) + bounds (bounds-map-from-objects objects :exclude-ids #{parent-id}) + layout-data {:row-tracks [{:start-p (gpt/point 0 0) :size 100}] + :column-tracks [{:start-p (gpt/point 0 0) :size 100}]}] + + (t/is (nil? (gb/layout-content-bounds bounds parent layout-data)))))) + +;; ---- Tests for min-size-layout with nil bounds ---- + +(t/deftest child-min-width-grid-with-missing-child-bounds + (t/testing "child-min-width falls back when grid layout child bounds are missing" + (let [grandchild-id (uuid/next) + child-id (uuid/next) + grandchild (make-rect grandchild-id 0 0 30 30) + child (-> (make-grid-frame child-id [grandchild-id] :w 100 :h 100) + (assoc :layout-grid-dir :row + :layout-item-h-sizing :fill)) + objects (make-objects [child grandchild]) + ;; Exclude grandchild from bounds to simulate missing entry + bounds (bounds-map-from-objects objects :exclude-ids #{grandchild-id}) + child-bounds (grc/rect->points (grc/make-rect 0 0 100 100))] + + (let [result (msl/child-min-width child child-bounds bounds objects)] + (t/is (= (ctl/child-min-width child) result)))))) + +(t/deftest child-min-height-grid-with-missing-child-bounds + (t/testing "child-min-height falls back when grid layout child bounds are missing" + (let [grandchild-id (uuid/next) + child-id (uuid/next) + grandchild (make-rect grandchild-id 0 0 30 30) + child (-> (make-grid-frame child-id [grandchild-id] :w 100 :h 100) + (assoc :layout-grid-dir :column + :layout-item-v-sizing :fill)) + objects (make-objects [child grandchild]) + bounds (bounds-map-from-objects objects :exclude-ids #{grandchild-id}) + child-bounds (grc/rect->points (grc/make-rect 0 0 100 100))] + + (let [result (msl/child-min-height child child-bounds bounds objects)] + (t/is (= (ctl/child-min-height child) result)))))) + +(t/deftest child-min-width-grid-with-present-child-bounds + (t/testing "child-min-width handles bounded children in a fill-width grid" + (let [grandchild-id (uuid/next) + child-id (uuid/next) + grandchild (make-rect grandchild-id 0 0 30 30) + child (-> (make-grid-frame child-id [grandchild-id] :w 100 :h 100) + (assoc :layout-item-h-sizing :fill)) + objects (make-objects [child grandchild]) + bounds (bounds-map-from-objects objects) + child-bounds (grc/rect->points (grc/make-rect 0 0 100 100))] + + (t/is (number? (msl/child-min-width child child-bounds bounds objects)))))) + +(t/deftest child-min-height-grid-with-present-child-bounds + (t/testing "child-min-height handles bounded children in a fill-height grid" + (let [grandchild-id (uuid/next) + child-id (uuid/next) + grandchild (make-rect grandchild-id 0 0 30 30) + child (-> (make-grid-frame child-id [grandchild-id] :w 100 :h 100) + (assoc :layout-item-v-sizing :fill)) + objects (make-objects [child grandchild]) + bounds (bounds-map-from-objects objects) + child-bounds (grc/rect->points (grc/make-rect 0 0 100 100))] + + (t/is (number? (msl/child-min-height child child-bounds bounds objects)))))) diff --git a/common/test/common_tests/runner.cljc b/common/test/common_tests/runner.cljc index 7738c5dd9e..d3d2f7d48e 100644 --- a/common/test/common_tests/runner.cljc +++ b/common/test/common_tests/runner.cljc @@ -22,6 +22,7 @@ [common-tests.files.shapes-builder-test] [common-tests.files.validate-test] [common-tests.geom-align-test] + [common-tests.geom-bounds-layout-nil-test] [common-tests.geom-bounds-map-test] [common-tests.geom-flex-layout-test] [common-tests.geom-grid-layout-test] @@ -95,6 +96,7 @@ 'common-tests.files-migrations-test 'common-tests.files.validate-test 'common-tests.geom-align-test + 'common-tests.geom-bounds-layout-nil-test 'common-tests.geom-bounds-map-test 'common-tests.geom-flex-layout-test 'common-tests.geom-grid-layout-test