From bab3a7380727f71e20f88a794f2fc73cc897073b 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 its order
 is explicitly set

Fixes https://github.com/googlefonts/ufo2ft/issues/880
---
 Lib/ufo2ft/util.py                            |  8 ++-
 .../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                 | 61 +++++++++++++++++++
 14 files changed, 157 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/util.py b/Lib/ufo2ft/util.py
index 17d4eff12..a7471a1a0 100644
--- a/Lib/ufo2ft/util.py
+++ b/Lib/ufo2ft/util.py
@@ -32,15 +32,21 @@ def makeOfficialGlyphOrder(font, glyphOrder=None):
     If not explicit glyphOrder is defined, sort glyphs alphabetically.
 
     If ".notdef" glyph is present in the font, force this to always be
-    the first glyph (at index 0).
+    the first glyph (at index 0). Also, if "space" is present in the font and
+    missing from glyphOrder, force it to be the second glyph (at index 1).
     """
     if glyphOrder is None:
         glyphOrder = getattr(font, "glyphOrder", ())
+    reorderSpace = "space" not in glyphOrder
+    print(reorderSpace)
     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..fedf226ce 100644
--- a/tests/outlineCompiler_test.py
+++ b/tests/outlineCompiler_test.py
@@ -761,6 +761,67 @@ def test_compile_strange_glyph_order(self, quadufo):
         compiler.compile()
         assert compiler.otf.getGlyphOrder() == EXPECTED_ORDER
 
+    def test_compile_reorder_space_glyph(self, quadufo):
+        """
+        Test that ufo2ft always puts .notdef first, and put space second if no
+        explicit glyph order is set.
+        """
+        EXPECTED_ORDER = [
+            ".notdef",
+            "space",
+            "a",
+            "b",
+            "c",
+            "d",
+            "e",
+            "f",
+            "g",
+            "h",
+            "i",
+            "j",
+            "k",
+            "l",
+        ]
+        # Check with no public.glyphOrder
+        del quadufo.lib["public.glyphOrder"]
+        assert not quadufo.glyphOrder
+        compiler = OutlineTTFCompiler(quadufo)
+        compiler.compile()
+        assert compiler.otf.getGlyphOrder() == EXPECTED_ORDER
+
+        # Empty glyphOrder is considered the same
+        quadufo.glyphOrder = []
+        compiler = OutlineTTFCompiler(quadufo)
+        compiler.compile()
+        assert compiler.otf.getGlyphOrder() == EXPECTED_ORDER
+
+        # Non-empty glyphOrder without "space" is considered the same
+        quadufo.glyphOrder = [n for n in EXPECTED_ORDER if n != "space"]
+        compiler = OutlineTTFCompiler(quadufo)
+        compiler.compile()
+        assert compiler.otf.getGlyphOrder() == EXPECTED_ORDER
+
+        EXPECTED_ORDER = [
+            ".notdef",
+            "a",
+            "b",
+            "c",
+            "d",
+            "space",
+            "e",
+            "f",
+            "g",
+            "h",
+            "i",
+            "j",
+            "k",
+            "l",
+        ]
+        quadufo.glyphOrder = EXPECTED_ORDER
+        compiler = OutlineTTFCompiler(quadufo)
+        compiler.compile()
+        assert compiler.otf.getGlyphOrder() == EXPECTED_ORDER
+
 
 class NamesTest:
     @pytest.mark.parametrize(