| Summary: | [OS X] Remove dead code from FontCache::systemFallbackForCharacters() | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Myles C. Maxfield <mmaxfield> | ||||||
| Component: | New Bugs | Assignee: | Myles C. Maxfield <mmaxfield> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | dbates | ||||||
| Priority: | P2 | ||||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Myles C. Maxfield
2015-08-20 01:45:36 PDT
Created attachment 259460 [details]
Patch
Comment on attachment 259460 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=259460&action=review > Source/WebCore/platform/graphics/mac/FontCacheMac.mm:-565 > - if (NSFont *bestVariation = [fontManager fontWithFamily:[substituteFont familyName] traits:traits weight:weight size:size]) { No need for the "size" variable Created attachment 259567 [details]
Patch
Comment on attachment 259567 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=259567&action=review > Source/WebCore/ChangeLog:8 > + CTFontCreateForCharactersWithLanguage() will always return the best For your consideration, I suggest writing this sentence in terms of FontCache::lookupCTFont() (which calls CTFontCreateForCharactersWithLanguage() at the time of writing) instead of CTFontCreateForCharactersWithLanguage() since FontCache::systemFallbackForCharacters() does not explicitly call CTFontCreateForCharactersWithLanguage(). > Source/WebCore/ChangeLog:10 > + font. Also, all fonts that will be created on WebKit's behalf are > + already printer fonts. You may want to consider adding a comment to the code to explain that FontCache::lookupCTFont()/CTFontCreateForCharactersWithLanguage() will return a printer-friendly font. > Source/WebCore/platform/graphics/mac/FontCacheMac.mm:-543 > - CGFloat size; With the removal of this size variable and associated logic the comment above fontManager is now out-of-date. Please update the comment above fontManager. Comment on attachment 259567 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=259567&action=review >> Source/WebCore/ChangeLog:10 >> + already printer fonts. > > You may want to consider adding a comment to the code to explain that FontCache::lookupCTFont()/CTFontCreateForCharactersWithLanguage() will return a printer-friendly font. Going forward, the concept of a "printer font" will become less and less relevant. I don't think it's worth paying it any more lip service. Committed r188802: <http://trac.webkit.org/changeset/188802> |