From 6999b7e501bb674c18c613d4de745ff7f93901d3 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Fri, 25 Sep 2026 17:44:32 +0000 Subject: [PATCH] :bug: Fix font upload crash and preserve variant metadata opentype.js 2.0.0 returns no fontSubfamily name for fonts whose Macintosh name table only holds custom records. That made parse-font-weight/parse-font-style call re-seq on nil and crash the upload. Treat a nil or blank variant as no match in app.common.media, and add resolve-font-metadata to app.main.data.fonts so the parsed branch falls back to the filename-derived family, weight and style. The woff2 branch now shares the same helper, so metadata is preserved instead of collapsing every face to weight 400. Closes #10436 AI-assisted-by: deepseek-v4.1-flash --- common/src/app/common/media.cljc | 34 +++++---- common/test/common_tests/media_test.cljc | 14 +++- frontend/src/app/main/data/fonts.cljs | 73 ++++++++++++------- .../test/frontend_tests/data/fonts_test.cljs | 66 +++++++++++++++++ frontend/test/frontend_tests/runner.cljs | 2 + 5 files changed, 146 insertions(+), 43 deletions(-) create mode 100644 frontend/test/frontend_tests/data/fonts_test.cljs diff --git a/common/src/app/common/media.cljc b/common/src/app/common/media.cljc index 8e6038ea11..63615e3309 100644 --- a/common/src/app/common/media.cljc +++ b/common/src/app/common/media.cljc @@ -83,25 +83,27 @@ (defn parse-font-weight [variant] - (cond - (re-seq #"(?i)(?:^|[-_\s])(hairline|thin)(?=(?:[-_\s]|$|italic\b))" variant) 100 - (re-seq #"(?i)(?:^|[-_\s])(extra\s*light|ultra\s*light)(?=(?:[-_\s]|$|italic\b))" variant) 200 - (re-seq #"(?i)(?:^|[-_\s])(light)(?=(?:[-_\s]|$|italic\b))" variant) 300 - (re-seq #"(?i)(?:^|[-_\s])(normal|regular)(?=(?:[-_\s]|$|italic\b))" variant) 400 - (re-seq #"(?i)(?:^|[-_\s])(medium)(?=(?:[-_\s]|$|italic\b))" variant) 500 - (re-seq #"(?i)(?:^|[-_\s])(semi\s*bold|demi\s*bold)(?=(?:[-_\s]|$|italic\b))" variant) 600 - (re-seq #"(?i)(?:^|[-_\s])(extra\s*bold|ultra\s*bold)(?=(?:[-_\s]|$|italic\b))" variant) 800 - (re-seq #"(?i)(?:^|[-_\s])(bold)(?=(?:[-_\s]|$|italic\b))" variant) 700 - (re-seq #"(?i)(?:^|[-_\s])(extra\s*black|ultra\s*black)(?=(?:[-_\s]|$|italic\b))" variant) 950 - (re-seq #"(?i)(?:^|[-_\s])(black|heavy|solid)(?=(?:[-_\s]|$|italic\b))" variant) 900 - :else 400)) + (let [variant (or variant "")] + (cond + (re-seq #"(?i)(?:^|[-_\s])(hairline|thin)(?=(?:[-_\s]|$|italic\b))" variant) 100 + (re-seq #"(?i)(?:^|[-_\s])(extra\s*light|ultra\s*light)(?=(?:[-_\s]|$|italic\b))" variant) 200 + (re-seq #"(?i)(?:^|[-_\s])(light)(?=(?:[-_\s]|$|italic\b))" variant) 300 + (re-seq #"(?i)(?:^|[-_\s])(normal|regular)(?=(?:[-_\s]|$|italic\b))" variant) 400 + (re-seq #"(?i)(?:^|[-_\s])(medium)(?=(?:[-_\s]|$|italic\b))" variant) 500 + (re-seq #"(?i)(?:^|[-_\s])(semi\s*bold|demi\s*bold)(?=(?:[-_\s]|$|italic\b))" variant) 600 + (re-seq #"(?i)(?:^|[-_\s])(extra\s*bold|ultra\s*bold)(?=(?:[-_\s]|$|italic\b))" variant) 800 + (re-seq #"(?i)(?:^|[-_\s])(bold)(?=(?:[-_\s]|$|italic\b))" variant) 700 + (re-seq #"(?i)(?:^|[-_\s])(extra\s*black|ultra\s*black)(?=(?:[-_\s]|$|italic\b))" variant) 950 + (re-seq #"(?i)(?:^|[-_\s])(black|heavy|solid)(?=(?:[-_\s]|$|italic\b))" variant) 900 + :else 400))) (defn parse-font-style [variant] - (if (or (re-seq #"(?i)(?:^|[-_\s])(italic)(?:[-_\s]|$)" variant) - (re-seq #"(?i)italic$" variant)) - "italic" - "normal")) + (let [variant (or variant "")] + (if (or (re-seq #"(?i)(?:^|[-_\s])(italic)(?:[-_\s]|$)" variant) + (re-seq #"(?i)italic$" variant)) + "italic" + "normal"))) (defn font-weight->name [weight] diff --git a/common/test/common_tests/media_test.cljc b/common/test/common_tests/media_test.cljc index 24302a02d1..7cea9644e1 100644 --- a/common/test/common_tests/media_test.cljc +++ b/common/test/common_tests/media_test.cljc @@ -28,7 +28,12 @@ (t/is (= 400 (media/parse-font-weight "Lighthaus"))) (t/is (= 400 (media/parse-font-weight "Blackwood"))) (t/is (= 400 (media/parse-font-weight "Thinker"))) - (t/is (= 400 (media/parse-font-weight "Mediaeval"))))) + (t/is (= 400 (media/parse-font-weight "Mediaeval")))) + + (t/testing "handles nil and blank variants safely" + (t/is (= 400 (media/parse-font-weight nil))) + (t/is (= 400 (media/parse-font-weight ""))) + (t/is (= 400 (media/parse-font-weight " "))))) (t/deftest test-parse-font-style (t/testing "matches italic with proper boundaries" @@ -40,7 +45,12 @@ (t/testing "does not match italic embedded in words" (t/is (= "normal" (media/parse-font-style "Italica"))) - (t/is (= "normal" (media/parse-font-style "Roboto-Regular"))))) + (t/is (= "normal" (media/parse-font-style "Roboto-Regular")))) + + (t/testing "handles nil and blank variants safely" + (t/is (= "normal" (media/parse-font-style nil))) + (t/is (= "normal" (media/parse-font-style ""))) + (t/is (= "normal" (media/parse-font-style " "))))) (t/deftest test-strip-image-extension (t/testing "removes extension from supported image files" diff --git a/frontend/src/app/main/data/fonts.cljs b/frontend/src/app/main/data/fonts.cljs index 2ec056487d..fc6cd2d0e0 100644 --- a/frontend/src/app/main/data/fonts.cljs +++ b/frontend/src/app/main/data/fonts.cljs @@ -115,6 +115,40 @@ :font-style font-style :uploads uploads}))))) +(def ^:private font-variant-tokens-re + ;; Strip known weight/style tokens and separators to derive the family name. + ;; Use word boundaries to avoid matching substrings (e.g. "Boldini" must not + ;; match "bold"). + #"(?i)(^|[-_\s])(extra\s*black|ultra\s*black|extra\s*bold|ultra\s*bold|semi\s*bold|demi\s*bold|extra\s*light|ultra\s*light|hairline|thin|light|normal|regular|medium|bold|black|heavy|solid|italic)([-_\s]|$)") + +(defn- font-family-from-name + "Derive a font family name from a font filename, stripping the known + weight/style tokens and separators." + [name] + (let [base-name (str/replace name #"\.[^.]+$" "") + raw-family-name (-> base-name + (str/replace font-variant-tokens-re "$1$3") + (str/replace #"[-_\s]+" " ") + (str/trim))] + (if (str/blank? raw-family-name) base-name raw-family-name))) + +(defn resolve-font-metadata + "Resolve the upload metadata for a font file. + + `names` holds the family and variant names read by opentype.js; each + may be nil when the font has no usable name records. `filename` is the + original file name. The opentype.js values win per field; any missing + value is derived from the filename instead." + [{:keys [family variant]} filename] + (let [base-name (str/replace filename #"\.[^.]+$" "") + variant (not-empty (str/trim (or variant ""))) + family (not-empty (str/trim (or family ""))) + variant-source (or variant base-name)] + {:font-family (or family (font-family-from-name filename)) + :font-weight (cm/parse-font-weight variant-source) + :font-style (cm/parse-font-style variant-source) + :variant-name variant})) + (defn process-upload "Given a seq of blobs and the team id, creates a ready-to-use fonts map with temporal ID's associated to each font entry. @@ -159,32 +193,21 @@ (and f-selection (or (not= hhea-ascender os2-ascent) (not= hhea-descender os2-descent)))) - data (js/Uint8Array. data)] - {:content {:data data - :name name - :type type} - :font-family (or family "") - :font-weight (cm/parse-font-weight variant) - :font-style (cm/parse-font-style variant) - :variant-name variant - :height-warning? height-warning?}) + data (js/Uint8Array. data) + metadata (resolve-font-metadata {:family family :variant variant} name)] + (assoc metadata + :content {:data data + :name name + :type type} + :height-warning? height-warning?)) ;; Font could not be parsed (woff2), extract metadata from filename - (let [base-name (str/replace name #"\.[^.]+$" "") - ;; Strip known weight/style tokens and separators to derive family name - ;; Use word boundaries to avoid matching substrings (e.g. "Boldini" should not match "bold") - raw-family-name (-> base-name - (str/replace #"(?i)(^|[-_\s])(extra\s*black|ultra\s*black|extra\s*bold|ultra\s*bold|semi\s*bold|demi\s*bold|extra\s*light|ultra\s*light|hairline|thin|light|normal|regular|medium|bold|black|heavy|solid|italic)([-_\s]|$)" "$1$3") - (str/replace #"[-_\s]+" " ") - (str/trim)) - family-name (if (str/blank? raw-family-name) base-name raw-family-name) - data (js/Uint8Array. data)] - {:content {:data data - :name name - :type type} - :font-family family-name - :font-weight (cm/parse-font-weight base-name) - :font-style (cm/parse-font-style base-name) - :height-warning? false}))) + (let [data (js/Uint8Array. data) + metadata (resolve-font-metadata {} name)] + (assoc metadata + :content {:data data + :name name + :type type} + :height-warning? false)))) (join [res {:keys [content] :as font}] (let [key-fn (juxt :font-family :font-weight :font-style) diff --git a/frontend/test/frontend_tests/data/fonts_test.cljs b/frontend/test/frontend_tests/data/fonts_test.cljs new file mode 100644 index 0000000000..29bc2b59d4 --- /dev/null +++ b/frontend/test/frontend_tests/data/fonts_test.cljs @@ -0,0 +1,66 @@ +;; 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.fonts-test + "Tests for the upload metadata resolution in app.main.data.fonts." + (:require + [app.main.data.fonts :as df] + [cljs.test :as t :include-macros true])) + +(t/deftest resolve-font-metadata-prefers-opentype-values + (t/testing "opentype.js values win over the filename" + (t/is (= {:font-family "Inter" + :font-weight 700 + :font-style "normal" + :variant-name "Bold"} + (df/resolve-font-metadata {:family "Inter" :variant "Bold"} + "Inter.ttf"))))) + +(t/deftest resolve-font-metadata-falls-back-to-filename + (t/testing "derives weight and style from the filename when the variant is missing" + (t/is (= {:font-family "Pretendard" + :font-weight 600 + :font-style "normal" + :variant-name nil} + (df/resolve-font-metadata {:family "Pretendard" :variant nil} + "Pretendard-SemiBold.otf")))) + + (t/testing "derives weight and style from the filename when the variant is blank" + (t/is (= {:font-family "Lato" + :font-weight 300 + :font-style "normal" + :variant-name nil} + (df/resolve-font-metadata {:family "Lato" :variant ""} + "Lato-Light.ttf")))) + + (t/testing "derives the family from the filename when it is missing" + (t/is (= {:font-family "Blender Inter" + :font-weight 700 + :font-style "normal" + :variant-name nil} + (df/resolve-font-metadata {:family nil :variant nil} + "Blender-Inter-Bold.ttf")))) + + (t/testing "derives family, weight and style from the filename when nothing is available" + (t/is (= {:font-family "Pretendard" + :font-weight 600 + :font-style "normal" + :variant-name nil} + (df/resolve-font-metadata {} "Pretendard-SemiBold.woff2")))) + + (t/testing "derives italic from the filename" + (t/is (= {:font-family "Roboto" + :font-weight 400 + :font-style "italic" + :variant-name nil} + (df/resolve-font-metadata {} "Roboto-Italic.ttf")))) + + (t/testing "handles filenames without extension" + (t/is (= {:font-family "Lato" + :font-weight 300 + :font-style "normal" + :variant-name nil} + (df/resolve-font-metadata {} "Lato-Light"))))) diff --git a/frontend/test/frontend_tests/runner.cljs b/frontend/test/frontend_tests/runner.cljs index 7e718bbf5d..651e5f24eb 100644 --- a/frontend/test/frontend_tests/runner.cljs +++ b/frontend/test/frontend_tests/runner.cljs @@ -12,6 +12,7 @@ [frontend-tests.data.dashboard-test] [frontend-tests.data.event-test] [frontend-tests.data.exports-assets-test] + [frontend-tests.data.fonts-test] [frontend-tests.data.nitrate-test] [frontend-tests.data.persistence-retry-test] [frontend-tests.data.persistence-test] @@ -151,6 +152,7 @@ 'frontend-tests.data.repo-test 'frontend-tests.data.store-test 'frontend-tests.data.exports-assets-test + 'frontend-tests.data.fonts-test 'frontend-tests.data.svg-upload-test 'frontend-tests.data.uploads-test 'frontend-tests.data.viewer-test