visit http://www.scribd.com/documents/30964170/Scribd-in-HTML5, and read the document in the html5 reader. the fonts are all mis-placed and not being displayed correctly due to not loading the web fonts appropriately
Created attachment 55715 [details] a small test case I am tyring to use to reproduce the problem
There are two issues here. First, WebKitGTK+ does not even try to download the fonts - it always thinks it has the 'â' font that is specified by scribd as the prefferred local font. I have a fix for this. Even after getting WebKitGTK+ to actually download the fonts, they are not being applied, though.
Created attachment 56741 [details] proposed fix
Attachment 56741 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1 WebCore/platform/graphics/cairo/FontCacheCairo.cpp:106: Use 0 instead of NULL. [readability/null] [5] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 56741 [details] proposed fix I suppose this is very difficult to test without pixel tests? Looks good to me in any case, assuming we don't regress somewhere else :) r=me with the style nit fixed.
Created attachment 56913 [details] second try After running all the tests I found a number of cases where we would have to regenerate results, and even a few failures caused by changes in the sizes of fonts (the new code would fallback all the way to Times New Roman, instead of matching with FC immediately, as was done before). What this means is essentially adding more special cases, for well-known font names the tests expect. I decided to not do that for the japanese font names, though, since the test result is a passing result, I decided to just generate a new result.
Comment on attachment 56913 [details] second try >+ // Handle generic family types specially, because fontconfig does not know them, but we have >+ // code to fallback correctly in our platform data implementation. Times New Roman is treated >+ // specially, as it is used as last fallback. >+ if (!family.length() || family.startsWith("-webkit-") >+ || (fontDescription.genericFamily() != FontDescription::NoFamily) >+ || isWellKnownFontName(family) || (family == "Times New Roman")) >+ return new FontPlatformData(fontDescription, family); Not sure if it makes sense to not have Time News Roman in the well known list at this point. >+ >+ // First check the font exists. >+ CString familyNameString = family.string().utf8(); >+ const char* fcfamily = familyNameString.data(); >+ >+ FcPattern* pattern = FcPatternCreate(); >+ if (!FcPatternAddString(pattern, FC_FAMILY, reinterpret_cast<const FcChar8*>(fcfamily))) { >+ FcPatternDestroy(pattern); >+ return 0; >+ } >+ >+ FcConfigSubstitute(0, pattern, FcMatchPattern); >+ FcDefaultSubstitute(pattern); >+ >+ FcObjectSet* objectSet = FcObjectSetCreate(); >+ if (!FcObjectSetAdd(objectSet, FC_FAMILY)) { >+ FcPatternDestroy(pattern); >+ FcObjectSetDestroy(objectSet); >+ return 0; >+ } >+ >+ FcFontSet* fontSet = FcFontList(0, pattern, objectSet); >+ FcPatternDestroy(pattern); >+ FcObjectSetDestroy(objectSet); >+ >+ if (!fontSet) >+ return 0; >+ >+ if (!fontSet->fonts) { >+ FcFontSetDestroy(fontSet); >+ return 0; >+ } >+ >+ FcFontSetDestroy(fontSet); >+ > return new FontPlatformData(fontDescription, family); Most of the code here is just freeing a couple of data structures all the time, guess we should just use GOwnPtr. Looks good other than that.
Created attachment 57542 [details] third time is the charm Address Xan's comments - added GOwnPtr infrastructure for Fc types, and moved Times New Roman to the well known font name handling.
Comment on attachment 56741 [details] proposed fix Cleared Xan Lopez's review+ from obsolete attachment 56741 [details] so that this bug does not appear in http://webkit.org/pending-commit.
Landed as r60793.
Oops, this bug should remain open - we only fixed the first problem.
Comment on attachment 57542 [details] third time is the charm Clearing review flag.
Created attachment 58720 [details] special-case mono and sans as well Shaun complained that yelp started using a serif font even though he was specifying 'sans'. The problem is this alias is not being special-cased.
Comment on attachment 58720 [details] special-case mono and sans as well doh
Created attachment 58776 [details] remove restrictions from font matching Trying the new code with a few new fonts, I noticed it was not matching them. Playing around with a test program I found that the default substitutions end up being too restrictive, and are not required to exclude fonts that do not exist, so this patch removes those.
Comment on attachment 58720 [details] special-case mono and sans as well OK.
Comment on attachment 58776 [details] remove restrictions from font matching OK, fc is weird.
Comment on attachment 58720 [details] special-case mono and sans as well Landed as r61701.
Comment on attachment 58776 [details] remove restrictions from font matching Clearing flags on attachment: 58776 Committed r61703: <http://trac.webkit.org/changeset/61703>
All reviewed patches have been landed. Closing bug.
I'll open a different bug to track the other problem, I'll post the URL here.
https://bugs.webkit.org/show_bug.cgi?id=41091