ChartsOrg / ChartsOrg/Charts

MarkerView.viewFromXib possible crash

Open
#5,023 1 comment 2 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.