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

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Philippe Normand]

+++ This bug was initially created as a clone of Bug #71359 +++

The fonts-liberation package (previously ttf-liberation) installs the fonts to /usr/share/fonts/truetype/liberation

We already added a case for Fedora font paths one year ago. For each font filename there were 2 possible paths. Adding a new search path involves adding a new dimension in the fontPaths variable in initializeFonts(). This is rather painful, I'd propose to have two separate arrays, one for the directories and one for the font filenames we need to register in fontconfig.

While this involves a speed penalization I think it's easier to maintain.

[Philippe Normand]

Created attachment 113446 [details]
proposed patch

[Martin Robinson]

Comment on attachment 113446 [details]
proposed patch

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. :/

[Philippe Normand]

(In reply to comment #2)
> (From update of attachment 113446 [details])
> 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?
> 

Hum right, good point! Will fix it in DRT as well.

> > 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. :/

Damn me! :/ Will fix in DRT as well. Thanks! (BTW it's a pity to have to copy/paste code between WKTR and DRT :()

[Philippe Normand]

Committed r99158: <http://trac.webkit.org/changeset/99158>