Now that we have jhbuild in place, I propose to use the fonts that we download using jhbuild, instead of system provided ones. This should further help to get stable test results across machines.
Created attachment 132569 [details] Use downloaded fonts Quick patch, feedback on how to determine the right path is welcome. Also, if this change is accepted in general, I think we can get rid of the getFontDirectories() function.
Looks fine to me if it passes the EWS checks. We should pass the path to DOWNLOADED_FONTS_DIR in a more flexible way in 81475.
Created attachment 132834 [details] Diff of results before and after font change applied, r111388 Fixes about 60 cases, makes three different ones fail, as it seems. I think we should apply this. I will upload a new version of the patch that simplifies FontManagement.cpp.
Created attachment 132995 [details] Use downloaded fonts, simplify FontManagement.cpp
Comment on attachment 132995 [details] Use downloaded fonts, simplify FontManagement.cpp View in context: https://bugs.webkit.org/attachment.cgi?id=132995&action=review > Tools/DumpRenderTree/efl/FontManagement.cpp:56 > +static bool addFontDirectory(const String& fontDirectory, FcConfig* config) In my opinion, we should not use WebKit utilities in order to avoid WebKit dependency. However, it seems this is ok for now.
(In reply to comment #5) > (From update of attachment 132995 [details]) > In my opinion, we should not use WebKit utilities in order to avoid WebKit dependency. However, it seems this is ok for now. Alright - thanks. Let's handle this concern separately in bug 80683 since I am not adding/changing any class usage dependency.
Comment on attachment 132995 [details] Use downloaded fonts, simplify FontManagement.cpp Looks awesome, thanks.
Comment on attachment 132995 [details] Use downloaded fonts, simplify FontManagement.cpp View in context: https://bugs.webkit.org/attachment.cgi?id=132995&action=review Looks good, but I have a concern about using the system filesystem encoding. > Tools/DumpRenderTree/efl/FontManagement.cpp:62 > + if (!ecore_file_is_dir(path) > + || !FcConfigAppFontAddDir(config, reinterpret_cast<const FcChar8*>(path))) { Do these functions accept paths in UTF-8 or in the system encoding? If it's in the system encoding you need to convert them from UTF-8 first.
Comment on attachment 132995 [details] Use downloaded fonts, simplify FontManagement.cpp View in context: https://bugs.webkit.org/attachment.cgi?id=132995&action=review > Tools/DumpRenderTree/efl/FontManagement.cpp:93 > - if (!addFontDirectories(getFontDirectories(), config)) { > + if (!addFontDirectory(String(DOWNLOADED_FONTS_DIR), config)) { You can avoid this problem entirely simply by making the argument to addFontDirectories a CString.
Created attachment 133099 [details] Use downloaded fonts, simplify FontManagement.cpp, CString Review comment about utf8 vs system locale addressed by using CString.
Comment on attachment 133099 [details] Use downloaded fonts, simplify FontManagement.cpp, CString Clearing flags on attachment: 133099 Committed r111598: <http://trac.webkit.org/changeset/111598>
All reviewed patches have been landed. Closing bug.