MarkerView.viewFromXib possible crash
- Dominant language
- Swift
- Stars
- 28k
- Forks
- 6k
- PR merge metrics
- No merged PRs in 30d
Description
* [x] I've read, understood, and done my best to follow the [*CONTRIBUTING guidelines](https://github.com/jjatie/Charts/blob/master/CONTRIBUTING.md).
## What did you do?
Added a `LineChartViewController` to a Mac app, with a `MarkerView` subclass as its marker.
## What did you expect to happen?
The marker would show up when a value on the chart was selected.
## What happened instead?
App crashed due to trying to force-unwrap a nil value.
## Charts Environment
**Charts version/Branch/Commit Number:** v4.1.0
**Xcode version:** 14.2
**Swift version:** .57
**Platform(s) running Charts:** MacOS
**macOS version running Xcode:** 13.2.1
## Solution:
I found the offending line of code in `MarkerView`'s `viewFromXib(in:)`:
```
if bundle.loadNibNamed(
NSNib.Name(String(describing: self)),
owner: nil,
topLevelObjects: &loadedObjects)
{
return loadedObjects?[0] as? MarkerView
}
```
What I found was that `loadedObjects` contains the `MarkerView`, but not always at index 0 (in my tests, there are always two elements in the array, one of which is an instance of `NSApplication`). So the issue can be fixed by changing the `return` line to:
```
return loadedObjects?.compactMap({ $0 as? Self }).first
```
(I also changed the return value of `viewFromXib` to `Self`, but that change could be omitted for a smaller change)
I can make a PR for this. Not sure if I should make the same change to the `#if !os(OSX)` section of the function, though. Any guidance on that would be appreciated.
Contributor guide
Research direction
Start at MarkerView.viewFromXib(in:) in the macOS section and inspect how bundle.loadNibNamed populates loadedObjects. Reproduce the marker selection in a macOS app using a MarkerView subclass, then verify that the method returns the loaded MarkerView rather than assuming index 0. Check whether the corresponding non-macOS section also needs equivalent coverage.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- swift
- Domain
- desktop
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 52/100