In ubuntu 12.04, the run-webkit-tests in chromium can't work normally. The error message: You are missing /usr/share/fonts/truetype/thai/Garuda.ttf But in ubuntu12.04, the package ttf-thai-tlwg install the Garuda.ttf into /usr/share/fonts/truetype/tlwg/Garuda.ttf
Created attachment 142136 [details] Patch
Comment on attachment 142136 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=142136&action=review > Tools/DumpRenderTree/chromium/TestShellLinux.cpp:174 > + if (!strcmp(fonts[i], "/usr/share/fonts/truetype/thai/Garuda.ttf")) { > + // ttf-thai-tlwg package uses different path in Ubuntu 12.04. > + static const char* tlwg = "/usr/share/fonts/truetype/tlwg/Garuda.ttf"; > + if (access(tlwg, R_OK)) { > + fprintf(stderr, "You are missing %s or %s. Try re-running build/install-build-deps.sh. Also see " > + "http://code.google.com/p/chromium/wiki/LayoutTestsLinux", > + fonts[i], tlwg); > + exit(1); > + } > + } else { > + fprintf(stderr, "You are missing %s. Try re-running build/install-build-deps.sh. Also see " > "http://code.google.com/p/chromium/wiki/LayoutTestsLinux", > fonts[i]); > - exit(1); > + exit(1); > + } > } I'd like: - removing Garuda.ttf from 'fonts' array - introduce new function to take two arguments for paths (tahi/Garuda.ttf and tlwg/Garuda.ttf). The function checks their existence and calls FcConfigAppFontAddFile(). The following check for lohit_pa.ttf might use the function too.
(In reply to comment #2) > I'd like: > - removing Garuda.ttf from 'fonts' array > - introduce new function to take two arguments for paths (tahi/Garuda.ttf and tlwg/Garuda.ttf). The function checks their existence and calls FcConfigAppFontAddFile(). > > The following check for lohit_pa.ttf might use the function too. Thanks for your review. Yeah, I should add the "Garuda.tff" into optionalFonts. And the "Garuda.tff" is not required for all the test, some tests don't need it, such as fast/worker etc. I think it should be optional. Add the "Garuda.tff" into optionalFonts, optionFounts array should be the interface for handling the exception. It is very convenient to fix this issue. So I don't introduce a new function.
Created attachment 142175 [details] Patch
Comment on attachment 142175 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=142175&action=review > Tools/DumpRenderTree/chromium/TestShellLinux.cpp:173 > static const char* const optionalFonts[] = { > "/usr/share/fonts/truetype/ttf-indic-fonts-core/lohit_pa.ttf", > + "/usr/share/fonts/truetype/thai/Garuda.ttf", I don't want to make Garuda.ttf optional.
(In reply to comment #5) > (From update of attachment 142175 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=142175&action=review > > > Tools/DumpRenderTree/chromium/TestShellLinux.cpp:173 > > static const char* const optionalFonts[] = { > > "/usr/share/fonts/truetype/ttf-indic-fonts-core/lohit_pa.ttf", > > + "/usr/share/fonts/truetype/thai/Garuda.ttf", > > I don't want to make Garuda.ttf optional. Okay, I have updated the patch already, please have a review again. Thanks.
Created attachment 142460 [details] Patch
Attachment 142460 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Tools/ChangeLog', u'Tools/DumpRenderTree/c..." exit_code: 1 Tools/DumpRenderTree/chromium/TestShellLinux.cpp:86: Missing space before ( in if( [whitespace/parens] [5] Tools/DumpRenderTree/chromium/TestShellLinux.cpp:88: Missing space before ( in if( [whitespace/parens] [5] Total errors found: 2 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 142461 [details] Patch
Comment on attachment 142461 [details] Patch Looks good.
Comment on attachment 142461 [details] Patch Clearing flags on attachment: 142461 Committed r117446: <http://trac.webkit.org/changeset/117446>
All reviewed patches have been landed. Closing bug.