869764 - remove font loading via t2embed library under Windows/GDI

Status: RESOLVED FIXED

(Core :: Graphics: Text, defect)

Description

[John Daggett (:jtd)]

Given that the WOFF sanitizer effectively rebuilds downloadable font data, there's no real reason to be using the t2embed library calls to activate downloaded fonts.  This library has been the source of many security patches from MS and only supports TrueType-flavor OpenType fonts.  We should just use the AddFontMemResourceEx call which is currently called when load calls to t2embed fail (and always for Postcript/CFF fonts).

Comment 1

[John Daggett (:jtd)]

Attachment: patch, eliminate use of t2embed functions

Eliminates use of t2embed library font loading calls.  Instead, just load directly via AddFontMemResourceEx.  This eliminates the extra load inherent in using the t2embed library, since internally it copies the font and then calls AddFontMemResourceEx internally.

I removed the vertical table check.  For fonts that Windows deems to be Japanese, it creates two fonts, one with the name 'xxx' and a separate, vertical version with the name '@xxx'.  But our code accesses the font via the 'xxx' name, so the '@xxx' font is superfluous, it can safely be ignored.  Without this, reftests using mplus fonts were failing because those fonts lack a vhea table but Windows deemed them to be "Japanese" fonts and thus created the extra vertical face.

Comment 2

[John Daggett (:jtd)]

Note: for background on the vertical font name issue, see the discussion in bug 637481.

Comment 3

[Masatoshi Kimura [:emk]]

Comment on attachment 770031 [details] [diff] [review]
patch, eliminate use of t2embed functions

>-    IsCffFont(const uint8_t* aFontData, bool& hasVertical);
>-
>-#endif
>+    IsCffFont(const uint8_t* aFontData);

Why is IsCffFont() called on all platforms now?

>--- a/gfx/thebes/gfxGDIFontList.cpp
>+++ b/gfx/thebes/gfxGDIFontList.cpp

Please also remove |#include <t2embapi.h>|.

>-void gfxGDIFontList::InitializeFontEmbeddingProcs()

Please remove the function declaration from gfxGDIFontList.h.

And please change or remove a comment in layout/reftests/font-face/reftest.list.

r=me assuming that this patch passed the test and bug 637481 didn't regress.

Comment 4

[John Daggett (:jtd)]

Pushed to inbound, including changes based on review comments:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d032499bafe1

Comment 5

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/d032499bafe1