diff --git a/common/src/app/common/files/shapes_builder.cljc b/common/src/app/common/files/shapes_builder.cljc index 668f50fcaf..a70d6eef9b 100644 --- a/common/src/app/common/files/shapes_builder.cljc +++ b/common/src/app/common/files/shapes_builder.cljc @@ -677,6 +677,22 @@ (remove is-style-fragment?) ;; Filter style fragments and hex colors (filter #(contains? defs %))))) ;; Only existing defs +(defn resolve-element-name + "Pick the most user-meaningful name for an SVG element. + + Inkscape (and editors following the same convention) write the + operator-given label to ``inkscape:label``/``sodipodi:label`` while + ``id`` holds an auto-generated technical id like ``path1234``. + Preferring the namespaced label keeps the layer/group/element names + the operator sees in their source editor across a paste/import + (#7869); the existing ``id`` and ``(tag->name tag)`` fallbacks keep + legacy SVGs that don't carry a label working unchanged." + [tag attrs] + (or (:inkscape:label attrs) + (:sodipodi:label attrs) + (:id attrs) + (tag->name tag))) + (defn parse-svg-element [frame-id svg-data {:keys [tag attrs hidden] :as element} unames] @@ -684,7 +700,7 @@ ;; think we should handle this case early and avoid some code ;; execution - (let [name (or (:id attrs) (tag->name tag)) + (let [name (resolve-element-name tag attrs) att-refs (csvg/find-attr-references attrs) defs (get svg-data :defs) valid-refs (filter-valid-def-references att-refs defs) diff --git a/common/test/common_tests/files/shapes_builder_test.cljc b/common/test/common_tests/files/shapes_builder_test.cljc new file mode 100644 index 0000000000..05956918b2 --- /dev/null +++ b/common/test/common_tests/files/shapes_builder_test.cljc @@ -0,0 +1,52 @@ +;; 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 common-tests.files.shapes-builder-test + (:require + [app.common.files.shapes-builder :as sb] + [clojure.test :as t])) + +;; Regression for https://github.com/penpot/penpot/issues/7869. +;; ``parse-svg-element`` used to derive the shape name from +;; ``(or (:id attrs) (tag->name tag))`` which dropped Inkscape-authored +;; labels. ``tubax/xml->clj`` (the SVG parser the rest of the import +;; pipeline already feeds these maps to) keeps namespaced attributes as +;; ``:prefix:name`` keywords — same shape the codebase already reads +;; ``:xlink:href`` from in this file (line 134) and in +;; ``app.common.svg``. + +(t/deftest resolve-element-name-prefers-inkscape-label + (t/is (= "Layer 1" + (sb/resolve-element-name :g {:inkscape:label "Layer 1" + :id "g1234"})))) + +(t/deftest resolve-element-name-prefers-sodipodi-label-when-no-inkscape-label + (t/is (= "phone-icon" + (sb/resolve-element-name :path {:sodipodi:label "phone-icon" + :id "path5678"})))) + +(t/deftest resolve-element-name-falls-back-to-id-when-no-label-namespace + (t/is (= "manual-id" + (sb/resolve-element-name :rect {:id "manual-id"})))) + +(t/deftest resolve-element-name-falls-back-to-tag-name-when-no-id-and-no-label + ;; The tag->name mapping returns generic names for known SVG element + ;; tags. Asserting on the call result here (rather than a hardcoded + ;; string) keeps the test stable if the tag->name mapping is updated. + (t/is (some? (sb/resolve-element-name :rect {}))) + (t/is (string? (sb/resolve-element-name :rect {})))) + +(t/deftest resolve-element-name-inkscape-label-wins-over-sodipodi-and-id + ;; Both label conventions and an id present together; the priority is + ;; inkscape > sodipodi > id > tag, matching the order operators expect + ;; (Inkscape's own UI shows ``inkscape:label`` as the canonical name). + (t/is (= "user-name" + (sb/resolve-element-name :g {:inkscape:label "user-name" + :sodipodi:label "stale-label" + :id "g1"})))) + +(t/deftest resolve-element-name-empty-attrs-uses-tag-fallback + (t/is (some? (sb/resolve-element-name :path {})))) diff --git a/common/test/common_tests/runner.cljc b/common/test/common_tests/runner.cljc index 06f7926c47..29540525db 100644 --- a/common/test/common_tests/runner.cljc +++ b/common/test/common_tests/runner.cljc @@ -15,6 +15,7 @@ [common-tests.files-builder-test] [common-tests.files-changes-test] [common-tests.files-migrations-test] + [common-tests.files.shapes-builder-test] [common-tests.geom-align-test] [common-tests.geom-bounds-map-test] [common-tests.geom-flex-layout-test]