RESOLVED FIXED Bug 42782
WebKitTestRunner needs to support loading custom fonts (via the WEBKIT_TESTFONTS environment variable)
https://bugs.webkit.org/show_bug.cgi?id=42782
Summary WebKitTestRunner needs to support loading custom fonts (via the WEBKIT_TESTFO...
Adam Roben (:aroben)
Reported 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.
Attachments
Patch (4.45 KB, patch)
2010-08-03 02:41 PDT, Jon Honeycutt
aroben: review+
Adam Roben (:aroben)
Comment 1 2010-07-21 13:30:46 PDT
Adam Roben (:aroben)
Comment 2 2010-07-28 13:09:44 PDT
Working on this now.
Adam Roben (:aroben)
Comment 3 2010-07-28 13:18:10 PDT
Jon is working on this, actually.
Jon Honeycutt
Comment 4 2010-08-03 02:41:13 PDT
Adam Roben (:aroben)
Comment 5 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
Jon Honeycutt
Comment 6 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!
Jon Honeycutt
Comment 7 2010-08-03 15:57:14 PDT
Landed in r64601.
Note You need to log in before you can comment on or make changes to this bug.