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 Mac crash bug introduced by commit e0ffbf106d #5322

Merged
merged 3 commits into from
Aug 10, 2023

Conversation

CharlieFenton
Copy link
Contributor

Fixes #5321

Commit e0ffbf1 failed t check for a NULL return from wxLocale::GetLanguageInfo(wxLANGUAGE_DEFAULT) which causes BOINC Manager to crash if the default system locale is unknown to wxWidgets.

This also means that the code added by #5176 which adds automatic selection of the language to the list of languages does not work on the Mac. In fact, that code broke automatic language selection on the Mac, which worked correctly in BOINC 7.22.2.

I have tried to guard the new automatic language selection code with #ifndef __WXMAC__, but I am getting an assert from wxWidgets "Invalid combobox index" if I scroll all the way to the top of the language selection combo box.
@BrianNixon can you please look at this and tel me what more I need to do. Also, please note that line 752 of DlgOptions.cpp also calls wxLocale::GetLanguageInfo(wxLANGUAGE_DEFAULT) without checking for a NULL return.

I have also fixed 4 compiler warnings introduced by commit 59eb2c2.

@CharlieFenton CharlieFenton marked this pull request as draft August 9, 2023 13:10
@CharlieFenton
Copy link
Contributor Author

Additional explanation of the problem:
wxWidgets does not properly detect the default system locale (language and country / region codes) for any language other than English. It's not clear to me whether this is due to a bug in macOS or if wxWidgets is not doing it correctly, but I suspect the problem may be in MacOS. The language portion of the code returned by the Mac is always "en" but the region / country code is correct. So for Japanese (Japan), we get en_JP. For France we get en_FR, and so forth. Since these are nonsense codes, wxWidgets treats it as wxLANGUAGE_UNKNOWN and returns NULL for the language info.

This means that all the logic he put in for Automatic Detection of the language is not useable on the Mac, and so the "Automatic Detection" item Brian Nixon added to the language menu should not appear on the Mac since it does not work. Note that automatic detection worked correctly on the Mac before #5176, though the Other Options dialog may not have always shown the language which BOINC was actually using.

Rather than totally eliminating all the changes for automatic language detection from #5176, it would be better to just remove that portion which broke automatic detection on the Mac.

@CharlieFenton
Copy link
Contributor Author

I think I've figured out how to make this work for the Mac and still keep the (Automatic Detection) menu item. I'll work on this more later this week.

@CharlieFenton CharlieFenton marked this pull request as ready for review August 10, 2023 12:01
@CharlieFenton
Copy link
Contributor Author

@AenBleidd I believe this is fixed properly now. Please merge when checks pass.

@AenBleidd
Copy link
Member

Looks good to me. Thank you for the fix!

@AenBleidd AenBleidd merged commit e17b12c into master Aug 10, 2023
CharlieFenton pushed a commit that referenced this pull request Aug 11, 2023
Fix Mac crash bug introduced by commit e0ffbf1
@CharlieFenton CharlieFenton deleted the mac_fix_7_24_0_crash branch August 11, 2023 10:50
@BrianNixon
Copy link
Contributor

@CharlieFenton: Thank you for fixing this. Evidently I had assumed that asking wxWidgets about the default language couldn’t fail – though I don’t recall what led me to that line of thinking, as even a cursory look at the wxWidgets source shows that it can.

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

Successfully merging this pull request may close these issues.

CRASH ! BOINC Manager 7.24.0 on macOS
3 participants