Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

Fix 'effectiveAppearance' crash on pre-Mojave #13314

Merged
merged 1 commit into from
Jan 13, 2021
Merged

Fix 'effectiveAppearance' crash on pre-Mojave #13314

merged 1 commit into from
Jan 13, 2021

Conversation

hartez
Copy link
Contributor

@hartez hartez commented Jan 6, 2021

Description of Change

Moves use of NSApplication EffectiveAppearance property inside of Mojave or later check.

Issues Resolved

API Changes

None

Platforms Affected

  • macOS

Behavioral/Visual Changes

None

Before/After Screenshots

Not applicable

Testing Procedure

Run this on High Sierra, see if it crashes. (It shouldn't.)

PR Checklist

  • Targets the correct branch
  • Tests are passing (or failures are unrelated)

@CartBlanche
Copy link

@hartez Could this change be back ported to 4.8 too, as it's been happening since at least 4.7?

@rmarinho
Copy link
Member

rmarinho commented Jan 7, 2021

@CartBlanche are you using 4.8 on a High Sierra?

@hartez I think this might be better on 4.7 or something that are built using older Xcode .

@CartBlanche
Copy link

@rmarinho I have 2 projects. One uses 4.8 and the other (due to better MacOS support) uses 5.x. Hence why I've asked for it on both. I cannot currently update my 4.8 project to 5.x, because MasterDetail has been deprecated and is broken in 5.x, for me. Also Flyouts don't work correctly and consistently across all platforms in 5.x either, so I can't switch to using that either. At least from my testing a week ago. I'm also 1 week away from releasing the 4.8 project on the app stores.

@samhouts samhouts added p/iOS 🍎 i/high Completely doesn't work, crashes, or is unusably slow, has no obvious workaround; occurs less often t/bug 🐛 labels Jan 8, 2021
@hartez
Copy link
Contributor Author

hartez commented Jan 12, 2021

@hartez Could this change be back ported to 4.8 too, as it's been happening since at least 4.7?

Has it? That call to EffectiveAppearance didn't show up until PR #10646; AFAICT, it's never been in 4.8.

@Redth Redth merged commit a663f51 into 5.0.0 Jan 13, 2021
@Redth Redth deleted the fix-13071 branch January 13, 2021 19:22
@CartBlanche
Copy link

@hartez Could this change be back ported to 4.8 too, as it's been happening since at least 4.7?

Has it? That call to EffectiveAppearance didn't show up until PR #10646; AFAICT, it's never been in 4.8.

@hartez Based on this other customer's experience, that's where he experienced the 1st fail. In 4.7.x. See here
#13071 (comment)

@CartBlanche
Copy link

So @hartez, @Redth will it be back ported to 4.8.x? 2nd is this fix now available in the preview nuget, if so which one?

@Redth
Copy link
Member

Redth commented Jan 14, 2021

@CartBlanche I can't find any trace of the use of EffectiveAppearance API's before 5.0 versions, which makes sense as @hartez noted that the code wasn't PR'd until #10646 which was in 5.0 releases.

The issue you linked to #13071 has the repro which also references a 5.0 prerelease of forms, hence the error.

This particular fix isn't going to be backported because it doesn't apply to 4.8. There may be another issue going on causing a crash in 4.8? It's not clear though as the repro all focuses around 5.0. I think @rmarinho may have been able to track down an install of high sierra to test a file->new project in 4.8, will follow up later with him.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
i/high Completely doesn't work, crashes, or is unusably slow, has no obvious workaround; occurs less often p/iOS 🍎 t/bug 🐛
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] and [CRASH] App Crashes on startup on High Sierra. Runs fine on higher operating systems
6 participants