[webkit-reviews] review granted: [Bug 71445] [GTK] [WK2] ttf-liberation fonts moved to a new location (in Debian) : [Attachment 113446] proposed patch

[bugzilla-daemon at webkit.org]

Martin Robinson <mrobinson at webkit.org> has granted Philippe Normand
<pnormand at igalia.com>'s request for review:
Bug 71445: [GTK] [WK2] ttf-liberation fonts moved to a new location (in Debian)
https://bugs.webkit.org/show_bug.cgi?id=71445

Attachment 113446: proposed patch
https://bugs.webkit.org/attachment.cgi?id=113446&action=review

------- Additional Comments from Martin Robinson <mrobinson at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=113446&action=review


Thanks for the fix! Please fix the nits below before landing.

> Tools/WebKitTestRunner/InjectedBundle/gtk/ActivateFontsGtk.cpp:109
> +    GOwnPtr<gchar> directoriesDescription;
> +    for (size_t path = 0; path < G_N_ELEMENTS(fontDirectories); path++)
> +	   directoriesDescription.set(g_strjoin(":",
directoriesDescription.release(), fontDirectories[path], NULL));

Perhaps only calculate this if found is false below?

> Tools/WebKitTestRunner/InjectedBundle/gtk/ActivateFontsGtk.cpp:128
> +	   if (!found)
> +	       g_error("Could not find font %s in %s. Either install this font
or file a bug "
>		       "at http://bugs.webkit.org if it is installed in another
location.",

The curly brace should stay here because the g_error take multiple lines. This
rule applies to the number of lines and not the number of statements. :/