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 localization crash on iOS 11 and 12 #1278

Merged
merged 1 commit into from
Apr 19, 2022
Merged

Fix localization crash on iOS 11 and 12 #1278

merged 1 commit into from
Apr 19, 2022

Conversation

macdrevx
Copy link
Contributor

@macdrevx macdrevx commented Apr 15, 2022

  • Style.localizeLabels(into:forLayerIds:) crashed on iOS 11 and 12 if
    any of the symbol layers had text-field set to nil.

Fixes #954

Pull request checklist:

  • Write tests for all new functionality. If tests were not written, please explain why.
  • Describe the changes in this PR, especially public API changes.
  • Add a changelog entry to to bottom of the relevant section (typically the ## main heading near the top).
  • Review and agree to the Contributor License Agreement (CLA).

* Style.localizeLabels(into:forLayerIds:) crashed on iOS 11 and 12 if
  any of the symbol layers had text-field set to nil.

Fixes #954
@macdrevx
Copy link
Contributor Author

Added a test that exercises the crash when run with the test host on an iOS 12 device and passes with the code change.

Copy link
Contributor

@evil159 evil159 left a comment

Choose a reason for hiding this comment

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

I wonder if we should eliminate force unwrapping(lines 28 and 52) here to prevent these kinds of crashes happening in future. Nice thing about crashing is that they are more prominent to the SDK users, so they are more likely to report them. While thrown errors are more likely to be ignored.

@macdrevx macdrevx merged commit b9280de into main Apr 19, 2022
@macdrevx macdrevx deleted the fix/954 branch April 19, 2022 10:27
@macdrevx
Copy link
Contributor Author

@evil159 these are situations where we have complete control of the inputs, so if it throws, it's our fault and finding out about it through a crash seems reasonable to me. They don't seem like actionable errors to propagate to the public API.

OdNairy pushed a commit that referenced this pull request Aug 22, 2023
* Add 1tap Munich navigation scenario

* Update new route file
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.

Crash under iOS 12 in mapView.mapboxMap.style.localizeLabels()
2 participants