Changed fontlist to list all avalilable fonts.

Changed fontlist to list all avalilable fonts.

Right now, GetFontList_SlowBlocking only lists up font family names.
For example only Osaka shows up and you cannot select Osaka-mono.

I changed it to list all available fonts.
Fonts that start with a period are hidden fonts so we ignore them.


BUG=86734

[keishi-hattori, 2011-06-22 07:44:01]

font_settings_utils_mac.mm tells me that it used to store font names but later
changed to store font family names. I can't figure out why. Bug 86734 indicates
that we definitely need to be able to specify with font name.

[brettw, 2011-06-22 15:59:09]

What does the preferences list look like with this change? How does this compare
to Safari? I see Safari listing "Osaka" and it doesn't have "Osaka-mono".

[csilv, 2011-06-22 18:54:04]

I think it would be a mistake make this change.  The fonts menu can already be
unweildy if a user has a lot of fonts... displaying every variation of a font
family will significantly increase the size of this menu, and thus decrease
usability.

The current behavior of allowing selection by family is consistent with both
Safari and Firefox.  Neither browser allows selection of Osaka-mono.

Apple's decision to include Osaka-mono under the Osaka family is unfortunate. 
Even Apple's font dialog doesn't recognize Osaka if you select the 'Monospace'
filter.  If this is really important we should consider whether we can
auto-select a monospace variant of a family when it is chosen for the monospace
setting.

[Nobuyoshi.Miyokawa_gmail.com, 2011-06-22 23:18:57]

just FYI.

Firefox 4.0.1 and 5.0 can allow us to select Osaka-mono without showing other
font's variants, such as Courier Bold, Courier Bold Oblique, etc.

[keishi-hattori, 2011-06-23 02:24:19]

On 2011/06/22 18:54:04, csilv wrote:
> I think it would be a mistake make this change.  The fonts menu can already be
> unweildy if a user has a lot of fonts... displaying every variation of a font
> family will significantly increase the size of this menu, and thus decrease
> usability.
> 
> The current behavior of allowing selection by family is consistent with both
> Safari and Firefox.  Neither browser allows selection of Osaka-mono.

In Firefox I am able to select Osaka-mono. It looks like they may have hard
coded that in.

http://hg.mozilla.org/mozilla-central/file/48e72227c2fa/modules/libpref/src/i...

> Apple's decision to include Osaka-mono under the Osaka family is unfortunate. 
> Even Apple's font dialog doesn't recognize Osaka if you select the 'Monospace'
> filter.  If this is really important we should consider whether we can
> auto-select a monospace variant of a family when it is chosen for the
monospace
> setting.

Personally I have never noticed this, but I think its really important.

I am going to upload a patch with a way to automatically detect these kinds of
cases.

[keishi-hattori, 2011-06-24 06:51:19]

Here is a screenshot comparison before and after the patch. Only Osaka-Mono is
added and the list is sorted alphabetically. 
https://docs.google.com/leaf?id=0B_wjZbesoS1tODczMGNmNjUtYTFkMy00ZDExLWE1MzAt...

[csilv, 2011-06-24 20:51:30]

Looking good.  See comments.

http://codereview.chromium.org/7204026/diff/4002/chrome/browser/ui/webui/opti...
File chrome/browser/ui/webui/options/font_settings_utils_mac.mm (right):

http://codereview.chromium.org/7204026/diff/4002/chrome/browser/ui/webui/opti...
chrome/browser/ui/webui/options/font_settings_utils_mac.mm:27: NSFontManager*
fontManager = [[[NSFontManager alloc] init] autorelease];
Use scoped_nsobject instead of autorelease.

http://codereview.chromium.org/7204026/diff/4002/content/common/font_list_mac.mm
File content/common/font_list_mac.mm (right):

http://codereview.chromium.org/7204026/diff/4002/content/common/font_list_mac...
content/common/font_list_mac.mm:28: initWithKey:@"localizedName" ascending:YES];
indent line continuations 4 space, here and below.

http://codereview.chromium.org/7204026/diff/4002/content/common/font_list_mac...
content/common/font_list_mac.mm:30: [NSArray arrayWithObject: sortDescriptor]];
eliminate space after colon

[brettw, 2011-06-24 20:56:26]

I'm not a Mac or Obj-C person, so I defer to csilv on style and implementation.
I just have a couple of general comments.

http://codereview.chromium.org/7204026/diff/4002/chrome/browser/ui/webui/opti...
File chrome/browser/ui/webui/options/font_settings_utils_mac.mm (right):

http://codereview.chromium.org/7204026/diff/4002/chrome/browser/ui/webui/opti...
chrome/browser/ui/webui/options/font_settings_utils_mac.mm:26: NSString*
familyName = [font familyName];
Local variables should be lower_case_with_underscores.

http://codereview.chromium.org/7204026/diff/4002/chrome/browser/ui/webui/opti...
chrome/browser/ui/webui/options/font_settings_utils_mac.mm:30: // The condition
below is for Osaka-Mono which exceptionally stores font
I'd probably not put the comment in the middle of the if statement, and instead
put it above, with "The fixed pitch check is for..."

http://codereview.chromium.org/7204026/diff/4002/content/common/font_list_mac.mm
File content/common/font_list_mac.mm (right):

http://codereview.chromium.org/7204026/diff/4002/content/common/font_list_mac...
content/common/font_list_mac.mm:51: if (isFamilyFixedPoint != [fontManager
fontNamed:fontName
Is there some way this function can be made more clear by separating out parts
of it into a file-static helper function? It's hard to follow this big block of
code.

[keishi1, 2011-07-04 09:52:37]

I fixed the issues and had to move the patch to my google.com account
http://codereview.chromium.org/7248065/

[brettw, 2011-07-07 17:02:05]

Rubber stamp LGTM for content (I assume csilv actually checked the code well).

[csilv, 2011-07-07 18:36:30]

On 2011/07/07 17:02:05, brettw wrote:
> Rubber stamp LGTM for content (I assume csilv actually checked the code well).

Yup, gave the lgtm on the new issue.