.notdef must not claim to be a character

font.go

@@ -88,15 +88,36 @@ type fontUse struct {
 // newFontUse returns an empty usage record already including .notdef (glyph 0).
 func newFontUse() *fontUse {
 	return &fontUse{
-		gids:  map[opentype.GlyphIndex]bool{0: true},
-		toUni: map[opentype.GlyphIndex][]rune{},
+		gids: map[opentype.GlyphIndex]bool{0: true},
+		// .notdef says a character was asked for that the font does not have.
+		// It is given the replacement character rather than nothing, because
+		// the page already shows the loss — the face's own .notdef draws a box
+		// — and text read off that page should say the same thing the page
+		// says. Omitting the entry instead makes poppler read "a 漢字 b かな c
+		// 한글 d" as "abcd", which loses the spaces as well as the characters
+		// and leaves a reader no way to know anything was there.
+		toUni: map[opentype.GlyphIndex][]rune{0: {'\uFFFD'}},
 	}
 }
 
 // mark records that glyph gid is used and, when runes is non-empty and the
 // glyph has no mapping yet, associates it with that text for copy/paste.
+//
+// Glyph 0 is .notdef, which stands for a character the font does not have. Its
+// mapping is fixed at the replacement character by newFontUse and nothing here
+// may change it, whatever text asked for it.
+//
+// It used to be otherwise: the first character the font lacked took glyph 0's
+// /ToUnicode entry, and every later missing character then read back as that
+// first one. Poppler read a file written here as "a 漢漢 b 漢漢 c 漢漢 d" where
+// "a 漢字 b かな c 한글 d" had been written, and turned "السلام عليكم" into one
+// Arabic letter repeated eleven times. That is not text with holes in it; it is
+// text that is confidently wrong, which is the worse of the two failures.
 func (u *fontUse) mark(gid opentype.GlyphIndex, runes []rune) {
 	u.gids[gid] = true
+	if gid == 0 {
+		return
+	}
 	if len(runes) > 0 {
 		if _, ok := u.toUni[gid]; !ok {
 			cp := make([]rune, len(runes))

tounicode_test.go

@@ -13,30 +13,57 @@ import (
 
 func TestBuildToUnicodeChunking(t *testing.T) {
 	u := newFontUse()
-	// 150 mapped glyphs force two bfchar blocks (100 + 50).
+	// 150 mapped glyphs, and .notdef is always mapped, so 151 entries force
+	// two bfchar blocks of 100 and 51.
 	for i := 1; i <= 150; i++ {
 		u.mark(opentype.GlyphIndex(i), []rune{rune('A' + i)})
 	}
 	out := string(buildToUnicode(u))
 	if strings.Count(out, "beginbfchar") != 2 {
 		t.Errorf("expected 2 bfchar blocks, got %d", strings.Count(out, "beginbfchar"))
 	}
-	if !strings.Contains(out, "100 beginbfchar") || !strings.Contains(out, "50 beginbfchar") {
+	if !strings.Contains(out, "100 beginbfchar") || !strings.Contains(out, "51 beginbfchar") {
 		t.Errorf("unexpected block sizes:\n%s", out[:200])
 	}
 }
 
-func TestBuildToUnicodeSkipsUnmapped(t *testing.T) {
-	u := newFontUse() // only .notdef, which has no text
+func TestBuildToUnicodeMapsNotdefToTheReplacementCharacter(t *testing.T) {
+	// A font used for nothing but characters it does not have still says so.
+	// .notdef reads back as U+FFFD, which is what the page shows too: the
+	// face's own .notdef draws a box.
+	u := newFontUse()
 	out := string(buildToUnicode(u))
-	if strings.Contains(out, "beginbfchar") {
-		t.Error("no entries expected for an all-unmapped font")
+	if !strings.Contains(out, "1 beginbfchar") {
+		t.Errorf("expected one entry for .notdef:\n%s", out)
+	}
+	if !strings.Contains(out, "<0000> <FFFD>") {
+		t.Errorf("glyph 0 is not mapped to the replacement character:\n%s", out)
 	}
 	if !strings.Contains(out, "endcmap") {
 		t.Error("CMap trailer missing")
 	}
 }
 
+func TestNotdefKeepsItsMappingWhateverAsksForIt(t *testing.T) {
+	// The defect this replaced: the first character a font lacked took glyph
+	// 0's entry, so every later missing character read back as that first one.
+	// Poppler read a file written here as "a 漢漢 b 漢漢 c 漢漢 d" where
+	// "a 漢字 b かな c 한글 d" had been written.
+	u := newFontUse()
+	u.mark(0, []rune{'漢'})
+	u.mark(0, []rune{'字'})
+	u.mark(0, []rune{'\U0001F600'})
+	if got := u.toUni[0]; len(got) != 1 || got[0] != '\uFFFD' {
+		t.Errorf("glyph 0 maps to %q, want the replacement character", string(got))
+	}
+	out := string(buildToUnicode(u))
+	for _, unwanted := range []string{"6F22", "5B57", "D83D"} {
+		if strings.Contains(out, unwanted) {
+			t.Errorf("the CMap claims .notdef is %s:\n%s", unwanted, out)
+		}
+	}
+}
+
 func TestUTF16beHexAstral(t *testing.T) {
 	// U+1F600 encodes as a surrogate pair: eight hex digits.
 	if got := utf16beHex([]rune{0x1F600}); got != "D83DDE00" {