[Webkit-unassigned] [Bug 71445] [GTK] [WK2] ttf-liberation fonts moved to a new location (in Debian)

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=71445


Martin Robinson <mrobinson at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #113446|review?                     |review+
               Flag|                            |




--- Comment #2 from Martin Robinson <mrobinson at webkit.org>  2011-11-03 02:53:12 PST ---
(From update of attachment 113446)
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. :/

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.