Bug 147018

Summary:

Web Inspector: [Freetype] Allow inspector to retrieve a list of system fonts

Product:

WebKit

Reporter:

Devin Rousso <hi>

Component:

Web Inspector

Assignee:

Michael Catanzaro <mcatanzaro>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

cgarcia, clopez, commit-queue, graouts, joepeck, jonowells, mattbaker, mcatanzaro, mmaxfield, mrobinson, nvasilyev, timothy, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Patch	none

Devin Rousso

Reported 2015-07-16 14:34:32 PDT

This will be really useful for a font picker and for autocompletion. See https://bugs.webkit.org/show_bug.cgi?id=147009 for more details.

Attachments
Patch (7.82 KB, patch) 2015-07-28 19:49 PDT, Michael Catanzaro	no flags	Details Formatted Diff Diff
Patch (7.67 KB, patch) 2015-07-28 19:55 PDT, Michael Catanzaro	no flags	Details Formatted Diff Diff
Patch (7.69 KB, patch) 2015-07-29 08:37 PDT, Michael Catanzaro	no flags	Details Formatted Diff Diff
Patch (8.24 KB, patch) 2015-07-29 18:49 PDT, Michael Catanzaro	no flags	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Radar WebKit Bug Importer

Comment 1 2015-07-16 14:34:55 PDT

<rdar://problem/21863018>

Michael Catanzaro

Comment 2 2015-07-16 15:39:30 PDT

I think we'll need to use FcFontList: """ FcFontSet * FcFontList(FcConfig *config, FcPattern *p, FcObjectSet *os); Selects fonts matching p, creates patterns from those fonts containing only the objects in os and returns the set of unique such patterns. If config is NULL, the default configuration is checked to be up to date, and used. """ Maybe someone with more endurance for fontconfig could translate that to English....

Michael Catanzaro

Comment 3 2015-07-16 15:39:52 PDT

Forgot the link: http://www.freedesktop.org/software/fontconfig/fontconfig-devel/fcfontlist.html

Michael Catanzaro

Comment 4 2015-07-28 19:49:10 PDT

Created attachment 257724 [details] Patch

Michael Catanzaro

Comment 5 2015-07-28 19:55:31 PDT

Created attachment 257725 [details] Patch

Carlos Garcia Campos

Comment 6 2015-07-28 22:58:38 PDT

Comment on attachment 257725 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=257725&action=review > Source/WebCore/platform/graphics/freetype/FontCacheFreeType.cpp:105 > +static Vector<String> patternToFamilies(const FcPattern& pattern) > +{ > + char* patternChars = reinterpret_cast<char*>(FcPatternFormat( > + const_cast<FcPattern*>(&pattern), > + reinterpret_cast<const FcChar8*>("%{family}"))); I think it's more convenient to pass the pointer here, since it's what you need and you don't need more ugly casts. This is a private static function after all.

Michael Catanzaro

Comment 7 2015-07-29 07:42:43 PDT

(In reply to comment #6) > I think it's more convenient to pass the pointer here, since it's what you > need and you don't need more ugly casts. This is a private static function > after all. Yeah... I will just remove the const though, then no cast is needed.

Michael Catanzaro

Comment 8 2015-07-29 08:37:40 PDT

Created attachment 257748 [details] Patch

Brian Burg

Comment 9 2015-07-29 13:12:43 PDT

Comment on attachment 257748 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=257748&action=review The LayoutTests changes look good to me, with a small suggestion. > LayoutTests/inspector/css/get-system-fonts.html:6 > +// Testing that we can get the fonts on the system... Please move the test description to inside <body>. You might want to improve the wording, i.e., "Tests that the Web Inspector can enumerate system fonts, and checks for the existence of common fonts." > LayoutTests/inspector/css/get-system-fonts.html:16 > InspectorTest.log("Has at least 5 fonts: " + (fontFamilyNames.length >= 5)); This should really be an assertion: InspectorTest.assert(cond, message). But you'd have to rebaseline the text for other platforms.

Michael Catanzaro

Comment 10 2015-07-29 17:45:27 PDT

(In reply to comment #9) > > LayoutTests/inspector/css/get-system-fonts.html:6 > > +// Testing that we can get the fonts on the system... > > Please move the test description to inside <body>. You might want to improve > the wording, i.e., "Tests that the Web Inspector can enumerate system fonts, > and checks for the existence of common fonts." OK > > LayoutTests/inspector/css/get-system-fonts.html:16 > > InspectorTest.log("Has at least 5 fonts: " + (fontFamilyNames.length >= 5)); > > This should really be an assertion: InspectorTest.assert(cond, message). But > you'd have to rebaseline the text for other platforms. OK

Michael Catanzaro

Comment 11 2015-07-29 18:49:56 PDT

Created attachment 257798 [details] Patch

Carlos Garcia Campos

Comment 12 2015-07-29 23:09:08 PDT

Comment on attachment 257798 [details] Patch Ok.

WebKit Commit Bot

Comment 13 2015-07-30 07:51:16 PDT

Comment on attachment 257798 [details] Patch Clearing flags on attachment: 257798 Committed r187583: <http://trac.webkit.org/changeset/187583>

WebKit Commit Bot

Comment 14 2015-07-30 07:51:21 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.