RESOLVED FIXED 147018
Web Inspector: [Freetype] Allow inspector to retrieve a list of system fonts 
https://bugs.webkit.org/show_bug.cgi?id=147018
Summary Web Inspector: [Freetype] Allow inspector to retrieve a list of system fonts
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
DetailsFormatted DiffDiff
Patch (7.67 KB, patch)
2015-07-28 19:55 PDT, Michael Catanzaro 		no flags
DetailsFormatted DiffDiff
Patch (7.69 KB, patch)
2015-07-29 08:37 PDT, Michael Catanzaro 		no flags
DetailsFormatted DiffDiff
Patch (8.24 KB, patch)
2015-07-29 18:49 PDT, Michael Catanzaro 		no flags
DetailsFormatted DiffDiff
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.