[webkit-reviews] review denied: [Bug 55036] [Qt] FontCache::createFontPlatformData() is broken, a default font is returned even if the family does not match : [Attachment 103235] Patch

[bugzilla-daemon at webkit.org]

Andreas Kling <kling at webkit.org> has denied Pierre Rossi
<pierre.rossi at gmail.com>'s request for review:
Bug 55036: [Qt] FontCache::createFontPlatformData() is broken, a default font
is returned even if the family does not match
https://bugs.webkit.org/show_bug.cgi?id=55036

Attachment 103235: Patch
https://bugs.webkit.org/attachment.cgi?id=103235&action=review

------- Additional Comments from Andreas Kling <kling at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=103235&action=review


r- to get some more polish up in here. :)

> Source/WebCore/ChangeLog:3
> +	   Fix the QFont fallback issue.

That could mean a lot of things. The bug title would fit very nicely here
instead.

> Source/WebCore/ChangeLog:12
> +	   This is a showstopper to get webfonts working
> +	   properly in QtWebKit.

This is bug tracker talk, and can be excluded from the ChangeLog.

> Source/WebCore/ChangeLog:18
> +	   This change should be covered by existing tests,
> +	   but there's a good chance some of these tests
> +	   will in turn need to be fixed.

We should strive to be more certain in ChangeLog entries.
Run the tests with your change and find out what breaks, then (if applicable)
include the updated qt-4.8 baselines in the patch.

> Source/WebCore/platform/graphics/qt/FontCacheQt.cpp:114
> +#if QT_VERSION >= QT_VERSION_CHECK(4, 8, 0)
> +    QFontDatabase db;
> +    if (!db.hasFamily(familyName))
> +	   return 0;
> +#endif

This looks good to me. Is the API available on the Qt-4.8 used by our bots
though?