Bug 233070 – [Cocoa] REGRESSION(r281291): Text Style fonts don't have the correct weight set

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

[Myles C. Maxfield]

[Cocoa] Text style fonts shouldn't have their weight/width/slope overridden by font-weight/font-stretch/font-style

[Myles C. Maxfield]

Created attachment 444106 [details]
Needs test

[Myles C. Maxfield]

<rdar://problem/84663666>

[Myles C. Maxfield]

Created attachment 444107 [details]
Needs test

[Myles C. Maxfield]

Created attachment 444110 [details]
Patch

[Myles C. Maxfield]

Created attachment 444784 [details]
Patch

[Myles C. Maxfield]

Created attachment 444860 [details]
Patch

[Myles C. Maxfield]

This needs more investigation, I think. We do:

textStyle = kCTUIFontTextStyleBody;
fontDescriptor = adoptCF(CTFontDescriptorCreateWithTextStyle(textStyle, contentSizeCategory(), nullptr));
...
auto font = adoptCF(CTFontCreateWithFontDescriptor(fontDescriptor.get(), 0, nullptr));
...
fontDescription.setWeight(cssWeightOfSystemFont(font.get()));

So we’re creating the text style font, pulling out its weight, then setting the font-weight property to that, which should work, if we then re-apply that same weight later.

I think I need more investigation here. This patch might be wrong.

[Myles C. Maxfield]

Oh, I think I see what's happening.

Just like:
font: bold 48px 'Helvetica'; font-weight: normal;
causes the text to be rendered with normal weight, not bold,

font: -apple-system-body; font-weight: normal;
causes text to be rendered with normal weight, not whatever weight -apple-system-body corresponds to.

[Myles C. Maxfield]

Aha! CFDictionaryGetValue(CTFontCopyTraits(CTFontCreateWithFontDescriptor(CTFontDescriptorCreateWithTextStyle(kCTUIFontTextStyleBody, contentSizeCategory(), nullptr))), kCTFontWeightTrait) is returning 0

[Myles C. Maxfield]

I can just use kCTFontCSSWeightAttribute instead.

[Myles C. Maxfield]

Created attachment 445050 [details]
Patch

[Myles C. Maxfield]

Created attachment 445055 [details]
Patch

[Cameron McCormack (:heycam)]

Comment on attachment 445055 [details]
Patch

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

> Source/WebCore/rendering/RenderThemeCocoa.mm:287
> +    CFNumberRef resultRef = static_cast<CFNumberRef>(CTFontCopyAttribute(font, kCTFontCSSWeightAttribute));

"auto resultRef"

[Myles C. Maxfield]

Comment on attachment 445055 [details]
Patch

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

>> Source/WebCore/rendering/RenderThemeCocoa.mm:287
>> +    CFNumberRef resultRef = static_cast<CFNumberRef>(CTFontCopyAttribute(font, kCTFontCSSWeightAttribute));
> 
> "auto resultRef"

I wrote a memory leak :( this line needs an adoptCF().

[Myles C. Maxfield]

Committed r286254 (244617@main): <https://commits.webkit.org/244617@main>