Fix crash removing multipoint or duplicate annotation #4766
Conversation
| { | ||
| id <MGLAnnotation> annotation = object; | ||
| MGLAnnotationTag annotationTag = [self annotationTagForAnnotation:annotation]; | ||
| if (annotationTag != MGLAnnotationTagNotFound) | ||
| MGLAnnotationTag annotationTag = (MGLAnnotationTag)(NSUInteger)context; |
There was a problem hiding this comment.
I take it MGLMultiPoint doesn’t return a valid annotationTag, which is why we can assume the tag exists (now that we’re no longer trying to support multipoint here)?
There was a problem hiding this comment.
We do give MGLMultiPoints annotation tags, but we don’t bother to observe the coordinate key path on MGLMultiPoints, because MGLMultiPoints are immutable.
This context usage could potentially be unsafe if a developer subclasses MGLMapView and registers it as an observer for a non-multipoint annotation’s coordinate key path. The context can be used to distinguish multiple observations on the same key path and object, or to distinguish observation by multiple levels of the inheritance hierarchy, but not both.
There was a problem hiding this comment.
Fixed in 1f23e4536d6719012886649cc6ad6fc97857437d.
Even if the context in that case happens to match the annotation’s tag – say, because the developer passed in NULL (which is equal to 0) for the first added annotation (which would have a tag of 0) – the result would be an unnecessary but harmless update. There could still be trouble around removing the observer, but this is such an edge case as to be programmer error.
|
👍 |
Fixed a crash removing a polyline or polygon by observing the coordinate key path on all annotations, not just non-MGLMultiPoints. Pass in the annotation tag as the observation context. This fixes a crash when an annotation is added to the map view more than once, then removed more than once.
There’s no point observing the coordinate key path on MGLMultiPoint, since that property is immutable. Use the observation context instead of performing an annotation tag lookup when a coordinate has changed.
If a subclass of MGLMapView registers itself as an observer of a non-multipoint annotation’s coordinate key path but fails to handle the change in its -observeValueForKeyPath:ofObject:change:context: implementation, the context would likely be a value other than the annotation’s tag. In that case, avoid doing anything in response to the coordinate changing. Even if the context in that case happens to match the annotation’s tag – say, because the developer passed in NULL (which is equal to 0) for the first added annotation (which would have a tag of 0) – the result would be an unnecessary but harmless update.
1ec5
force-pushed
the
1ec5-annotation-coordinate-duplicate-3835
branch
from
1f23e45
to
2aaa8b9
Compare
Fixed a crash removing a polyline or polygon by only removing the
coordinatekey path observer from non-MGLMultiPoints. Noted in the changelog that you still need to replace an MGLMultiPoint to have it appear in a different location or in a different form.The annotation tag is now passed in as the observation context. This fixes a crash when an annotation is added to the map view more than once, then removed more than once.
/ref #3835 (comment)
/cc @boundsj @friedbunny