From 03d10bb67a72153e1aca0e7c67eaf022769d75b9 Mon Sep 17 00:00:00 2001 From: Jesse Bounds Date: Fri, 7 Apr 2017 15:23:39 -0700 Subject: [ios, macos] Guard against looking up annotation contexts MGLAnnotationTagNotFound (#8686) --- platform/ios/CHANGELOG.md | 4 +++ platform/ios/src/MGLMapView.mm | 74 ++++++++++++++++++++++------------------ platform/macos/CHANGELOG.md | 1 + platform/macos/src/MGLMapView.mm | 11 ++++-- 4 files changed, 54 insertions(+), 36 deletions(-) diff --git a/platform/ios/CHANGELOG.md b/platform/ios/CHANGELOG.md index bd542eeeb3..fcbb3a26a6 100644 --- a/platform/ios/CHANGELOG.md +++ b/platform/ios/CHANGELOG.md @@ -2,6 +2,10 @@ Mapbox welcomes participation and contributions from everyone. Please read [CONTRIBUTING.md](../../CONTRIBUTING.md) to get started. +## 3.5.2 + +* Fixed an issue that caused a crash when the user location annotation was presenting a callout view and the map was moved. ([#8686](https://github.com/mapbox/mapbox-gl-native/pull/8686)) + ## 3.5.1 - April 5, 2017 * Fixed an issue that caused the return type of a map view delegate method to bridge incorrectly to applications written in Swift. ([#8541](https://github.com/mapbox/mapbox-gl-native/pull/8541)) diff --git a/platform/ios/src/MGLMapView.mm b/platform/ios/src/MGLMapView.mm index 7458c39299..540f6de861 100644 --- a/platform/ios/src/MGLMapView.mm +++ b/platform/ios/src/MGLMapView.mm @@ -1452,7 +1452,9 @@ public: } else { - nextElement = _annotationContextsByAnnotationTag.at(_selectedAnnotationTag).accessibilityElement; + if (_selectedAnnotationTag != MGLAnnotationTagNotFound) { + nextElement = _annotationContextsByAnnotationTag.at(_selectedAnnotationTag).accessibilityElement; + } } [self deselectAnnotation:self.selectedAnnotation animated:YES]; UIAccessibilityPostNotification(UIAccessibilityScreenChangedNotification, nextElement); @@ -2002,19 +2004,21 @@ public: { const mbgl::Point point = MGLPointFromLocationCoordinate2D(annotation.coordinate); - MGLAnnotationContext &annotationContext = _annotationContextsByAnnotationTag.at(annotationTag); - if (annotationContext.annotationView) - { - // Redundantly move the associated annotation view outside the scope of the animation-less transaction block in -updateAnnotationViews. - annotationContext.annotationView.center = [self convertCoordinate:annotationContext.annotation.coordinate toPointToView:self]; - } + if (annotationTag != MGLAnnotationTagNotFound) { + MGLAnnotationContext &annotationContext = _annotationContextsByAnnotationTag.at(annotationTag); + if (annotationContext.annotationView) + { + // Redundantly move the associated annotation view outside the scope of the animation-less transaction block in -updateAnnotationViews. + annotationContext.annotationView.center = [self convertCoordinate:annotationContext.annotation.coordinate toPointToView:self]; + } - MGLAnnotationImage *annotationImage = [self imageOfAnnotationWithTag:annotationTag]; - NSString *symbolName = annotationImage.styleIconIdentifier; + MGLAnnotationImage *annotationImage = [self imageOfAnnotationWithTag:annotationTag]; + NSString *symbolName = annotationImage.styleIconIdentifier; - // Update the annotation’s backing geometry to match the annotation model object. Any associated annotation view is also moved by side effect. However, -updateAnnotationViews disables the view’s animation actions, because it can’t distinguish between moves due to the viewport changing and moves due to the annotation’s coordinate changing. - _mbglMap->updateAnnotation(annotationTag, mbgl::SymbolAnnotation { point, symbolName.UTF8String }); - [self updateCalloutView]; + // Update the annotation’s backing geometry to match the annotation model object. Any associated annotation view is also moved by side effect. However, -updateAnnotationViews disables the view’s animation actions, because it can’t distinguish between moves due to the viewport changing and moves due to the annotation’s coordinate changing. + _mbglMap->updateAnnotation(annotationTag, mbgl::SymbolAnnotation { point, symbolName.UTF8String }); + [self updateCalloutView]; + } } } else if ([keyPath isEqualToString:@"coordinates"] && [object isKindOfClass:[MGLMultiPoint class]]) @@ -3055,17 +3059,18 @@ public: for (auto const& annotationTag: annotationTags) { - if (!_annotationContextsByAnnotationTag.count(annotationTag)) + if (!_annotationContextsByAnnotationTag.count(annotationTag) || + annotationTag == MGLAnnotationTagNotFound) { continue; } + MGLAnnotationContext annotationContext = _annotationContextsByAnnotationTag.at(annotationTag); NSAssert(annotationContext.annotation, @"Missing annotation for tag %u.", annotationTag); if (annotationContext.annotation) { [annotations addObject:annotationContext.annotation]; } - } return [annotations copy]; @@ -3077,8 +3082,8 @@ public: /// Returns the annotation assigned the given tag. Cheap. - (id )annotationWithTag:(MGLAnnotationTag)tag { - if ( ! _annotationContextsByAnnotationTag.count(tag)) - { + if ( ! _annotationContextsByAnnotationTag.count(tag) || + tag == MGLAnnotationTagNotFound) { return nil; } @@ -3680,10 +3685,12 @@ public: { return self.userLocation; } - if ( ! _annotationContextsByAnnotationTag.count(_selectedAnnotationTag)) - { + + if ( ! _annotationContextsByAnnotationTag.count(_selectedAnnotationTag) || + _selectedAnnotationTag == MGLAnnotationTagNotFound) { return nil; } + MGLAnnotationContext &annotationContext = _annotationContextsByAnnotationTag.at(_selectedAnnotationTag); return annotationContext.annotation; } @@ -3749,19 +3756,16 @@ public: MGLAnnotationView *annotationView = nil; if (annotation != self.userLocation) - { - MGLAnnotationContext &annotationContext = _annotationContextsByAnnotationTag.at(annotationTag); - - annotationView = annotationContext.annotationView; - - if (annotationView && annotationView.enabled) - { - // Annotations represented by views use the view frame as the positioning rect. - positioningRect = annotationView.frame; - - [annotationView.superview bringSubviewToFront:annotationView]; - - [annotationView setSelected:YES animated:animated]; + if (annotationTag != MGLAnnotationTagNotFound) { + MGLAnnotationContext &annotationContext = _annotationContextsByAnnotationTag.at(annotationTag); + annotationView = annotationContext.annotationView; + if (annotationView && annotationView.enabled) { + { + // Annotations represented by views use the view frame as the positioning rect. + positioningRect = annotationView.frame; + [annotationView.superview bringSubviewToFront:annotationView]; + [annotationView setSelected:YES animated:animated]; + } } } @@ -5039,8 +5043,12 @@ public: if (isAnchoredToAnnotation) { MGLAnnotationTag tag = [self annotationTagForAnnotation:annotation]; - MGLAnnotationContext &annotationContext = _annotationContextsByAnnotationTag.at(tag); - MGLAnnotationView *annotationView = annotationContext.annotationView; + MGLAnnotationView *annotationView = nil; + + if (tag != MGLAnnotationTagNotFound) { + MGLAnnotationContext &annotationContext = _annotationContextsByAnnotationTag.at(tag); + annotationView = annotationContext.annotationView; + } CGRect rect = [self positioningRectForCalloutForAnnotationWithTag:tag]; diff --git a/platform/macos/CHANGELOG.md b/platform/macos/CHANGELOG.md index 326a70ebb2..1d24f2a6e8 100644 --- a/platform/macos/CHANGELOG.md +++ b/platform/macos/CHANGELOG.md @@ -3,6 +3,7 @@ ## 0.4.1 * Fixed an issue causing code signing failures and bloating the framework. ([#8640](https://github.com/mapbox/mapbox-gl-native/pull/8640)) +* Fixed an issue that could cause a crash if annotations unknown to the map view were interacted with. ([#8686](https://github.com/mapbox/mapbox-gl-native/pull/8686)) * Renamed the "Data-Driven Styling" guide to "Using Style Functions at Runtime" and clarified the meaning of data-driven styling in the guide's discussion of runtime style functions. ([#8627](https://github.com/mapbox/mapbox-gl-native/pull/8627)) ## 0.4.0 diff --git a/platform/macos/src/MGLMapView.mm b/platform/macos/src/MGLMapView.mm index a5a5c53df3..a873d9ef82 100644 --- a/platform/macos/src/MGLMapView.mm +++ b/platform/macos/src/MGLMapView.mm @@ -1796,10 +1796,12 @@ public: for (auto const& annotationTag: annotationTags) { - if (!_annotationContextsByAnnotationTag.count(annotationTag)) + if (!_annotationContextsByAnnotationTag.count(annotationTag) || + annotationTag == MGLAnnotationTagNotFound) { continue; } + MGLAnnotationContext annotationContext = _annotationContextsByAnnotationTag.at(annotationTag); NSAssert(annotationContext.annotation, @"Missing annotation for tag %u.", annotationTag); if (annotationContext.annotation) @@ -1816,7 +1818,8 @@ public: /// Returns the annotation assigned the given tag. Cheap. - (id )annotationWithTag:(MGLAnnotationTag)tag { - if (!_annotationContextsByAnnotationTag.count(tag)) { + if ( ! _annotationContextsByAnnotationTag.count(tag) || + tag == MGLAnnotationTagNotFound) { return nil; } @@ -2147,9 +2150,11 @@ public: } - (id )selectedAnnotation { - if (!_annotationContextsByAnnotationTag.count(_selectedAnnotationTag)) { + if ( ! _annotationContextsByAnnotationTag.count(_selectedAnnotationTag) || + _selectedAnnotationTag == MGLAnnotationTagNotFound) { return nil; } + MGLAnnotationContext &annotationContext = _annotationContextsByAnnotationTag.at(_selectedAnnotationTag); return annotationContext.annotation; } -- cgit v1.2.1