RESOLVED FIXED 110602
[GTK] DumpRenderTree prints out suggesting to set WEBKIT_TOP_LEVEL when fonts are missing but doesn't use it.
https://bugs.webkit.org/show_bug.cgi?id=110602
Summary [GTK] DumpRenderTree prints out suggesting to set WEBKIT_TOP_LEVEL when fonts...
Claudio Saavedra
Reported 2013-02-22 06:07:17 PST
This is the only usage of WEBKIT_TOP_LEVEL I can see: if (!g_getenv("WEBKIT_TOP_LEVEL")) g_setenv("WEBKIT_TOP_LEVEL", TOP_LEVEL_DIR, FALSE); And later on: g_error("Could not locate test fonts at %s. Is WEBKIT_TOP_LEVEL set?", fontsPath.data()); Unless I am grossly overlooking something, this makes no sense.
Attachments
Patch (1.94 KB, patch)
2013-12-09 03:12 PST, Brendan Long
no flags
Patch (2.12 KB, patch)
2013-12-09 03:26 PST, Brendan Long
no flags
Brendan Long
Comment 1 2013-10-07 15:07:01 PDT
TOP_LEVEL_DIR is used in getTopLevelPath: Tools/DumpRenderTree/gtk/DumpRenderTree.cpp-CString getTopLevelPath() Tools/DumpRenderTree/gtk/DumpRenderTree.cpp-{ Tools/DumpRenderTree/gtk/DumpRenderTree.cpp- if (!g_getenv("WEBKIT_TOP_LEVEL")) Tools/DumpRenderTree/gtk/DumpRenderTree.cpp: g_setenv("WEBKIT_TOP_LEVEL", TOP_LEVEL_DIR, FALSE); Tools/DumpRenderTree/gtk/DumpRenderTree.cpp- Tools/DumpRenderTree/gtk/DumpRenderTree.cpp: return TOP_LEVEL_DIR; Tools/DumpRenderTree/gtk/DumpRenderTree.cpp-} The error you're getting is probably because WebKitGTK expects this repo: https://github.com/mrobinson/webkitgtk-test-fonts To be checked out in webkit/WebKitBuild/Dependencies/Root
Zan Dobersek
Comment 2 2013-10-08 00:33:31 PDT
WEBKIT_TOP_LEVEL is used in WebCore to load image resources from the trunk (rather than from the installation location) when running DRT: http://trac.webkit.org/browser/trunk/Source/WebCore/platform/graphics/gtk/ImageGtk.cpp#L39
Claudio Saavedra
Comment 3 2013-10-08 01:08:16 PDT
(In reply to comment #2) > WEBKIT_TOP_LEVEL is used in WebCore to load image resources from the trunk (rather than from the installation location) when running DRT: > http://trac.webkit.org/browser/trunk/Source/WebCore/platform/graphics/gtk/ImageGtk.cpp#L39 Not used for fonts I guess. It's misleading. Retitling.
Brendan Long
Comment 4 2013-12-09 03:12:45 PST
Brendan Long
Comment 5 2013-12-09 03:14:23 PST
This makes WEBKIT_TOP_LEVEL work again, using the environment variable if available, and the build-time constant if not. I also removed the printing of the font path in the debug message, since it's guaranteed to be null (see the if statement right before).
Brendan Long
Comment 6 2013-12-09 03:26:49 PST
Created attachment 218746 [details] Patch Actually, it's probably better to make this error message more useful, so now it prints a description of where the fonts should be installed and describes what WEBKIT_TOP_LEVEL is.
Brendan Long
Comment 7 2014-01-14 15:28:01 PST
Anyone want to review this?
WebKit Commit Bot
Comment 8 2014-01-26 21:17:16 PST
Comment on attachment 218746 [details] Patch Clearing flags on attachment: 218746 Committed r162823: <http://trac.webkit.org/changeset/162823>
WebKit Commit Bot
Comment 9 2014-01-26 21:17:19 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.