Bug 42782 - WebKitTestRunner needs to support loading custom fonts (via the WEBKIT_TESTFONTS environment variable)
Summary: WebKitTestRunner needs to support loading custom fonts (via the WEBKIT_TESTFO...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows XP
: P2 Normal
Assignee: Jon Honeycutt
URL:
Keywords: InRadar, PlatformOnly
Depends on:
Blocks: 43379
  Show dependency treegraph
 
Reported: 2010-07-21 13:30 PDT by Adam Roben (:aroben)
Modified: 2010-08-03 15:57 PDT (History)
3 users (show)

See Also:


Attachments
Patch (4.45 KB, patch)
2010-08-03 02:41 PDT, Jon Honeycutt
aroben: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Roben (:aroben) 2010-07-21 13:30:19 PDT
DumpRenderTree knows how to load custom fonts from the directory specified in the WEBKIT_TESTFONTS environment variable. We use this to use Mac fonts when running the tests.

WebKitTestRunner needs to support this, too.
Comment 1 Adam Roben (:aroben) 2010-07-21 13:30:46 PDT
<rdar://problem/8218343>
Comment 2 Adam Roben (:aroben) 2010-07-28 13:09:44 PDT
Working on this now.
Comment 3 Adam Roben (:aroben) 2010-07-28 13:18:10 PDT
Jon is working on this, actually.
Comment 4 Jon Honeycutt 2010-08-03 02:41:13 PDT
Created attachment 63309 [details]
Patch
Comment 5 Adam Roben (:aroben) 2010-08-03 04:50:55 PDT
Comment on attachment 63309 [details]
Patch

> +static const wstring& fontsPath()
> +{
> +    static wstring path;
> +    static bool initialized;
> +
> +    if (initialized)
> +        return path;
> +    initialized = true;
> +
> +    DWORD size = GetEnvironmentVariable(fontsEnvironmentVariable, 0, 0);

The style being used elsewhere in WebKit2 code is to prefix Win32 APIs with :: and to always use the W variant.

> +    Vector<TCHAR> buffer(size);

If you switch to using the W variant, this should be Vector<WCHAR>.

> +    if (!GetEnvironmentVariable(fontsEnvironmentVariable, buffer.data(), buffer.size()))
> +        return path;
> +
> +    path = buffer.data();
> +    if (path[path.length() - 1] != '\\')
> +        path.append(L"\\");
> +
> +    return path;
> +}

I guess you decided that falling back to the path to the executable when WEBKIT_TESTFONTS is undefined isn't helpful?


>  void activateFonts()
>  {
> -    // FIXME: Not implemented.
> +    static LPCTSTR fontsToInstall[] = {
> +        TEXT("AHEM____.ttf"),
> +        TEXT("Apple Chancery.ttf"),
> +        TEXT("Courier Bold.ttf"),
> +        TEXT("Courier.ttf"),
> +        TEXT("Helvetica Bold Oblique.ttf"),
> +        TEXT("Helvetica Bold.ttf"),
> +        TEXT("Helvetica Oblique.ttf"),
> +        TEXT("Helvetica.ttf"),
> +        TEXT("Helvetica Neue Bold Italic.ttf"),
> +        TEXT("Helvetica Neue Bold.ttf"),
> +        TEXT("Helvetica Neue Condensed Black.ttf"),
> +        TEXT("Helvetica Neue Condensed Bold.ttf"),
> +        TEXT("Helvetica Neue Italic.ttf"),
> +        TEXT("Helvetica Neue Light Italic.ttf"),
> +        TEXT("Helvetica Neue Light.ttf"),
> +        TEXT("Helvetica Neue UltraLight Italic.ttf"),
> +        TEXT("Helvetica Neue UltraLight.ttf"),
> +        TEXT("Helvetica Neue.ttf"),
> +        TEXT("Lucida Grande.ttf"),
> +        TEXT("Lucida Grande Bold.ttf"),
> +        TEXT("Monaco.ttf"),
> +        TEXT("Papyrus.ttf"),
> +        TEXT("Times Bold Italic.ttf"),
> +        TEXT("Times Bold.ttf"),
> +        TEXT("Times Italic.ttf"),
> +        TEXT("Times Roman.ttf"),
> +        TEXT("WebKit Layout Tests 2.ttf"),
> +        TEXT("WebKit Layout Tests.ttf"),
> +        TEXT("WebKitWeightWatcher100.ttf"),
> +        TEXT("WebKitWeightWatcher200.ttf"),
> +        TEXT("WebKitWeightWatcher300.ttf"),
> +        TEXT("WebKitWeightWatcher400.ttf"),
> +        TEXT("WebKitWeightWatcher500.ttf"),
> +        TEXT("WebKitWeightWatcher600.ttf"),
> +        TEXT("WebKitWeightWatcher700.ttf"),
> +        TEXT("WebKitWeightWatcher800.ttf"),
> +        TEXT("WebKitWeightWatcher900.ttf")
> +    };
> +
> +    wstring resourcesPath = fontsPath();
> +
> +    for (unsigned i = 0; i < ARRAYSIZE(fontsToInstall); ++i)
> +        AddFontResourceEx(wstring(resourcesPath + fontsToInstall[i]).c_str(), FR_PRIVATE, 0);

If you switch to the W variant, you should use an LPCWSTR array.

r=me
Comment 6 Jon Honeycutt 2010-08-03 14:53:25 PDT
(In reply to comment #5)
> (From update of attachment 63309 [details])
> > +static const wstring& fontsPath()
> > +{
> > +    static wstring path;
> > +    static bool initialized;
> > +
> > +    if (initialized)
> > +        return path;
> > +    initialized = true;
> > +
> > +    DWORD size = GetEnvironmentVariable(fontsEnvironmentVariable, 0, 0);
> 
> The style being used elsewhere in WebKit2 code is to prefix Win32 APIs with :: and to always use the W variant.

Fixed.

> 
> > +    Vector<TCHAR> buffer(size);
> 
> If you switch to using the W variant, this should be Vector<WCHAR>.

Fixed.

> 
> > +    if (!GetEnvironmentVariable(fontsEnvironmentVariable, buffer.data(), buffer.size()))
> > +        return path;
> > +
> > +    path = buffer.data();
> > +    if (path[path.length() - 1] != '\\')
> > +        path.append(L"\\");
> > +
> > +    return path;
> > +}
> 
> I guess you decided that falling back to the path to the executable when WEBKIT_TESTFONTS is undefined isn't helpful?

Yes - it doesn't appear that we create this directory anymore, and old-run-webkit-tests requires that WEBKIT_TESTFONTS be set.

> 
> 
> >  void activateFonts()
> >  {
> > -    // FIXME: Not implemented.
> > +    static LPCTSTR fontsToInstall[] = {
> > +        TEXT("AHEM____.ttf"),
> > +        TEXT("Apple Chancery.ttf"),
> > +        TEXT("Courier Bold.ttf"),
> > +        TEXT("Courier.ttf"),
> > +        TEXT("Helvetica Bold Oblique.ttf"),
> > +        TEXT("Helvetica Bold.ttf"),
> > +        TEXT("Helvetica Oblique.ttf"),
> > +        TEXT("Helvetica.ttf"),
> > +        TEXT("Helvetica Neue Bold Italic.ttf"),
> > +        TEXT("Helvetica Neue Bold.ttf"),
> > +        TEXT("Helvetica Neue Condensed Black.ttf"),
> > +        TEXT("Helvetica Neue Condensed Bold.ttf"),
> > +        TEXT("Helvetica Neue Italic.ttf"),
> > +        TEXT("Helvetica Neue Light Italic.ttf"),
> > +        TEXT("Helvetica Neue Light.ttf"),
> > +        TEXT("Helvetica Neue UltraLight Italic.ttf"),
> > +        TEXT("Helvetica Neue UltraLight.ttf"),
> > +        TEXT("Helvetica Neue.ttf"),
> > +        TEXT("Lucida Grande.ttf"),
> > +        TEXT("Lucida Grande Bold.ttf"),
> > +        TEXT("Monaco.ttf"),
> > +        TEXT("Papyrus.ttf"),
> > +        TEXT("Times Bold Italic.ttf"),
> > +        TEXT("Times Bold.ttf"),
> > +        TEXT("Times Italic.ttf"),
> > +        TEXT("Times Roman.ttf"),
> > +        TEXT("WebKit Layout Tests 2.ttf"),
> > +        TEXT("WebKit Layout Tests.ttf"),
> > +        TEXT("WebKitWeightWatcher100.ttf"),
> > +        TEXT("WebKitWeightWatcher200.ttf"),
> > +        TEXT("WebKitWeightWatcher300.ttf"),
> > +        TEXT("WebKitWeightWatcher400.ttf"),
> > +        TEXT("WebKitWeightWatcher500.ttf"),
> > +        TEXT("WebKitWeightWatcher600.ttf"),
> > +        TEXT("WebKitWeightWatcher700.ttf"),
> > +        TEXT("WebKitWeightWatcher800.ttf"),
> > +        TEXT("WebKitWeightWatcher900.ttf")
> > +    };
> > +
> > +    wstring resourcesPath = fontsPath();
> > +
> > +    for (unsigned i = 0; i < ARRAYSIZE(fontsToInstall); ++i)
> > +        AddFontResourceEx(wstring(resourcesPath + fontsToInstall[i]).c_str(), FR_PRIVATE, 0);
> 
> If you switch to the W variant, you should use an LPCWSTR array.

Fixed.

> 
> r=me

Thanks!
Comment 7 Jon Honeycutt 2010-08-03 15:57:14 PDT
Landed in r64601.