Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bridge CTFontRef directly to UIFont when converting attributes on NSAttributedString #1579

Merged
merged 4 commits into from
Jul 11, 2019

Conversation

rahul-malik
Copy link
Contributor

In iOS13, there is an issue with the font returned from "-[UIFontj fontWithName:size:]". When asking for "SFUI-Regular" returns "Times New Roman" font. It seems like a reasonable default would be to return the system font in the case that the requested font was not respected.

In iOS13, there is an issue with the font returned from "-[UIFontj fontWithName:size:]". When asking for "SFUI-Regular" returns "Times New Roman" font. It seems like a reasonable default would be to return the system font in the case that the requested font was not respected.
@CLAassistant
Copy link

CLAassistant commented Jul 10, 2019

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@nguyenhuy nguyenhuy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Wondering if you filed a radar since this seems to be an issue in UIFont itself.

@rahul-malik
Copy link
Contributor Author

@nguyenhuy - Yep! Radar has been filed under FB6592770

@mikezucc
Copy link
Contributor

somewhere, somehow, someone consciously put Times New Roman in an catch/else case

@rahul-malik rahul-malik changed the title Fallback to system font if the fontFamily is incorrect Bridge CTFontRef directly to UIFont when converting attributes on NSAttributedString Jul 11, 2019
@rahul-malik
Copy link
Contributor Author

@maicki @nguyenhuy - I updated the title since the change is more than updating the fallback. It isn't clear why we were trying to create a new font instance when CTFontRef toll-free bridges to UIFont. This appears to pass tests both internally at Pinterest and Texture so I feel like the change is pretty safe and should also solve the iOS 13 regression users are experiencing

@nguyenhuy
Copy link
Member

nguyenhuy commented Jul 11, 2019

Great change, @rahul-malik! Yeah, I think it's safe and we should just get it in.

The original logic is pretty old -- dates back to the initial commit. Perhaps @appleguy has some context on why it's done that way.

@ex3ndr
Copy link

ex3ndr commented Oct 26, 2019

Hey! Any ETA to new release with this fix? This is pretty heavy issue.

@rahul-malik
Copy link
Contributor Author

Hey! We’re actively working on creating a new release now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants