Bug 146333 – [Cocoa] Sans-serif generic font family should map to PingFang

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Myles C. Maxfield]

[Cocoa] Sans-serif generic font family should map to PingFang

[Myles C. Maxfield]

Created attachment 255603 [details]
Patch

[Myles C. Maxfield]

Comment on attachment 255603 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=255603&action=review

> Source/WebCore/ChangeLog:4
> +        https://bugs.webkit.org/show_bug.cgi?id=146333

Need radar number

[Myles C. Maxfield]

Comment on attachment 255603 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=255603&action=review

> LayoutTests/platform/mac/fast/text/han-generic-font-families-expected.txt:1
> +layer at (0,0) size 800x600

Turn this in to a reftest

[Myles C. Maxfield]

Comment on attachment 255603 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=255603&action=review

> LayoutTests/fast/text/han-generic-font-families.html:13
> +    d.textContent = "此段中æç®ä½ææ¬ç¨äºæµè§å¨å¸å±æµè¯ã";

Woah this is a mistake

[Build Bot]

Comment on attachment 255603 [details]
Patch

Attachment 255603 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6722903991123968

New failing tests:
fast/text/han-generic-font-families.html

[Build Bot]

Created attachment 255605 [details]
Archive of layout-test-results from ews104 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5

[Build Bot]

Comment on attachment 255603 [details]
Patch

Attachment 255603 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5472806514982912

New failing tests:
fast/text/han-generic-font-families.html

[Build Bot]

Created attachment 255607 [details]
Archive of layout-test-results from ews103 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-mavericks  Platform: Mac OS X 10.9.5

[Myles C. Maxfield]

Created attachment 255615 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 255615 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=255615&action=review

We really need to kill this code, and to use APIs for language specific font fallback.

> Source/WebCore/page/mac/SettingsMac.mm:49
> +#if PLATFORM(COCOA)

Did you mean PLATFORM(MAC)?

> Source/WebCore/page/mac/SettingsMac.mm:-72
> -#if PLATFORM(COCOA)

Looks like some old mass replace mistake.

[Myles C. Maxfield]

(In reply to comment #10)
> Comment on attachment 255615 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=255615&action=review
> 
> We really need to kill this code, and to use APIs for language specific font
> fallback.

I'm not actually sure that makes sense. Which font do we use for fallback? Do we still want to hardcode Latin fonts and fall back from those? Why is Latin special?

(Maybe Latin is special because it is the only script where the concepts of "serif" and "sans serif" are really applicable)

Using naive fallback is troublesome because of some markup like "font-family: sans-serif, 'Heiti SC'". We want the sans-serif to match so that Heiti isn't rendered. Another option would be to keep the same structure, but locally probe what the fallback would be inside CoreText; this seems a little silly because we're mapping a known value to a known value. But then again, the mapping might change over time, so maybe it is valuable to dynamically create the mapping.

> 
> > Source/WebCore/page/mac/SettingsMac.mm:49
> > +#if PLATFORM(COCOA)
> 
> Did you mean PLATFORM(MAC)?
> 
> > Source/WebCore/page/mac/SettingsMac.mm:-72
> > -#if PLATFORM(COCOA)
> 
> Looks like some old mass replace mistake.

[Myles C. Maxfield]

Committed r186001: <http://trac.webkit.org/changeset/186001>

[Myles C. Maxfield]

(In reply to comment #11)
> (In reply to comment #10)
> > Comment on attachment 255615 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=255615&action=review
> > 
> > We really need to kill this code, and to use APIs for language specific font
> > fallback.
> 
> I'm not actually sure that makes sense. Which font do we use for fallback?
> Do we still want to hardcode Latin fonts and fall back from those? Why is
> Latin special?
> 
> (Maybe Latin is special because it is the only script where the concepts of
> "serif" and "sans serif" are really applicable)
> 
> Using naive fallback is troublesome because of some markup like
> "font-family: sans-serif, 'Heiti SC'". We want the sans-serif to match so
> that Heiti isn't rendered. Another option would be to keep the same
> structure, but locally probe what the fallback would be inside CoreText;
> this seems a little silly because we're mapping a known value to a known
> value. But then again, the mapping might change over time, so maybe it is
> valuable to dynamically create the mapping.

Oh, another thought - this code runs on web process creation. Doing a bunch of CT calls might actually cause a performance hit

> 
> > 
> > > Source/WebCore/page/mac/SettingsMac.mm:49
> > > +#if PLATFORM(COCOA)
> > 
> > Did you mean PLATFORM(MAC)?
> > 
> > > Source/WebCore/page/mac/SettingsMac.mm:-72
> > > -#if PLATFORM(COCOA)
> > 
> > Looks like some old mass replace mistake.

[David Kilzer (:ddkilzer)]

This caused:

Bug 146385: REGRESSION (r186001): fast/text/han-generic-font-families.html always fails on Yosemite
<https://bugs.webkit.org/show_bug.cgi?id=146385>