[webkit-reviews] review granted: [Bug 177755] [GTK] Support the "system" CSS font family : [Attachment 322423] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Oct 3 00:19:46 PDT 2017
Carlos Garcia Campos <cgarcia at igalia.com> has granted Adrian Perez
<aperez at igalia.com>'s request for review:
Bug 177755: [GTK] Support the "system" CSS font family
https://bugs.webkit.org/show_bug.cgi?id=177755
Attachment 322423: Patch
https://bugs.webkit.org/attachment.cgi?id=322423&action=review
--- Comment #5 from Carlos Garcia Campos <cgarcia at igalia.com> ---
Comment on attachment 322423
--> https://bugs.webkit.org/attachment.cgi?id=322423
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=322423&action=review
> Source/WebCore/platform/graphics/freetype/FontCacheFreeType.cpp:333
> +#if PLATFORM(GTK)
> + || equalLettersIgnoringASCIICase(familyNameString,
"-webkit-system-font")
> + || equalLettersIgnoringASCIICase(familyNameString,
"-webkit-system-ui")
> +#endif
You can mode the ifdefed code to the beginning, since the order doesn't matter,
right?
> Source/WebCore/platform/graphics/gtk/GdkCairoUtilities.cpp:61
> +String getDefaultGtkSystemUiFont()
I would call this just defaultGtkSystemFont().
> Source/WebCore/platform/graphics/gtk/GdkCairoUtilities.cpp:67
> + auto spaceChar = strrchr(fontString.get(), ' ');
auto*
> Source/WebCore/platform/graphics/gtk/GdkCairoUtilities.cpp:68
> + return String::fromUTF8(fontString.get(), spaceChar - fontString.get());
I know we are always expecting a valid font name - space - size string, but I
would either assert if that's not the case or handling that case, I don't know
what happens if the user sets an an empty string in the dconf setting, for
example. Since we have a duplicated string, maybe it's easier to set '\0' in
the place of the last space (if found), and then just use String::fromUTF8().
More information about the webkit-reviews
mailing list