Closed Bug 869764 Opened 7 years ago Closed 6 years ago

remove font loading via t2embed library under Windows/GDI


(Core :: Graphics: Text, defect)

Windows XP
Not set





(Reporter: jtd, Assigned: jtd)




(1 file)

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).
OS: Mac OS X → Windows XP
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.
Attachment #770031 - Flags: review?(VYV03354)
Note: for background on the vertical font name issue, see the discussion in bug 637481.
Comment on attachment 770031 [details] [diff] [review]
patch, eliminate use of t2embed functions

>-    IsCffFont(const uint8_t* aFontData, bool& hasVertical);
>+    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.
Attachment #770031 - Flags: review?(VYV03354) → review+
Pushed to inbound, including changes based on review comments:
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
Depends on: 1228799
You need to log in before you can comment on or make changes to this bug.