Chromium bug: http://crbug.com/31879 OpenType font with CFF glyphs is not handled correctly on Windows XP. Nonexistent glyphs are treated as existent (http://crbug.com/31879, http://crbug.com/30759, and http://crbug.com/32510) and vice versa (http://crbug.com/29805). fillBMPGlyphs() function in platform/graphics/chromium/GlyphPageTreeNodeChromiumWin.cpp should be modified so the function can detect OpenType fonts with CFF glyphs: http://trac.webkit.org/browser/trunk/WebCore/platform/graphics/chromium/GlyphPageTreeNodeChromiumWin.cpp?rev=45325#L137 136 int invalidGlyph = 0xFFFF; 137 if (!isVistaOrNewer() && !(tm.tmPitchAndFamily & TMPF_TRUETYPE)) // <----- TMPF_TRUETYPE is not set for OpenType fonts with CFF glyphs!! 138 invalidGlyph = 0x1F;
Created attachment 48386 [details] opentype_detection_v1
Attachment 48386 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebCore/platform/graphics/chromium/GlyphPageTreeNodeChromiumWin.cpp:137: One space before end of line comments [whitespace/comments] [5] Total errors found: 1 If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 48387 [details] opentype_detection_v2_style_fixed
Comment on attachment 48387 [details] opentype_detection_v2_style_fixed Looks reasonable, but I wish we had a test for this change. I'm not an expert on the Windows font system. You might want to run this by someone who's an expert on GlyphPageTreeNodeChromiumWin before landing.
(In reply to comment #4) Adam, reviewers, Thanks for the review! I agree that it's better to add a test for this issue. I'll create _v3 patch that has a test something like "LayoutTests/fast/css/font-face-opentype.html." Before starting this, let me ask two questions: - To write the test, a sample OpenType font is necessary. Though we have some TrueType sample fonts (e.g. LayoutTests/fast/css/resources/Ahem.ttf) in WebKit tree, but we don't have any OpenType (CFF) fonts yet. What kind of fonts can we add to the WebKit tree, in terms of license, etc? - This issue is Chromium only (and Windows XP only). Does this make sense to put the test HTML on LayoutTests/fast/css/? Is there better place to live? Thanks, Yusuke
> - To write the test, a sample OpenType font is necessary. Though we have some > TrueType sample fonts (e.g. LayoutTests/fast/css/resources/Ahem.ttf) in WebKit > tree, but we don't have any OpenType (CFF) fonts yet. What kind of fonts can we > add to the WebKit tree, in terms of license, etc? In general, we accept patches under BSD and LGPL 2.1. Hopefully there exist OpenType fonts under that license. > - This issue is Chromium only (and Windows XP only). Does this make sense to > put the test HTML on LayoutTests/fast/css/? Is there better place to live? As long as the test passes on other platforms, it's good to have the test coverage. :) LayoutTests/fast/css seems fine, unless you see a directory more specific to @font-face.
Created attachment 48702 [details] opentype_detection_v3
(In reply to comment #7) Added a layout test. This test passes on Chrome with the patch and Safari, but fails on Chrome current release. The patch against GlyphPageTreeNodeChromiumWin.cpp is not changed at all.
Comment on attachment 48702 [details] opentype_detection_v3 > --- /dev/null > +++ b/LayoutTests/platform/mac/fast/css/font-face-opentype-expected.txt It's hard to understand why this expectation is correct. I recommend: - Add a PNG result, or - Check the result by JavaScript code such as element.offsetWidth==n*16.
(In reply to comment #9) Sure, I'll add a PNG result. I think there's no way (except polling) to check if a @font-face remote font has loaded. The <body onload=""> handler does not help since it might be called before the font loading finishes. Therefore, checking the offsetWidth from JavaScript seems not to be a good idea.
Created attachment 48705 [details] opentype_detection_v4_pixel_test_added
Comment on attachment 48705 [details] opentype_detection_v4_pixel_test_added > diff --git a/LayoutTests/platform/mac/fast/css/font-face-opentype-expected.png b/LayoutTests/platform/mac/fast/css/font-face-opentype-expected.png > new file mode 100644 ok, the test seems good.
Comment on attachment 48705 [details] opentype_detection_v4_pixel_test_added ok.
Comment on attachment 48705 [details] opentype_detection_v4_pixel_test_added Clearing flags on attachment: 48705 Committed r54855: <http://trac.webkit.org/changeset/54855>
All reviewed patches have been landed. Closing bug.