Bug 89437

Summary:

[GTK] Read fonts path when running layout tests from alternative fonts dir when main dir doesn't exist

Product:

WebKit

Reporter:

Carlos Garcia Campos <cgarcia>

Component:

Tools / Tests

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abarth, dpranke, gustavo, mrobinson, ojan, pnormand, webkit.review.bot

Priority:

Keywords:

Gtk

Version:

528+ (Nightly build)

Hardware:

OS:

Linux

Attachments:

Description	Flags
Patch	none
Updated patch	mrobinson: review-
Updated patch	mrobinson: review+

Description Carlos Garcia Campos 2012-06-18 23:50:43 PDT

This would allow to use DRT with the fonts installed in any place.

Comment 1 Carlos Garcia Campos 2012-06-18 23:53:57 PDT

Created attachment 148267 [details]
Patch

Comment 2 Philippe Normand 2012-06-20 13:50:58 PDT

Comment on attachment 148267 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=148267&action=review

Looks good. Can you please also update the relevant wiki page? Thanks!

> Tools/Scripts/webkitpy/layout_tests/port/gtk.py:68
> +        if os.environ.get('WEBKIT_TEST_FONTS_PATH'):
> +            environment['WEBKIT_TEST_FONTS_PATH'] = os.environ['WEBKIT_TEST_FONTS_PATH']

This can be one line
environment['WEBKIT_TEST_FONTS_PATH'] = os.environ.get('WEBKIT_TEST_FONTS_PATH','')

Comment 3 Martin Robinson 2012-06-20 13:57:44 PDT

I think Carlos and I agreed that instead of an environment variable we should just look for fonts in an alternate location.

Comment 4 Philippe Normand 2012-06-20 14:00:12 PDT

Oh, ok I didn't know about that. Might have been a good idea to comment on the bug and/or pull the patch out :)

Comment 5 Carlos Garcia Campos 2012-07-02 03:38:23 PDT

Created attachment 150391 [details]
Updated patch

Use an alternative directory instead of an env var as suggested by Martin.

Comment 6 Martin Robinson 2012-07-02 07:37:48 PDT

Comment on attachment 150391 [details]
Updated patch

View in context: https://bugs.webkit.org/attachment.cgi?id=150391&action=review

This patch looks great, except that it should probably also deal with WebKitTestRunner in Tools/WebKitTestRunner/InjectedBundle/gtk/ActivateFontsGtk.cpp.

> Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:176
> +    const char* webkitOutputDir = g_getenv("WEBKITOUTPUTDIR");

Perhaps you could do:

GOwnPtr<char> defaultOutputDir = g_build_filename(getTopLevelPath().data(), "WebKitBuild");
const char* outputDirFromEnvironment = g_getenv("WEBKITOUTPUTDIR");
const char* webkitOutputDir = outputDirFromEnvironment ? outputDirFromEnvironment : topLevelPath.get();

and avoid the duplication below?

Comment 7 Carlos Garcia Campos 2012-07-02 08:49:51 PDT

Created attachment 150430 [details]
Updated patch

Comment 8 Martin Robinson 2012-07-02 08:54:17 PDT

Comment on attachment 150430 [details]
Updated patch

Thanks!

Comment 9 Carlos Garcia Campos 2012-07-02 09:11:08 PDT

Committed r121684: <http://trac.webkit.org/changeset/121684>