From 86c563f11f40ac01260a8977a0f9069068303aca Mon Sep 17 00:00:00 2001 From: Luis de Dios Date: Mon, 10 Aug 2026 11:35:51 +0200 Subject: [PATCH] :bug: Fix font family typography asset persist across files in new created text layers (#11134) --- ...workspace-texts-typography-persist.spec.js | 110 ++++++++++++++++++ frontend/src/app/main/data/workspace.cljs | 5 +- .../src/app/main/data/workspace/texts.cljs | 3 +- .../data/workspace_texts_test.cljs | 57 +++++++++ 4 files changed, 172 insertions(+), 3 deletions(-) create mode 100644 frontend/playwright/ui/specs/workspace-texts-typography-persist.spec.js diff --git a/frontend/playwright/ui/specs/workspace-texts-typography-persist.spec.js b/frontend/playwright/ui/specs/workspace-texts-typography-persist.spec.js new file mode 100644 index 0000000000..d9b6c98c02 --- /dev/null +++ b/frontend/playwright/ui/specs/workspace-texts-typography-persist.spec.js @@ -0,0 +1,110 @@ +import { test, expect } from "@playwright/test"; +import { readFile } from "node:fs/promises"; +import { WasmWorkspacePage } from "../pages/WasmWorkspacePage"; + +// --------------------------------------------------------------------------- +// BUG 10925 - Font family typography asset must not persist across files in +// newly created text layers. +// +// `save-font` writes the current font (plus the typography refs of the edited +// shape, when it uses one) into the session-global `:workspace-global +// :default-font`. That state is what seeds the content of brand-new text +// shapes via `v2-default-text-content`. Because it is session-global it +// survives a file switch, so a text created in file B could end up referencing +// a typography asset that only exists in file A (see workspace/texts.cljs +// save-font and workspace.cljs initialize/finalize-workspace). +// +// This E2E reproduces the leak faithfully in a single SPA session: +// 1. Open file A (has a text shape linked to a typography asset). +// 2. Change a font attribute on that shape (triggers `emit-update!` -> +// `save-font` with the current text-node attrs, typography refs included). +// 3. Switch to file B (same session, fragment navigation keeps JS state). +// 4. Create a brand-new text layer in file B. +// 5. Assert the new text uses the DEFAULT Penpot font ("Source Sans Pro"), +// not the typography font-family carried over from file A. +// --------------------------------------------------------------------------- + +const FILE_A = { + id: "1062e0a0-8fe0-80ae-8007-e70b4993f5ef", + pageId: "1062e0a0-8fe0-80ae-8007-e70b4993f5f0", + // "Text with typography asset one" carries a ref to in-file typography whose + // font-family is "IM Fell French Canon SC" (multiselection-typography.json). +}; + +const FILE_B = { + id: "434b0541-fa2f-802f-8006-59827d964a9b", + pageId: "434b0541-fa2f-802f-8006-59827d964a9c", + // render-wasm/get-file-text-custom-fonts.json - a mostly empty file whose + // only text uses the default font (no typography asset). +}; + +async function serveTwoFiles(page) { + const fileABody = await readFile( + "playwright/data/workspace/multiselection-typography.json", + "utf-8", + ); + const fileBBody = await readFile( + "playwright/data/render-wasm/get-file-text-custom-fonts.json", + "utf-8", + ); + + // Dispatch on the `id` query param of the `get-file` RPC so each file gets + // its own fixture while keeping a single SPA session alive. + await page.route(/get\-file\?/, (route) => { + const url = new URL(route.request().url()); + const fileId = url.searchParams.get("id"); + const body = fileId === FILE_A.id ? fileABody : fileBBody; + return route.fulfill({ + status: 200, + contentType: "application/transit+json", + body, + }); + }); +} + +test.beforeEach(async ({ page }) => { + await WasmWorkspacePage.init(page); + // WASM_FLAGS already enables the v2 text editor / render-wasm. Add the WASM + // text editor on top so typography styles are read through the current text + // values path. + await WasmWorkspacePage.mockConfigFlags(page, ["enable-feature-text-editor-wasm"]); +}); + +test("BUG 10925 - typography font does not leak into new text in a different file", async ({ page }) => { + const workspace = new WasmWorkspacePage(page, { textEditor: true }); + await workspace.setupEmptyFile(); + await workspace.mockRPC( + "get-font-variants?team-id=*", + "render-wasm/get-font-variants-custom-fonts.json", + ); + + await serveTwoFiles(page); + + // ---- File A: select the text linked to a typography and change a font ---- + await workspace.goToWorkspace({ fileId: FILE_A.id, pageId: FILE_A.pageId }); + await workspace.waitForFirstRender(); + await workspace.doubleClickLeafLayer("Text with typography asset one"); + await workspace.textEditor.startEditing(); + + // Changing a font attribute triggers save-font with the current text-node + // attrs (including the typography refs) storing them into default-font. + await workspace.textEditor.changeFontSize(24); + await workspace.textEditor.stopEditing(); + + // ---- File B: same SPA session, switch to a file with no typography ---- + await workspace.goToWorkspace({ fileId: FILE_B.id, pageId: FILE_B.pageId }); + await workspace.waitForFirstRender(); + + // Create a brand-new text layer in file B and query its font-family. + await workspace.createTextShape(100, 100, 300, 200, "hello"); + await workspace.textEditor.stopEditing(); + await workspace.clickLeafLayer("hello"); + await workspace.textEditor.startEditing(); + await workspace.page.keyboard.press("ControlOrMeta+a"); + + const fontFamily = workspace.rightSidebar.getByTitle("Font Family"); + await expect(fontFamily).toContainText("Source Sans Pro"); + // The custom typography family from file A (IM Fell French Canon SC) must NOT + // be carried over. + await expect(fontFamily).not.toContainText("IM Fell"); +}); \ No newline at end of file diff --git a/frontend/src/app/main/data/workspace.cljs b/frontend/src/app/main/data/workspace.cljs index cedfad1d96..9bf11fb701 100644 --- a/frontend/src/app/main/data/workspace.cljs +++ b/frontend/src/app/main/data/workspace.cljs @@ -346,7 +346,8 @@ (assoc :recent-colors (:recent-colors storage/user)) (assoc :recent-fonts (:recent-fonts storage/user)) (assoc :current-file-id file-id) - (assoc :workspace-presence {}))) + (assoc :workspace-presence {}) + (update :workspace-global dissoc :default-font))) ptk/WatchEvent (watch [_ state stream] @@ -544,7 +545,7 @@ :workspace-tokens :workspace-undo :workspace-versions) - (update :workspace-global dissoc :read-only?) + (update :workspace-global dissoc :read-only? :default-font) (assoc-in [:workspace-global :options-mode] :design) (update :files d/update-vals #(dissoc % :data)))) diff --git a/frontend/src/app/main/data/workspace/texts.cljs b/frontend/src/app/main/data/workspace/texts.cljs index 2a905523b6..fb33068adc 100644 --- a/frontend/src/app/main/data/workspace/texts.cljs +++ b/frontend/src/app/main/data/workspace/texts.cljs @@ -824,7 +824,8 @@ (let [multiple? (->> data vals (d/seek #(= % :multiple)))] (cond-> state (not multiple?) - (assoc-in [:workspace-global :default-font] data)))))) + (assoc-in [:workspace-global :default-font] + (dissoc data :typography-ref-id :typography-ref-file))))))) (defn apply-text-modifier [shape text-modifier] diff --git a/frontend/test/frontend_tests/data/workspace_texts_test.cljs b/frontend/test/frontend_tests/data/workspace_texts_test.cljs index a52202bc48..5822b571cf 100644 --- a/frontend/test/frontend_tests/data/workspace_texts_test.cljs +++ b/frontend/test/frontend_tests/data/workspace_texts_test.cljs @@ -12,6 +12,7 @@ [app.common.types.modifiers :as ctm] [app.common.types.shape :as cts] [app.common.types.text :as txt] + [app.common.uuid :as uuid] [app.main.data.workspace.texts :as dwt] [app.main.ui.workspace.shapes.text.viewport-texts-html :as vth] [cljs.test :as t :include-macros true] @@ -377,6 +378,62 @@ (t/is (= "0.1" (:letter-spacing (first typographies))) "float letter-spacing is normalised to 2-decimal string"))))))) +;; --------------------------------------------------------------------------- +;; Tests: save-font must not persist typography refs into the global default font +;; +;; Root cause of #10925: typography assets are file-specific references, but +;; save-font used to write :typography-ref-id / :typography-ref-file into the +;; session-global [:workspace-global :default-font]. That state survives a file +;; switch, and v2-default-text-content bakes it into brand-new text shapes in +;; the other file, so they got a non-existent typography asset instead of the +;; default Penpot font. save-font now strips those two keys. +;; --------------------------------------------------------------------------- + +(t/deftest save-font-strips-typography-refs-from-default-font + (t/async + done + (let [file (-> (cthf/sample-file :file1) + (cths/add-sample-shape :text1 + :type :text + :x 0 :y 0 + :content (txt/change-text nil "hello"))) + store (ths/setup-store file) + attrs {:font-id "roboto" + :font-family "Roboto" + :font-variant-id "regular" + :font-size "14" + :typography-ref-id (uuid/next) + :typography-ref-file (:id file)}] + (ths/run-store + store done [(dwt/save-font attrs)] + (fn [new-state] + (let [default-font (get-in new-state [:workspace-global :default-font])] + (t/is (some? default-font)) + (t/is (= "roboto" (:font-id default-font))) + (t/is (nil? (:typography-ref-id default-font))) + (t/is (nil? (:typography-ref-file default-font))))))))) + +(t/deftest save-font-preserves-other-font-attrs + (t/async + done + (let [store (ths/setup-store (cthf/sample-file :file1)) + attrs {:font-family "Open Sans" + :font-id "opensans" + :font-variant-id "regular" + :font-size "18" + :line-height "1.5" + :letter-spacing "0" + :typography-ref-id (uuid/next) + :typography-ref-file (uuid/next)}] + (ths/run-store store done [(dwt/save-font attrs)] + (fn [new-state] + (let [default-font (get-in new-state [:workspace-global :default-font])] + (t/is (= "Open Sans" (:font-family default-font))) + (t/is (= "18" (:font-size default-font))) + (t/is (= "1.5" (:line-height default-font))) + (t/is (nil? (:typography-ref-id default-font))) + (t/is (nil? (:typography-ref-file default-font))))))))) + ;; --------------------------------------------------------------------------- ;; Tests: fix-position with degenerate selrect ;; ---------------------------------------------------------------------------