Merge pull request #7770 from penpot/ladybenko-12587-fix-text-editor-crash-empty

🐛 Fix crash when using a font family with a number in its name

frontend/src/app/util/text/content/styles.cljs

@@ -37,11 +37,14 @@
          (not= (str/slice v -2) "px"))
     (str v "px")
 
+    (and (= k :font-family) (seq v))
+    (str/quote v)
+
     :else
     v))
 
 (defn normalize-attr-value
-  "This function strips units from attr values"
+  "This function strips units from attr values and un-scapes font-family"
   [k v]
   (cond
     (and (or (= k :font-size)
@@ -49,6 +52,9 @@
          (= (str/slice v -2) "px"))
     (str/slice v 0 -2)
 
+    (= k :font-family)
+    (str/unquote v)
+
     :else
     v))
 

frontend/text-editor/src/editor/TextEditor.js

@@ -209,7 +209,7 @@ export class TextEditor extends EventTarget {
     const rotation = transform?.rotation ?? 0.0;
     const scale = transform?.scale ?? 1.0;
     this.#updatePositionFromCanvas();
-    this.#element.style.transformOrigin = 'top left';
+    this.#element.style.transformOrigin = "top left";
     this.#element.style.transform = `scale(${scale}) translate(${x}px, ${y}px) rotate(${rotation}deg)`;
   }
 
@@ -225,7 +225,7 @@ export class TextEditor extends EventTarget {
       y: viewport.y + shape.selrect.y,
       rotation: shape.rotation,
       scale: viewport.zoom,
-    })
+    });
   }
 
   /**

frontend/text-editor/src/editor/content/dom/Content.js

@@ -75,26 +75,43 @@ export function mapContentFragmentFromDocument(document, root, styleDefaults) {
         currentParagraph = createParagraph(undefined, currentStyle);
       }
     }
-    const textSpan = createTextSpan(new Text(currentNode.nodeValue), currentStyle);
+    const textSpan = createTextSpan(
+      new Text(currentNode.nodeValue),
+      currentStyle,
+    );
     const fontSize = textSpan.style.getPropertyValue("font-size");
     if (!fontSize) {
       console.warn("font-size", fontSize);
-      textSpan.style.setProperty("font-size", styleDefaults?.getPropertyValue("font-size") ?? DEFAULT_FONT_SIZE);
+      textSpan.style.setProperty(
+        "font-size",
+        styleDefaults?.getPropertyValue("font-size") ?? DEFAULT_FONT_SIZE,
+      );
     }
     const fontFamily = textSpan.style.getPropertyValue("font-family");
     if (!fontFamily) {
       console.warn("font-family", fontFamily);
-      textSpan.style.setProperty("font-family", styleDefaults?.getPropertyValue("font-family") ?? DEFAULT_FONT_FAMILY);
+      const fontFamilyValue =
+        styleDefaults?.getPropertyValue("font-family") ?? DEFAULT_FONT_FAMILY;
+      const quotedFontFamily = fontFamilyValue.startsWith('"')
+        ? fontFamilyValue
+        : `"${fontFamilyValue}"`;
+      textSpan.style.setProperty("font-family", quotedFontFamily);
     }
     const fontWeight = textSpan.style.getPropertyValue("font-weight");
     if (!fontWeight) {
       console.warn("font-weight", fontWeight);
-      textSpan.style.setProperty("font-weight", styleDefaults?.getPropertyValue("font-weight") ?? DEFAULT_FONT_WEIGHT)
+      textSpan.style.setProperty(
+        "font-weight",
+        styleDefaults?.getPropertyValue("font-weight") ?? DEFAULT_FONT_WEIGHT,
+      );
     }
-    const fills = textSpan.style.getPropertyValue('--fills');
+    const fills = textSpan.style.getPropertyValue("--fills");
     if (!fills) {
       console.warn("fills", fills);
-      textSpan.style.setProperty("--fills", styleDefaults?.getPropertyValue("--fills") ?? DEFAULT_FILLS);
+      textSpan.style.setProperty(
+        "--fills",
+        styleDefaults?.getPropertyValue("--fills") ?? DEFAULT_FILLS,
+      );
     }
 
     currentParagraph.appendChild(textSpan);

frontend/text-editor/src/editor/content/dom/Style.js

@@ -13,6 +13,19 @@ const DEFAULT_FONT_SIZE_VALUE = parseFloat(DEFAULT_FONT_SIZE);
 const DEFAULT_LINE_HEIGHT = "1.2";
 const DEFAULT_FONT_WEIGHT = "400";
 
+/** Sanitizes font-family values to be quoted, so it handles multi-word font names
+ * with numbers like "Font Awesome 7 Free"
+ *
+ * @param {string} value
+ */
+export function sanitizeFontFamily(value) {
+  if (value && value.length > 0 && !value.startsWith('"')) {
+    return `"${value}"`;
+  } else {
+    return value;
+  }
+}
+
 /**
  * Merges two style declarations. `source` -> `target`.
  *
@@ -25,7 +38,7 @@ export function mergeStyleDeclarations(target, source) {
   // for (const styleName of source) {
   for (let index = 0; index < source.length; index++) {
     const styleName = source.item(index);
-    const styleValue = source.getPropertyValue(styleName);
+    let styleValue = source.getPropertyValue(styleName);
     target.setProperty(styleName, styleValue);
   }
   return target;
@@ -122,11 +135,14 @@ export function getComputedStylePolyfill(element) {
       if (currentValue) {
         const priority = currentElement.style.getPropertyPriority(styleName);
         if (priority === "important") {
-          const newValue = currentElement.style.getPropertyValue(styleName);
+          let newValue = currentElement.style.getPropertyValue(styleName);
           inertElement.style.setProperty(styleName, newValue);
         }
       } else {
-        const newValue = currentElement.style.getPropertyValue(styleName);
+        let newValue = currentElement.style.getPropertyValue(styleName);
+        if (styleName === "font-family") {
+          newValue = sanitizeFontFamily(newValue);
+        }
         inertElement.style.setProperty(styleName, newValue);
       }
     }
@@ -210,10 +226,16 @@ export function setStyle(element, styleName, styleValue, styleUnit) {
     typeof styleValue !== "string" &&
     typeof styleValue !== "number"
   ) {
-    if (styleName === "--fills" && styleValue === null) debugger;
     element.style.setProperty(styleName, JSON.stringify(styleValue));
   } else {
-    element.style.setProperty(styleName, styleValue + (styleUnit ?? ""));
+    if (styleName === "font-family") {
+      styleValue = sanitizeFontFamily(styleValue);
+    }
+
+    element.style.setProperty(
+      styleName,
+      styleValue + (styleUnit ? styleUnit : ""),
+    );
   }
   return element;
 }
@@ -289,15 +311,20 @@ export function getStyle(element, styleName, styleUnit) {
  * @returns {HTMLElement}
  */
 export function setStylesFromObject(element, allowedStyles, styleObject) {
-  for (const [styleName, styleUnit] of allowedStyles) {
+  if (element.tagName === "SPAN")
-    if (!(styleName in styleObject)) {
+    for (const [styleName, styleUnit] of allowedStyles) {
-      continue;
+      if (!(styleName in styleObject)) {
+        continue;
+      }
+      let styleValue = styleObject[styleName];
+      if (styleName === "font-family") {
+        styleValue = sanitizeFontFamily(styleValue);
+      }
+
+      if (styleValue) {
+        setStyle(element, styleName, styleValue, styleUnit);
+      }
     }
-    const styleValue = styleObject[styleName];
-    if (styleValue) {
-      setStyle(element, styleName, styleValue, styleUnit);
-    }
-  }
   return element;
 }
 

frontend/text-editor/src/editor/content/dom/TextSpan.js

@@ -115,9 +115,9 @@ export function createTextSpan(textOrLineBreak, styles, attrs) {
     textOrLineBreak instanceof Text &&
     textOrLineBreak.nodeValue.length === 0
   ) {
-    console.trace("nodeValue", textOrLineBreak.nodeValue);
     throw new TypeError("Invalid text span child, cannot be an empty text");
   }
+
   return createElement(TAG, {
     attributes: { id: createRandomId(), ...attrs },
     data: { itype: TYPE },

frontend/text-editor/src/editor/controllers/SelectionController.js

@@ -53,6 +53,7 @@ import CommandMutations from "../commands/CommandMutations.js";
 import { isRoot, setRootStyles } from "../content/dom/Root.js";
 import { SelectionDirection } from "./SelectionDirection.js";
 import SafeGuard from "./SafeGuard.js";
+import { sanitizeFontFamily } from "../content/dom/Style.js";
 
 /**
  * Supported options for the SelectionController.
@@ -206,7 +207,7 @@ export class SelectionController extends EventTarget {
   /**
    * @type {TextEditorOptions}
    */
-  #options
+  #options;
 
   /**
    * Constructor
@@ -277,7 +278,12 @@ export class SelectionController extends EventTarget {
   #applyStylesToCurrentStyle(element) {
     for (let index = 0; index < element.style.length; index++) {
       const styleName = element.style.item(index);
-      const styleValue = element.style.getPropertyValue(styleName);
+
+      let styleValue = element.style.getPropertyValue(styleName);
+      if (styleName === "font-family") {
+        styleValue = sanitizeFontFamily(styleValue);
+      }
+
       this.#currentStyle.setProperty(styleName, styleValue);
     }
   }
@@ -525,21 +531,21 @@ export class SelectionController extends EventTarget {
     const root = this.#textEditor.root;
     const firstParagraph = root.firstElementChild;
     const lastParagraph = root.lastElementChild;
-    
+
     if (!firstParagraph || !lastParagraph) {
       return this;
     }
 
     const firstTextSpan = firstParagraph.firstElementChild;
     const lastTextSpan = lastParagraph.lastElementChild;
-    
+
     if (!firstTextSpan || !lastTextSpan) {
       return this;
     }
 
     const firstTextNode = firstTextSpan.firstChild;
     const lastTextNode = lastTextSpan.lastChild;
-    
+
     if (!firstTextNode || !lastTextNode) {
       return this;
     }
@@ -548,10 +554,10 @@ export class SelectionController extends EventTarget {
     const range = document.createRange();
     range.setStart(firstTextNode, 0);
     range.setEnd(lastTextNode, lastTextNode.nodeValue?.length || 0);
-    
+
     this.#selection.removeAllRanges();
     this.#selection.addRange(range);
-    
+
     // Ensure internal state is synchronized
     this.#focusNode = this.#selection.focusNode;
     this.#focusOffset = this.#selection.focusOffset;
@@ -560,10 +566,10 @@ export class SelectionController extends EventTarget {
     this.#range = range;
     this.#ranges.clear();
     this.#ranges.add(range);
-    
+
     // Notify style changes
     this.#notifyStyleChange();
-    
+
     return this;
   }
 
@@ -783,7 +789,6 @@ export class SelectionController extends EventTarget {
     if (this.#savedSelection) {
       return this.#savedSelection.focusNode;
     }
-    if (!this.#focusNode) console.trace("focusNode", this.#focusNode);
     return this.#focusNode;
   }
 
@@ -1156,13 +1161,16 @@ export class SelectionController extends EventTarget {
       this.focusParagraph.after(fragment);
       mergeParagraphs(a, b);
     } else {
-      if (this.isTextSpanStart) { 
+      if (this.isTextSpanStart) {
         this.focusTextSpan.before(...fragment.firstElementChild.children);
       } else if (this.isTextSpanEnd) {
         this.focusTextSpan.after(...fragment.firstElementChild.children);
       } else {
         const newTextSpan = splitTextSpan(this.focusTextSpan, this.focusOffset);
-        this.focusTextSpan.after(...fragment.firstElementChild.children, newTextSpan);
+        this.focusTextSpan.after(
+          ...fragment.firstElementChild.children,
+          newTextSpan,
+        );
       }
     }
     if (isLineBreak(collapseNode)) {
@@ -1292,7 +1300,10 @@ export class SelectionController extends EventTarget {
     this.#textNodeIterator.currentNode = this.focusNode;
 
     const originalNodeValue = this.focusNode.nodeValue || "";
-    const wordStart = findPreviousWordBoundary(originalNodeValue, this.focusOffset);
+    const wordStart = findPreviousWordBoundary(
+      originalNodeValue,
+      this.focusOffset,
+    );
 
     // Start node
     if (wordStart === this.focusOffset && this.focusOffset === 0) {

frontend/text-editor/src/playground/text.js

@@ -71,6 +71,7 @@ export class TextSpan {
     if (!font) {
       throw new Error(`Invalid font "${fontFamily}"`);
     }
+
     return new TextSpan({
       fontId: font.id, // leafElement.style.getPropertyValue("--font-id"),
       fontFamilyHash: 0,