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

Fix a crash where fonts randomly become unavailable #1354

Closed
wants to merge 1 commit into from

Conversation

ninarg
Copy link
Contributor

@ninarg ninarg commented Jun 20, 2024

Why?

We have a crash in Tori and FINN NMP related to fonts. The problem is that some fonts suddenly "disappear" from memory a while after having been successfully loaded on app startup, so our force unwrap of the FINN font will crash the app.

What?

We found a consistent way to reproduce this:

  1. Open a real-estate ad
  2. Open the map
  3. Move the app to the background
  4. Open the app again. Navigate back from map to ad and scroll. CRASH! 💣

I found this thread of others experiencing the same issue. The last post includes a fix, where you use another method for registering fonts based on URL. Currently we are using this method to register fonts: CTFontManagerRegisterGraphicsFont, but the fix is to switch to CTFontManagerRegisterFontsForURL, where you are able to specify a scope.
There's a scope called .process which means the font will be available for the lifetime of "the process" which registered it, e.g. it needs to be registered once after every restart of the app, as we already do.

After having changed to CTFontManagerRegisterFontsForURL, the app does not crash anymore when following the steps above, so it seems to fix it. 🤞

Version Change

Minor.

@ninarg ninarg requested review from a team, AtishPapa and Breiby and removed request for a team June 20, 2024 12:15
Copy link
Member

@bstien bstien left a comment

Choose a reason for hiding this comment

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

Holy crap! Awesome job digging into this😄🎉🎉

Copy link
Contributor

@AndreyMomot AndreyMomot left a comment

Choose a reason for hiding this comment

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

🤞 🥳

Copy link
Member

@Breiby Breiby left a comment

Choose a reason for hiding this comment

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

Wow, that's interesting! Any idea why it doesn't crash in Legendary?

@ninarg
Copy link
Contributor Author

ninarg commented Jun 20, 2024

Wow, that's interesting! Any idea why it doesn't crash in Legendary?

No idea 🤷 mystery

@ninarg
Copy link
Contributor Author

ninarg commented Jun 24, 2024

@Breiby Figured out why legendary is working. There we do a double registering of fonts, so we do it in FinniversKit, but also add the fonts as "Fonts provided by application" to Info.plist. So legendary would fallback on that one and not crash. So seems like I might not have to merge this PR, but rather fix it on the other side (add fonts to the main project for FINN NMP and Tori).

@AndreyMomot
Copy link
Contributor

Wow, nice catch! 💪
May we include some comment as required step somewhere in the App for new brands (DBA & Blocket) to follow these steps for Font registration as well ? :)

@ninarg
Copy link
Contributor Author

ninarg commented Jun 25, 2024

Closing this one, we found an alternative way to solve it.

@ninarg ninarg closed this Jun 25, 2024
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.

4 participants