keeleysam / tenfourfox

Automatically exported from code.google.com/p/tenfourfox
0 stars 0 forks source link

Font black list for ATSUI-incompatible webfonts #261

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Uplift from 
https://tenfourfox.tenderapp.com/discussions/problems/715-2430-crashes

Certain web fonts do not appear to be compatible with ATSUI or trigger bugs. On 
10.4, this generates warnings; on 10.5, this crashes the browser. This is due 
to Mozilla's changes in 24 that cause us to use ATSUI on both 10.4 and 10.5 (on 
10.5, previously they used a different code path).

Since the font name is identifiable, we should be able to intercept it before 
it becomes activated.

Original issue reported on code.google.com by classi...@floodgap.com on 7 Mar 2014 at 4:27

GoogleCodeExporter commented 9 years ago
We could sneak this into the font validator, since we would be treating this 
like a font that didn't validate.

Original comment by classi...@floodgap.com on 7 Mar 2014 at 5:10

GoogleCodeExporter commented 9 years ago
This should work. In gfxUserFontSet::LoadFont, after the GetFullNameFromSFNT 
call, we should be able to do

+ if (!originalFullName.EqualsLiteral("prisjakticons"))
  fe = gfxPlatform::GetPlatform()->MakePlatformFont(aProxy,
[...]

and then it will not try to load the font at all.

Original comment by classi...@floodgap.com on 11 Mar 2014 at 4:36

GoogleCodeExporter commented 9 years ago

Original comment by classi...@floodgap.com on 11 Mar 2014 at 4:47

GoogleCodeExporter commented 9 years ago
I had a similar (if not the same) problem in Leopard WebKit as well and found a 
solution to treat it without a real blacklist.

Take a look at the following code fragments:
This one does the actual loading from memory via ATS and verifies whether the 
font may work at all:
+    // The value "3" means that the font is private and can't be seen by 
anyone else.
+    OSStatus err = ATSFontActivateFromMemory((void*)buffer->data(), 
buffer->size(), 3, kATSFontFormatUnspecified, NULL, kATSOptionFlagsDefault, 
&containerRef);
+    if (err != noErr || !containerRef)
+        return 0;
+    ItemCount fontCount;
+    err = ATSFontFindFromContainer(containerRef, kATSOptionFlagsDefault, 0, 
NULL, &fontCount);
+    
+    // We just support the first font in the list.
+    if (err != noErr || fontCount == 0) {
+        ATSFontDeactivate(containerRef, NULL, kATSOptionFlagsDefault);
+        return 0;
+    }
+    
+    ATSFontRef fontRef = 0;
+    err = ATSFontFindFromContainer(containerRef, kATSOptionFlagsDefault, 1, 
&fontRef, NULL);
+    if (err != noErr || !fontRef) {
+        ATSFontDeactivate(containerRef, NULL, kATSOptionFlagsDefault);
+        return 0;
+    }
+
+    CFStringRef fontName;
+    err = ATSFontGetPostScriptName(fontRef, kATSOptionFlagsDefault, &fontName);
+    RetainPtr<CFStringRef> fontNameRetained = adoptCF(fontName);
+    if (err != noErr || !fontName || CFEqual(fontName, 
CFSTR("Padauk-Regular"))) {
+        ATSFontDeactivate(containerRef, NULL, kATSOptionFlagsDefault);
+        return 0;
+    }
+
+    cgFontRef = adoptCF(CGFontCreateWithPlatformFont(&fontRef));
+    // Workaround for <rdar://problem/5675504>.
+    if (cgFontRef && !CGFontGetNumberOfGlyphs(cgFontRef.get()))
+        cgFontRef = 0;
+    if (!cgFontRef) {
+        ATSFontDeactivate(containerRef, NULL, kATSOptionFlagsDefault);
+        return 0;
+    }
The results are a ATSFontRef (fontRef) and a CGFontRef (cgFontRef).
I added passing the ATSFontRef just in case I would need it but it turned out 
to not be necessary. But as I changed quite a number of classes and interfaces 
I left it in the stable version (537) but the unstable version (538) doesn't 
have it at all and as far as I know doesn't have any problems.
The only font that is filtered out completely is Padauk-Regular, which is a 
Burmese font on Wikipedia.

This part tries to create a CoreText CTFontRef from either the CGFontRef or the 
ATSFontRef and checks whether turning off the CoreText font cascading (by 
adding LastResort as the only font it will resort to) will work for the 
CTFontRef:
+    {
+        RetainPtr<CTFontRef> newFont;
+        if (m_atsFont > 0)
+        {
+            newFont = adoptCF(CTFontCreateWithPlatformFont(m_atsFont, m_size, 
0, 0));
+        }
+        else
+        {
+            newFont = adoptCF(CTFontCreateWithGraphicsFont(m_cgFont.get(), 
m_size, 0, 0));
+        }
+        RetainPtr<CFStringRef> newFontPostScriptName = 
CTFontCopyPostScriptName(newFont.get());
+        m_CTFont = adoptCF(CTFontCreateCopyWithAttributes(newFont.get(), 
m_size, 0, cascadeToLastResortFontDescriptor()));
+        RetainPtr<CFStringRef> ctFontPostScriptName = 
CTFontCopyPostScriptName(m_CTFont.get());
+        if ((newFontPostScriptName.get() != ctFontPostScriptName.get())
+                || (ctFontPostScriptName.get() && newFontPostScriptName.get()
+                    && !CFEqual(ctFontPostScriptName.get(), 
newFontPostScriptName.get())))
+            m_CTFont = newFont;
+    }
This part is obviously unnecessary when either CoreText is not used or 
automatic CoreText font cascading doesn't matter.

I found the following message in the system log after having navigated to 
http://www.prisjakt.nu :
"/System/Library/Frameworks/ApplicationServices.framework/Frameworks/ATS.framewo
rk/Support/ATSServer[99]: ********* Bad On-Memory Font Detected ********* 
(fontID = 1245351)"
Nevertheless the site loads and displays OK without crashing the browser, so it 
seems the above checks correctly recognized the problem.

Original comment by Tobias.N...@gmail.com on 11 Mar 2014 at 7:18

GoogleCodeExporter commented 9 years ago
There's actually one more error message from ATS server int system log after 
loading http://www.prisjakt.nu :
"com.apple.ATSServer[99]: FODBError: FODBRemoveFromAnnex - Got called to remove 
an annexIndex that does not match what is in the font"
It comes together with the one mentioned in my last posting.

Original comment by Tobias.N...@gmail.com on 11 Mar 2014 at 7:23

GoogleCodeExporter commented 9 years ago
gfxUserFontSet already checks the container and the PostScript name after the 
OTS has declared the font good. It looks like the only new code is the part 
that calls CGFontCreateWithPlatformFont and CGFontGetNumberOfGlyphs. I think I 
might try this for 29, but I'd rather stay with a minimal approach for 24 for 
the moment.

Original comment by classi...@floodgap.com on 11 Mar 2014 at 8:14

GoogleCodeExporter commented 9 years ago
This didn't work, but I need to verify that really is the SFNT name. Message 
noted in Console:

Mar 15 14:20:23 Bruce /Applications/TenFourFoxG5.app/Contents/MacOS/firefox: 
Glyph count mismatch: ATSFontGetGlyphCount returned 248; FOCountGlyphs returned 
147.

Original comment by classi...@floodgap.com on 15 Mar 2014 at 9:27

GoogleCodeExporter commented 9 years ago
Eventually had to match on the psname, since Tobias' code implies that's safe 
to do on these bad fonts, and that does match and reject correctly. The warning 
does not appear anymore on 10.4.

Original comment by classi...@floodgap.com on 15 Mar 2014 at 9:51

GoogleCodeExporter commented 9 years ago

Original comment by classi...@floodgap.com on 16 Mar 2014 at 8:08

GoogleCodeExporter commented 9 years ago
In 24.4.0 prisjakt.nu now loads without crashing the browser on 10.5.

Original comment by chtru...@web.de on 17 Mar 2014 at 5:35