Andrey Antukh dc38eaa843
🐛 Fix font upload crash and preserve font metadata (#11951)
* 🐛 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.

A nil or blank variant is now treated as no match in
app.common.media, defaulting to weight 400 and style normal. The
variant parsing also accepts oblique as italic, and the new
parse-font-family helper derives the family from a filename by
stripping the known weight/style tokens, separating a style token
glued to a weight token ("Roboto-BoldItalic" -> "Roboto").
first-nonblank-string returns the first nonblank candidate, so a
blank preferredFamily/preferredSubfamily no longer shadows a valid
fontFamily/fontSubfamily.

app.main.data.fonts gains resolve-font-metadata and
prepare-font-entry. opentype.js values win per field; any missing
value falls back to the filename-derived family, weight and style.
The parsed and woff2 branches share the same helper, so metadata is
preserved instead of collapsing every face to weight 400, and
fonts with missing vertical-metric tables upload with no height
warning instead of failing.

process-upload reports per-file failures (unknown signature,
unparseable file, preparation error) with a single bad-font
notification instead of silently dropping them, and never errors
for per-file failures.

The upload now sends :variant-name to create-font-variant, which
accepts it as an optional param (ships in 2.20), so the foundry
name survives the upload.

Closes #10436

AI-assisted-by: deepseek-v4.1-flash
AI-assisted-by: muse-spark-1.3-contributor
Signed-off-by: CHOI JUNSOO <junsoo1172@gmail.com>

* 🐛 Parse glued font style tokens in the font parsers

EvaMarco's review on #11951: parse-font-family and
parse-font-weight handled glued tokens like "BoldItalic", but
parse-font-style did not. For names such as
"opensans-bolditalic-webfont.woff2" the three parsers disagreed:
the italic face resolved to style normal and collided with the
bold-only file on the (family, weight, style) join key. One
binary silently overwrote the other during upload, losing one
font face without any warning.

A new private normalize-font-name helper is applied by all three
parsers. It splits a style token glued to a weight token, now
with a lookahead boundary so "LightItalic_v2" splits too, and
strips a trailing technical suffix (-VF or _v<number>).
parse-font-family, parse-font-weight and parse-font-style now
interpret the name consistently.

Tests cover the three cases from the review, the bold-only
control case, and an end-to-end process-upload check that a bold
and a bold-italic WOFF2 survive as two variants.

Relates to #10436 and PR #11951.

AI-assisted-by: inkling-2-small-free

* Remove duplicate frontend test registration

---------

Signed-off-by: CHOI JUNSOO <junsoo1172@gmail.com>
2026-11-12 15:29:30 +01:00

376 lines
15 KiB
Clojure

;; 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.fonts-test
(:require
[app.common.uuid :as uuid]
[app.main.data.fonts :as df]
[app.main.fonts :as fonts]
[app.main.store :as st]
[app.util.globals :as globals]
[app.util.http :as http]
[app.util.webapi :as wa]
[beicon.v2.core :as rx]
[cljs.test :as t :include-macros true]
[frontend-tests.helpers.async :as async]
[frontend-tests.helpers.mock :as mock]
[potok.v2.core :as ptk]))
(def sample-font
{:id "sourcesanspro"
:name "Source Sans Pro"
:family "sourcesanspro"
:variants
[{:id "200"
:name "200"
:weight "200"
:style "normal"
:suffix "extralight"
:ttf-url "sourcesanspro-extralight.ttf"}
{:id "200italic"
:name "200 Italic"
:weight "200"
:style "italic"
:suffix "extralightitalic"
:ttf-url "sourcesanspro-extralightitalic.ttf"}
{:id "300"
:name "300"
:weight "300"
:style "normal"
:suffix "light"
:ttf-url "sourcesanspro-light.ttf"}
{:id "300italic"
:name "300 Italic"
:weight "300"
:style "italic"
:suffix "lightitalic"
:ttf-url "sourcesanspro-lightitalic.ttf"}
{:id "regular"
:name "400"
:weight "400"
:style "normal"
:ttf-url "sourcesanspro-regular.ttf"}
{:id "italic"
:name "400 Italic"
:weight "400"
:style "italic"
:ttf-url "sourcesanspro-italic.ttf"}
{:id "bold"
:name "700"
:weight "700"
:style "normal"
:ttf-url "sourcesanspro-bold.ttf"}
{:id "bolditalic"
:name "700 Italic"
:weight "700"
:style "italic"
:ttf-url "sourcesanspro-bolditalic.ttf"}
{:id "black"
:name "900"
:weight "900"
:style "normal"
:ttf-url "sourcesanspro-black.ttf"}
{:id "blackitalic"
:name "900 Italic"
:weight "900"
:style "italic"
:ttf-url "sourcesanspro-blackitalic.ttf"}]
:backend :builtin})
(t/deftest find-closest-weight-variant-test
(t/testing "finds exact weight match"
(let [result (fonts/find-closest-variant sample-font "400" nil)]
(t/is (= "400" (:weight result)))
(t/is (= "normal" (:style result)))))
(t/testing "finds exact weight match with style"
(let [result (fonts/find-closest-variant sample-font "400" "italic")]
(t/is (= "400" (:weight result)))
(t/is (= "italic" (:style result)))))
(t/testing "chooses higher weight when exactly between two weights"
(let [result (fonts/find-closest-variant sample-font "350" nil)]
(t/is (= "400" (:weight result)))))
(t/testing "finds exact weight match with style"
(let [result (fonts/find-closest-variant sample-font "350" "italic")]
(t/is (= "400" (:weight result)))
(t/is (= "italic" (:style result)))))
(t/testing "finds closest weight below minimum available"
(let [result (fonts/find-closest-variant sample-font "0" nil)]
(t/is (= "200" (:weight result)))))
(t/testing "finds closest weight above maximum available"
(let [result (fonts/find-closest-variant sample-font "1000" nil)]
(t/is (= "900" (:weight result)))))
(t/testing "keeps the closest weight match when style is not found"
(let [font {:id "sourcesanspro"
:name "Source Sans Pro"
:family "sourcesanspro"
:variants
[{:id "200italic"
:name "200 Italic"
:weight "200"
:style "italic"
:suffix "extralightitalic"
:ttf-url "sourcesanspro-extralightitalic.ttf"}
{:id "300"
:name "300"
:weight "300"
:style "normal"
:suffix "light"
:ttf-url "sourcesanspro-light.ttf"}
{:id "300italic"
:name "300 Italic"
:weight "300"
:style "italic"
:suffix "lightitalic"
:ttf-url "sourcesanspro-lightitalic.ttf"}]}
result (fonts/find-closest-variant font "200" nil)]
(t/is (= "200" (:weight result)))
(t/is (= "italic" (:style result))))))
;; --- preview sprite ----------------------------------------------------------
;;
;; The sprite feature (FLAG :font-preview) caches a pre-parsed SVG node shared by
;; every open font dropdown. `:refs` counts the open dropdowns so the node is only
;; detached when the last one closes. The unit test runner has no browser DOM, so
;; the environment boundary (`globals/browser?`) is mocked and DOM nodes are
;; replaced with minimal fakes exposing only what attach/detach touches.
(t/use-fixtures
:each
{:before #(reset! fonts/preview-sprite {:status :idle :ids #{} :node nil :refs 0})})
(defn- fake-node
"A minimal DOM-like node exposing only what the sprite attach/detach touches."
[]
#js {:remove (fn [] nil)})
(t/deftest ^:async attach-preview-sprite-returns-nil-while-sprite-is-not-ready
(await
(mock/with-mocks*
{globals/browser? (mock/stub (constantly true))}
(reset! fonts/preview-sprite {:status :loading :ids #{} :node nil :refs 0})
(t/is (nil? (fonts/attach-preview-sprite!)))
(t/is (= 0 (:refs @fonts/preview-sprite)))
(reset! fonts/preview-sprite {:status :error :ids #{} :node nil :refs 0})
(t/is (nil? (fonts/attach-preview-sprite!)))
(t/is (= 0 (:refs @fonts/preview-sprite)))
;; Trailing settle: the body is synchronous, but `with-mocks*`
;; evaluates to a promise, so the body must settle one.
(await (async/settle)))))
(t/deftest ^:async attach-preview-sprite-increments-refs-and-returns-the-node
(await
(mock/with-mocks*
{globals/browser? (mock/stub (constantly true))}
(let [node (fake-node)]
(reset! fonts/preview-sprite {:status :ready :ids #{"a"} :node node :refs 0})
(t/is (identical? node (fonts/attach-preview-sprite!)))
(t/is (= 1 (:refs @fonts/preview-sprite)))
(t/is (identical? node (fonts/attach-preview-sprite!)))
(t/is (= 2 (:refs @fonts/preview-sprite)))
;; Trailing settle: the body is synchronous, but `with-mocks*`
;; evaluates to a promise, so the body must settle one.
(await (async/settle))))))
(t/deftest ^:async detach-preview-sprite-removes-node-only-when-last-reference-drops
(await
(mock/with-mocks*
{globals/browser? (mock/stub (constantly true))}
(let [removed? (volatile! false)
node #js {:remove (fn [] (vreset! removed? true))}]
(reset! fonts/preview-sprite {:status :ready :ids #{"a"} :node node :refs 0})
(fonts/attach-preview-sprite!)
(fonts/attach-preview-sprite!)
;; First detach keeps the node: another dropdown is still open.
(fonts/detach-preview-sprite! node)
(t/is (= 1 (:refs @fonts/preview-sprite)))
(t/is (false? @removed?))
;; Second detach reaches zero refs, so the node is removed from the DOM.
(fonts/detach-preview-sprite! node)
(t/is (= 0 (:refs @fonts/preview-sprite)))
(t/is (true? @removed?))
;; Trailing settle: the body is synchronous, but `with-mocks*`
;; evaluates to a promise, so the body must settle one.
(await (async/settle))))))
(t/deftest ^:async detach-preview-sprite-clamps-refs-at-zero
(await
(mock/with-mocks*
{globals/browser? (mock/stub (constantly true))}
(let [removed? (volatile! false)
node #js {:remove (fn [] (vreset! removed? true))}]
(reset! fonts/preview-sprite {:status :ready :ids #{"a"} :node node :refs 0})
(fonts/detach-preview-sprite! node)
(t/is (= 0 (:refs @fonts/preview-sprite)))
(t/is (true? @removed?))
;; Trailing settle: the body is synchronous, but `with-mocks*`
;; evaluates to a promise, so the body must settle one.
(await (async/settle))))))
(t/deftest ^:async prefetch-preview-sprite-fetches-only-from-idle-or-error
(let [calls (volatile! 0)
fetch (mock/stub (fn [& _]
(vswap! calls inc)
(rx/empty)))]
(await
(mock/with-mocks*
{globals/browser? (mock/stub (constantly true))
http/fetch fetch}
;; :ready → no refetch
(reset! fonts/preview-sprite {:status :ready :ids #{"a"} :node (fake-node) :refs 0})
(fonts/prefetch-preview-sprite!)
(t/is (= 0 @calls))
;; :loading → no refetch (an earlier request is in flight)
(reset! fonts/preview-sprite {:status :loading :ids #{} :node nil :refs 0})
(fonts/prefetch-preview-sprite!)
(t/is (= 0 @calls))
;; :error → retries
(reset! fonts/preview-sprite {:status :error :ids #{} :node nil :refs 0})
(fonts/prefetch-preview-sprite!)
(t/is (= 1 @calls))
;; :idle → first fetch
(reset! fonts/preview-sprite {:status :idle :ids #{} :node nil :refs 0})
(fonts/prefetch-preview-sprite!)
(t/is (= 2 @calls))
;; Trailing settle: the body is synchronous, but `with-mocks*`
;; evaluates to a promise, so the body must settle one.
(await (async/settle))))))
;; --- custom font upload ------------------------------------------------------
;;
;; Regression tests for https://github.com/penpot/penpot/issues/12304:
;; uploading a font whose opentype.js names are missing (nil subfamily,
;; as seen with some Fontshare families) crashed `process-upload` with
;; `re-seq must match against a string` and left the user with no
;; feedback. Unknown signatures and unparseable files were silently
;; dropped as well.
(defn- bytes->buffer
[bytes]
(.-buffer (js/Uint8Array. (clj->js bytes))))
(defn- fake-ot-font
[{:keys [family variant hhea os2]}]
(let [names {"preferredFamily" family
"fontFamily" family
"preferredSubfamily" variant
"fontSubfamily" variant}]
#js {:getEnglishName (fn [k] (get names k))
:tables #js {:hhea hhea :os2 os2}}))
(t/deftest parse-font-mtype-detects-font-signatures
(t/is (= "font/ttf" (#'df/parse-font-mtype (bytes->buffer [0 1 0 0]))))
(t/is (= "font/otf" (#'df/parse-font-mtype (bytes->buffer [79 84 84 79]))))
(t/is (= "font/woff" (#'df/parse-font-mtype (bytes->buffer [119 79 70 70]))))
(t/is (= "font/woff2" (#'df/parse-font-mtype (bytes->buffer [119 79 70 50]))))
(t/is (nil? (#'df/parse-font-mtype (bytes->buffer [1 2 3 4]))))
(t/is (nil? (#'df/parse-font-mtype nil)))
(t/is (nil? (#'df/parse-font-mtype (bytes->buffer [0 1])))))
(t/deftest prepare-font-entry-defaults-missing-subfamily
(let [font (fake-ot-font {:family "Satoshi"
:variant nil
:hhea #js {:ascender 1000 :descender 300}
:os2 #js {:usWinAscent 1000 :usWinDescent 300
:sTypoAscender 800 :sTypoDescender 200
:fsSelection 0}})
entry (df/prepare-font-entry {:font font
:type "font/otf"
:name "Satoshi-Regular.otf"
:data (bytes->buffer [79 84 84 79 0 0 0 0])})]
(t/is (= "Satoshi" (:font-family entry)))
(t/is (= 400 (:font-weight entry)))
(t/is (= "normal" (:font-style entry)))
(t/is (false? (:height-warning? entry)))))
(t/deftest prepare-font-entry-tolerates-missing-tables
(let [font (fake-ot-font {:family "Satoshi" :variant "Regular" :hhea nil :os2 nil})
entry (df/prepare-font-entry {:font font
:type "font/ttf"
:name "Satoshi-Regular.ttf"
:data (bytes->buffer [0 1 0 0 0 0 0 0])})]
(t/is (= "Satoshi" (:font-family entry)))
(t/is (false? (:height-warning? entry)))))
(t/deftest parse-font-returns-error-on-unparseable
(t/testing "read errors pass through"
(t/is (= {:error "x.ttf"} (#'df/parse-font {:error "x.ttf"}))))
(t/testing "woff2 keeps a nil font for filename-based metadata"
(let [result (#'df/parse-font {:data (bytes->buffer [119 79 70 50])
:type "font/woff2"
:name "Satoshi-Bold.woff2"})]
(t/is (nil? (:font result)))
(t/is (nil? (:error result)))))
(t/testing "opentype.js failures become errors instead of silent drops"
(let [garbage (bytes->buffer (into [0 1 0 0] (repeat 96 0)))
result (#'df/parse-font {:data garbage :type "font/ttf" :name "bad.ttf"})]
(t/is (= "bad.ttf" (:error result))))))
(t/deftest ^:async process-upload-reports-unknown-signature
(let [team-id (uuid/next)
blob #js {:name "mystery.bin"}
emitted (atom [])
result (atom nil)]
(await (mock/with-mocks*
{wa/read-file-as-array-buffer (mock/stub (fn [_] (rx/of (bytes->buffer [1 2 3 4]))))
st/emit! (mock/stub (fn [& evts] (swap! emitted into evts) nil))}
(await (async/observe (df/process-upload [blob] team-id)
:on-next #(reset! result %)))))
(t/is (= {} @result))
(t/is (= 1 (count @emitted)))
(t/is (= :app.main.data.notifications/show (ptk/type (first @emitted))))))
(t/deftest ^:async process-upload-woff2-succeeds-from-filename
(let [team-id (uuid/next)
blob #js {:name "Satoshi-Bold.woff2"}
emitted (atom [])
result (atom nil)]
(await (mock/with-mocks*
{wa/read-file-as-array-buffer (mock/stub (fn [_] (rx/of (bytes->buffer [119 79 70 50 0 0 0 0]))))
st/emit! (mock/stub (fn [& evts] (swap! emitted into evts) nil))}
(await (async/observe (df/process-upload [blob] team-id)
:on-next #(reset! result %)))
(await (async/settle))))
(t/is (= 1 (count (vals @result))))
(let [entry (first (vals @result))]
(t/is (= "Satoshi" (:font-family entry)))
(t/is (= 700 (:font-weight entry)))
(t/is (= "normal" (:font-style entry))))
(t/is (empty? @emitted))))
(t/deftest ^:async process-upload-keeps-bold-and-bolditalic-woff2
;; Regression for the review on #11951: the bold-italic file resolved to
;; style "normal", collided with the bold-only file on the (family,
;; weight, style) join key and silently overwrote it.
(let [team-id (uuid/next)
blobs [#js {:name "Inter-Bold.woff2"}
#js {:name "Inter-BoldItalic.woff2"}]
emitted (atom [])
result (atom nil)]
(await (mock/with-mocks*
{wa/read-file-as-array-buffer
(mock/stub (fn [_] (rx/of (bytes->buffer [119 79 70 50 0 0 0 0]))))
st/emit! (mock/stub (fn [& evts] (swap! emitted into evts) nil))}
(await (async/observe (df/process-upload blobs team-id)
:on-next #(reset! result %)))
(await (async/settle))))
(t/is (= 2 (count (vals @result))))
(t/is (every? #(= "Inter" (:font-family %)) (vals @result)))
(t/is (every? #(= 700 (:font-weight %)) (vals @result)))
(t/is (= #{"normal" "italic"} (set (map :font-style (vals @result)))))
(t/is (empty? @emitted))))