github/penpot
1
0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2026-08-11 15:29:06 +00:00
Code Issues Packages Projects Releases Wiki Activity

🐛 Fix font family typography asset persist across files in new created text layers (#11134)

Browse Source
This commit is contained in:
Luis de Dios 2026-08-10 11:35:51 +02:00 committed by GitHub
parent d63d6370c0
commit 86c563f11f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 172 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

110
frontend/playwright/ui/specs/workspace-texts-typography-persist.spec.js Normal file
View File

@ -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");
});

5
frontend/src/app/main/data/workspace.cljs
View File

@ -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))))

3
frontend/src/app/main/data/workspace/texts.cljs
View File

@ -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]

57
frontend/test/frontend_tests/data/workspace_texts_test.cljs
View File

@ -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
;; ---------------------------------------------------------------------------