From 353501dc181bec9c1e1e4c114c7c46ff4b1db404 Mon Sep 17 00:00:00 2001
From: Khaled Hosny <khaled@aliftype.com>
Date: Sat, 12 Oct 2024 17:56:53 +0300
Subject: [PATCH] [outlineCompiler] Make space the 2nd glyph unless
 public.glyphOrder is set

Fixes https://github.com/googlefonts/ufo2ft/issues/880
---
 Lib/ufo2ft/outlineCompiler.py                 |  2 +-
 Lib/ufo2ft/util.py                            |  5 ++++
 .../TestSpaceOrder.ufo/glyphs/_notdef.glif    |  5 ++++
 tests/data/TestSpaceOrder.ufo/glyphs/a.glif   |  5 ++++
 tests/data/TestSpaceOrder.ufo/glyphs/b.glif   |  5 ++++
 tests/data/TestSpaceOrder.ufo/glyphs/c.glif   |  5 ++++
 .../TestSpaceOrder.ufo/glyphs/contents.plist  | 24 +++++++++++++++++++
 tests/data/TestSpaceOrder.ufo/glyphs/d.glif   |  5 ++++
 tests/data/TestSpaceOrder.ufo/glyphs/e.glif   |  5 ++++
 tests/data/TestSpaceOrder.ufo/glyphs/f.glif   |  5 ++++
 tests/data/TestSpaceOrder.ufo/glyphs/g.glif   |  5 ++++
 .../data/TestSpaceOrder.ufo/glyphs/space.glif |  5 ++++
 .../TestSpaceOrder.ufo/layercontents.plist    | 10 ++++++++
 tests/data/TestSpaceOrder.ufo/metainfo.plist  | 10 ++++++++
 tests/outlineCompiler_test.py                 | 24 +++++++++++++++++++
 15 files changed, 119 insertions(+), 1 deletion(-)
 create mode 100644 tests/data/TestSpaceOrder.ufo/glyphs/_notdef.glif
 create mode 100644 tests/data/TestSpaceOrder.ufo/glyphs/a.glif
 create mode 100644 tests/data/TestSpaceOrder.ufo/glyphs/b.glif
 create mode 100644 tests/data/TestSpaceOrder.ufo/glyphs/c.glif
 create mode 100644 tests/data/TestSpaceOrder.ufo/glyphs/contents.plist
 create mode 100644 tests/data/TestSpaceOrder.ufo/glyphs/d.glif
 create mode 100644 tests/data/TestSpaceOrder.ufo/glyphs/e.glif
 create mode 100644 tests/data/TestSpaceOrder.ufo/glyphs/f.glif
 create mode 100644 tests/data/TestSpaceOrder.ufo/glyphs/g.glif
 create mode 100644 tests/data/TestSpaceOrder.ufo/glyphs/space.glif
 create mode 100644 tests/data/TestSpaceOrder.ufo/layercontents.plist
 create mode 100644 tests/data/TestSpaceOrder.ufo/metainfo.plist

diff --git a/Lib/ufo2ft/outlineCompiler.py b/Lib/ufo2ft/outlineCompiler.py
index d8816c629..ffc7245a4 100644
--- a/Lib/ufo2ft/outlineCompiler.py
+++ b/Lib/ufo2ft/outlineCompiler.py
@@ -130,7 +130,7 @@ def __init__(
         self.allGlyphs = glyphSet
         # store the glyph order
         if glyphOrder is None:
-            glyphOrder = font.glyphOrder
+            glyphOrder = font.lib.get("public.glyphOrder")
         self.glyphOrder = self.makeOfficialGlyphOrder(glyphOrder)
         # make a reusable character mapping
         self.unicodeToGlyphNameMapping = self.makeUnicodeToGlyphNameMapping()
diff --git a/Lib/ufo2ft/util.py b/Lib/ufo2ft/util.py
index 17d4eff12..842d2b9d9 100644
--- a/Lib/ufo2ft/util.py
+++ b/Lib/ufo2ft/util.py
@@ -34,13 +34,18 @@ def makeOfficialGlyphOrder(font, glyphOrder=None):
     If ".notdef" glyph is present in the font, force this to always be
     the first glyph (at index 0).
     """
+    reorderSpace = False
     if glyphOrder is None:
         glyphOrder = getattr(font, "glyphOrder", ())
+        reorderSpace = True
     names = set(font.keys())
     order = []
     if ".notdef" in names:
         names.remove(".notdef")
         order.append(".notdef")
+    if reorderSpace and "space" in names:
+        names.remove("space")
+        order.append("space")
     for name in glyphOrder:
         if name not in names:
             continue
diff --git a/tests/data/TestSpaceOrder.ufo/glyphs/_notdef.glif b/tests/data/TestSpaceOrder.ufo/glyphs/_notdef.glif
new file mode 100644
index 000000000..f128bbacc
--- /dev/null
+++ b/tests/data/TestSpaceOrder.ufo/glyphs/_notdef.glif
@@ -0,0 +1,5 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<glyph name=".notdef" format="2">
+  <outline>
+  </outline>
+</glyph>
diff --git a/tests/data/TestSpaceOrder.ufo/glyphs/a.glif b/tests/data/TestSpaceOrder.ufo/glyphs/a.glif
new file mode 100644
index 000000000..3a41467df
--- /dev/null
+++ b/tests/data/TestSpaceOrder.ufo/glyphs/a.glif
@@ -0,0 +1,5 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<glyph name="a" format="2">
+  <outline>
+  </outline>
+</glyph>
diff --git a/tests/data/TestSpaceOrder.ufo/glyphs/b.glif b/tests/data/TestSpaceOrder.ufo/glyphs/b.glif
new file mode 100644
index 000000000..6358f013a
--- /dev/null
+++ b/tests/data/TestSpaceOrder.ufo/glyphs/b.glif
@@ -0,0 +1,5 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<glyph name="b" format="2">
+  <outline>
+  </outline>
+</glyph>
diff --git a/tests/data/TestSpaceOrder.ufo/glyphs/c.glif b/tests/data/TestSpaceOrder.ufo/glyphs/c.glif
new file mode 100644
index 000000000..98548af80
--- /dev/null
+++ b/tests/data/TestSpaceOrder.ufo/glyphs/c.glif
@@ -0,0 +1,5 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<glyph name="c" format="2">
+  <outline>
+  </outline>
+</glyph>
diff --git a/tests/data/TestSpaceOrder.ufo/glyphs/contents.plist b/tests/data/TestSpaceOrder.ufo/glyphs/contents.plist
new file mode 100644
index 000000000..a420eec89
--- /dev/null
+++ b/tests/data/TestSpaceOrder.ufo/glyphs/contents.plist
@@ -0,0 +1,24 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
+<plist version="1.0">
+  <dict>
+    <key>b</key>
+    <string>b.glif</string>
+    <key>a</key>
+    <string>a.glif</string>
+    <key>c</key>
+    <string>c.glif</string>
+    <key>d</key>
+    <string>d.glif</string>
+    <key>space</key>
+    <string>space.glif</string>
+    <key>.notdef</key>
+    <string>_notdef.glif</string>
+    <key>e</key>
+    <string>e.glif</string>
+    <key>f</key>
+    <string>f.glif</string>
+    <key>g</key>
+    <string>g.glif</string>
+  </dict>
+</plist>
diff --git a/tests/data/TestSpaceOrder.ufo/glyphs/d.glif b/tests/data/TestSpaceOrder.ufo/glyphs/d.glif
new file mode 100644
index 000000000..880134caa
--- /dev/null
+++ b/tests/data/TestSpaceOrder.ufo/glyphs/d.glif
@@ -0,0 +1,5 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<glyph name="d" format="2">
+  <outline>
+  </outline>
+</glyph>
diff --git a/tests/data/TestSpaceOrder.ufo/glyphs/e.glif b/tests/data/TestSpaceOrder.ufo/glyphs/e.glif
new file mode 100644
index 000000000..67c2068d9
--- /dev/null
+++ b/tests/data/TestSpaceOrder.ufo/glyphs/e.glif
@@ -0,0 +1,5 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<glyph name="e" format="2">
+  <outline>
+  </outline>
+</glyph>
diff --git a/tests/data/TestSpaceOrder.ufo/glyphs/f.glif b/tests/data/TestSpaceOrder.ufo/glyphs/f.glif
new file mode 100644
index 000000000..26491803e
--- /dev/null
+++ b/tests/data/TestSpaceOrder.ufo/glyphs/f.glif
@@ -0,0 +1,5 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<glyph name="f" format="2">
+  <outline>
+  </outline>
+</glyph>
diff --git a/tests/data/TestSpaceOrder.ufo/glyphs/g.glif b/tests/data/TestSpaceOrder.ufo/glyphs/g.glif
new file mode 100644
index 000000000..75b3cddc9
--- /dev/null
+++ b/tests/data/TestSpaceOrder.ufo/glyphs/g.glif
@@ -0,0 +1,5 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<glyph name="g" format="2">
+  <outline>
+  </outline>
+</glyph>
diff --git a/tests/data/TestSpaceOrder.ufo/glyphs/space.glif b/tests/data/TestSpaceOrder.ufo/glyphs/space.glif
new file mode 100644
index 000000000..2c3e121d4
--- /dev/null
+++ b/tests/data/TestSpaceOrder.ufo/glyphs/space.glif
@@ -0,0 +1,5 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<glyph name="space" format="2">
+  <outline>
+  </outline>
+</glyph>
diff --git a/tests/data/TestSpaceOrder.ufo/layercontents.plist b/tests/data/TestSpaceOrder.ufo/layercontents.plist
new file mode 100644
index 000000000..b9c1a4f27
--- /dev/null
+++ b/tests/data/TestSpaceOrder.ufo/layercontents.plist
@@ -0,0 +1,10 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
+<plist version="1.0">
+  <array>
+    <array>
+      <string>public.default</string>
+      <string>glyphs</string>
+    </array>
+  </array>
+</plist>
diff --git a/tests/data/TestSpaceOrder.ufo/metainfo.plist b/tests/data/TestSpaceOrder.ufo/metainfo.plist
new file mode 100644
index 000000000..7b8b34ac6
--- /dev/null
+++ b/tests/data/TestSpaceOrder.ufo/metainfo.plist
@@ -0,0 +1,10 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
+<plist version="1.0">
+  <dict>
+    <key>creator</key>
+    <string>com.github.fonttools.ufoLib</string>
+    <key>formatVersion</key>
+    <integer>3</integer>
+  </dict>
+</plist>
diff --git a/tests/outlineCompiler_test.py b/tests/outlineCompiler_test.py
index 5755ce5d2..fb4a32fd7 100644
--- a/tests/outlineCompiler_test.py
+++ b/tests/outlineCompiler_test.py
@@ -761,6 +761,30 @@ def test_compile_strange_glyph_order(self, quadufo):
         compiler.compile()
         assert compiler.otf.getGlyphOrder() == EXPECTED_ORDER
 
+    def test_compile_reorder_space_glyph(self, FontClass):
+        """
+        Test that ufo2ft always puts .notdef first, and put space second if no
+        explicit glyph order is set.
+        """
+        # This UFO does not have a "public.glyphOrder"
+        ufo = FontClass(getpath("TestSpaceOrder.ufo"))
+
+        EXPECTED_ORDER = [".notdef", "space", "a", "b", "c", "d", "e", "f", "g"]
+        # We can’t assert a specific glyph order here because ufolib2 and
+        # defcon do things differently, so only assert it is not the expected
+        # final re-ordered one.
+        assert list(ufo.keys()) != EXPECTED_ORDER
+        assert ufo.glyphOrder != EXPECTED_ORDER
+        compiler = OutlineTTFCompiler(ufo)
+        compiler.compile()
+        assert compiler.otf.getGlyphOrder() == EXPECTED_ORDER
+
+        EXPECTED_ORDER = [".notdef", "a", "b", "c", "d", "space", "e", "f", "g"]
+        ufo.glyphOrder = EXPECTED_ORDER
+        compiler = OutlineTTFCompiler(ufo)
+        compiler.compile()
+        assert compiler.otf.getGlyphOrder() == EXPECTED_ORDER
+
 
 class NamesTest:
     @pytest.mark.parametrize(