Bug 146463

Summary: [Cocoa] Support bold and thin italicized system fonts
Product: WebKit Reporter: Myles C. Maxfield <mmaxfield>
Component: New BugsAssignee: Myles C. Maxfield <mmaxfield>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, commit-queue, dino, jonlee, rniwa, simon.fraser, thorton, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from ews104 for mac-mavericks-wk2
none
Archive of layout-test-results from ews102 for mac-mavericks
none
Patch
darin: review+
Patch for committing none

Description Myles C. Maxfield 2015-06-30 12:06:48 PDT
[iOS] Support bold and thin italicized system fonts
Comment 1 Myles C. Maxfield 2015-06-30 13:33:38 PDT
Created attachment 255845 [details]
Patch
Comment 2 Myles C. Maxfield 2015-06-30 13:34:08 PDT
<rdar://problem/20948885>
Comment 3 Myles C. Maxfield 2015-06-30 13:42:33 PDT
Comment on attachment 255845 [details]
Patch

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

> Source/WebCore/ChangeLog:3
> +        [iOS] Support bold and thin italicized system fonts

Should be marked as [Cocoa]
Comment 4 Build Bot 2015-06-30 14:05:03 PDT
Comment on attachment 255845 [details]
Patch

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

New failing tests:
fast/text/weighted-italicized-system-font.html
Comment 5 Build Bot 2015-06-30 14:05:05 PDT
Created attachment 255846 [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
Comment 6 Build Bot 2015-06-30 14:06:44 PDT
Comment on attachment 255845 [details]
Patch

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

New failing tests:
fast/text/weighted-italicized-system-font.html
Comment 7 Build Bot 2015-06-30 14:06:47 PDT
Created attachment 255848 [details]
Archive of layout-test-results from ews102 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-mavericks  Platform: Mac OS X 10.9.5
Comment 8 Myles C. Maxfield 2015-06-30 14:32:51 PDT
Looks like Mavericks doesn't have true italics. I'll try on Yosemite before updating TestExpectations.
Comment 9 Myles C. Maxfield 2015-06-30 22:41:03 PDT
Created attachment 255900 [details]
Patch
Comment 10 Darin Adler 2015-07-01 09:38:30 PDT
Comment on attachment 255900 [details]
Patch

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

> Source/WebCore/platform/graphics/mac/FontCacheMac.mm:244
> +        if (desiredTraits & NSFontItalicTrait) {
> +            NSFontManager *fontManager = [NSFontManager sharedFontManager];
> +            result = [fontManager convertFont:result toHaveTrait:desiredTraits];
> +        }

Would read better without the local variable.

> LayoutTests/platform/mac-mavericks/TestExpectations:23
> +# Mavericks doesn't have an italic system font
> +fast/text/weighted-italicized-system-font.html [ Failure ]

I think a different expected.html would be a better way to deal with this than expecting failure. We want to make sure that it correctly gives a non-italic result, right?
Comment 11 Myles C. Maxfield 2015-07-01 11:08:01 PDT
Created attachment 255933 [details]
Patch for committing
Comment 12 WebKit Commit Bot 2015-07-01 17:03:14 PDT
Comment on attachment 255933 [details]
Patch for committing

Clearing flags on attachment: 255933

Committed r186205: <http://trac.webkit.org/changeset/186205>