Summary: | Web Inspector: [iOS] Allow inspector to retrieve a list of system fonts | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Devin Rousso <hi> | ||||||
Component: | Web Inspector | Assignee: | Devin Rousso <hi> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | commit-queue, graouts, joepeck, jonowells, mattbaker, mmaxfield, nvasilyev, timothy, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
Attachments: |
|
Description
Devin Rousso
2015-07-16 23:51:57 PDT
Created attachment 257985 [details]
Patch
Comment on attachment 257985 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=257985&action=review r=me, but should probably get a nod from Myles. > Source/WebCore/platform/graphics/ios/FontCacheIOS.mm:471 > + NSDictionary *attributes = @{ }; > + RetainPtr<CTFontDescriptorRef> fontDescriptor = adoptCF(CTFontDescriptorCreateWithAttributes((CFDictionaryRef)attributes)); You can just inline this dictionary. > Source/WebCore/platform/graphics/ios/FontCacheIOS.mm:472 > + RetainPtr<NSArray> matchedDescriptors = adoptNS((NSArray *)CTFontDescriptorCreateMatchingFontDescriptors(fontDescriptor.get(), nullptr)); Nit: static_cast. Also, both of these can be "auto", the type is relatively easy to determine. > Source/WebCore/platform/graphics/ios/FontCacheIOS.mm:474 > + NSInteger numMatches = [matchedDescriptors.get() count]; Style: I actually think the ".git()" is unnecessary here. It should just work without it. > Source/WebCore/platform/graphics/ios/FontCacheIOS.mm:480 > + RetainPtr<CFStringRef> familyName = adoptCF(static_cast<CFStringRef>(CTFontDescriptorCopyAttribute((CTFontDescriptorRef)[matchedDescriptors.get() objectAtIndex:i], kCTFontFamilyNameAttribute))); Nit: static_cast. You can split this out into a couple lines to be more readable. Could also be auto given the CFStringRef cast will be on the line anyways. Comment on attachment 257985 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=257985&action=review >> Source/WebCore/platform/graphics/ios/FontCacheIOS.mm:480 >> + RetainPtr<CFStringRef> familyName = adoptCF(static_cast<CFStringRef>(CTFontDescriptorCopyAttribute((CTFontDescriptorRef)[matchedDescriptors.get() objectAtIndex:i], kCTFontFamilyNameAttribute))); > > Nit: static_cast. You can split this out into a couple lines to be more readable. Could also be auto given the CFStringRef cast will be on the line anyways. Why are you using CF* here when everywhere else you are using NS*? > Source/WebCore/platform/graphics/ios/FontCacheIOS.mm:488 > + if (familyName.startsWith('.')) Please move this check into the loop above. > Source/WebCore/platform/graphics/ios/FontCacheIOS.mm:489 > + continue; I'm not sure this line will ever be hit. CT may not ever return these period-prefixed font names. It's worth checking (even on older operating systems) Created attachment 258558 [details]
Patch
Comment on attachment 258558 [details]
Patch
r=me
Comment on attachment 258558 [details] Patch Clearing flags on attachment: 258558 Committed r188227: <http://trac.webkit.org/changeset/188227> All reviewed patches have been landed. Closing bug. |