Merge upstream changes from Warp's core-foundation-rs fork
Hey servo folks, Sending a PR to merge Warp's changes to core-foundation-rs upstream. Note, these changes are purely additive, and shouldn't have an affect on any existing core-foundation-rs APIs:
Changes
- Adds
get_matrixtoCTFont, matching CTFontGetMatrix. - Adds
get_typographic_boundstoCTRun, matching CTRunGetTypographicBounds - Adds a patch for a panic in
extract_number_for_keythat occurs in Ventura. The underlying issue is an issue within the Core Framework library on Ventura--but this patch allows this library to not panic on Ventura if someone tries to access any of the font traits.
cc @madsmtm--tagging you in case you'd be interested in reviewing. I think the changes here (notably the change to fix a crash on Ventura) would be pretty valuable to be merged upstream. Thanks!
Can you clean up the commit history and avoid the formatting changes?
Can you also give some example code or add a test case for reproducing the Ventura failure?
:umbrella: The latest upstream changes (presumably b009c87f2da0038bb474800cd88597843e5dbdae) made this pull request unmergeable. Please resolve the merge conflicts.
In the name of finally getting (at least some of) these changes merged upstream, I've sent out https://github.com/servo/core-foundation-rs/pull/674 and https://github.com/servo/core-foundation-rs/pull/675 for the bits adding new functions.
In terms of the Ventura fix, I can't think of an easy way to write a test for it. FWIW, this has been live in Warp (used by ~100k macOS users) for a year now with no indication of it causing any issues. I can send out a separate PR for that as well, or we can keep it in our own fork; either works for us.
I can send out a separate PR for that as well, or we can keep it in our own fork; either works for us.
I definitely think we will want this fix upstream. A separate PR might make sense at this point seeing as it has been done for the other changes.
In terms of the Ventura fix, I can't think of an easy way to write a test for it.
I think the best test for this would be a usage of the API that triggers the bug without the fix if you have a standalone reproduction (perhaps using a particular system font). But unless anyone else strongly objects, I think it would be better to land this without a test than not at all if you are unable to provide that. A link to the original bug in warp that lead to this fix would also potentially be helpful.
I'm sorry that progress on reviewing/merging this has been so slow. The Servo project is still getting back up to speed after a long hiatus and is still quite behind on maintenance of libraries. Hopefully this will improve over the next few months.
Yeah we (@alokedesai or I) can put together a separate PR for the extract_number_for_key fix.
No need to apologize for latency here - there was an explicit request to clean up the commit history and provide some example/test code for the extract_number_for_key issue, and nobody from our side followed up. :sweat_smile: