Visitar URL original
[ios][pv]fix admob banner scrollable on ios 18.2 (#183274) · mboetger/flutter@85e8651 · GitHub
Skip to content

Commit 85e8651

Browse files
hellohuanlinmboetger
hellohuanlin
authored and
mboetger
committed
[ios][pv]fix admob banner scrollable on ios 18.2 (flutter#183274)
The admob banner in scrollable list issue was reported for iOS 26 (flutter#165787), but it actually also happens on iOS 18.2. We limited the depth of search to a few web view plugins only because it was a hacky workaround. However, given that the workaround has been on for more than a year and no bad side-effect reported, we should be more comfortable to enable it for all now. This will fix our recently added XCUITest below, since we are running on iOS 18 still: Fixes flutter#183218 Fixes flutter#183272 *If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].* ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [ ] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
1 parent 9e7ae79 commit 85e8651

2 files changed

Lines changed: 26 additions & 17 deletions

File tree

‎engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm‎

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -569,15 +569,12 @@ - (void)releaseGesture {
569569
self.delayingRecognizer.state = UIGestureRecognizerStateFailed;
570570
}
571571

572-
- (BOOL)containsWebView:(UIView*)view remainingSubviewDepth:(int)remainingSubviewDepth {
573-
if (remainingSubviewDepth < 0) {
574-
return NO;
575-
}
572+
- (BOOL)containsWebView:(UIView*)view {
576573
if ([view isKindOfClass:[WKWebView class]]) {
577574
return YES;
578575
}
579576
for (UIView* subview in view.subviews) {
580-
if ([self containsWebView:subview remainingSubviewDepth:remainingSubviewDepth - 1]) {
577+
if ([self containsWebView:subview]) {
581578
return YES;
582579
}
583580
}
@@ -642,13 +639,10 @@ - (void)blockGesture {
642639
[self searchAndFixWebView:self.embeddedView];
643640
}
644641
} else if (@available(iOS 18.2, *)) {
645-
// This workaround is designed for WKWebView only. The 1P web view plugin provides a
646-
// WKWebView itself as the platform view. However, some 3P plugins provide wrappers of
647-
// WKWebView instead. So we perform DFS to search the view hierarchy (with a depth limit).
648-
// Passing a limit of 0 means only searching for platform view itself; Pass 1 to include its
649-
// children as well, and so on. We should be conservative and start with a small number. The
650-
// AdMob banner has a WKWebView at depth 7.
651-
if ([self containsWebView:self.embeddedView remainingSubviewDepth:1]) {
642+
// The 1P web view plugin provides a WKWebView itself as the platform view. However, some 3P
643+
// plugins provide wrappers of WKWebView instead, and AdMob banner has a WKWebView at
644+
// depth 7. So we perform DFS to search the view hierarchy.
645+
if ([self containsWebView:self.embeddedView]) {
652646
[self removeGestureRecognizer:self.delayingRecognizer];
653647
[self addGestureRecognizer:self.delayingRecognizer];
654648
}

‎engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm‎

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3487,8 +3487,9 @@ - (void)testFlutterPlatformViewTouchesEndedOrTouchesCancelledEventDoesNotFailThe
34873487

34883488
BOOL shouldReAddDelayingRecognizer = NO;
34893489
if (@available(iOS 26.0, *)) {
3490-
// TODO(hellohuanlin): find a solution for iOS 26,
3491-
// https://github.com/flutter/flutter/issues/175099.
3490+
// We use a different workaround for iOS 26 and it is tested separately.
3491+
// See:
3492+
// testFlutterPlatformViewBlockGestureUnderEagerPolicyShouldDisableAndReEnableTouchEventsGestureRecognizerForSimpleWebView.
34923493
} else if (@available(iOS 18.2, *)) {
34933494
shouldReAddDelayingRecognizer = YES;
34943495
}
@@ -3503,7 +3504,7 @@ - (void)testFlutterPlatformViewTouchesEndedOrTouchesCancelledEventDoesNotFailThe
35033504
}
35043505

35053506
- (void)
3506-
testFlutterPlatformViewBlockGestureUnderEagerPolicyShouldNotRemoveAndAddBackDelayingRecognizerForNestedWrapperWebView {
3507+
testFlutterPlatformViewBlockGestureUnderEagerPolicyShouldRemoveAndAddBackDelayingRecognizerForNestedWrapperWebView {
35073508
flutter::FlutterPlatformViewsTestMockPlatformViewDelegate mock_delegate;
35083509

35093510
flutter::TaskRunners runners(/*label=*/self.name.UTF8String,
@@ -3557,8 +3558,22 @@ - (void)testFlutterPlatformViewTouchesEndedOrTouchesCancelledEventDoesNotFailThe
35573558

35583559
[(FlutterTouchInterceptingView*)touchInteceptorView blockGesture];
35593560

3560-
XCTAssertEqual(touchInteceptorView.gestureRecognizers[0], delayingRecognizer);
3561-
XCTAssertEqual(touchInteceptorView.gestureRecognizers[1], forwardingRecognizer);
3561+
BOOL shouldReAddDelayingRecognizer = NO;
3562+
if (@available(iOS 26.0, *)) {
3563+
// We use a different workaround for iOS 26 and it is tested separately.
3564+
// See:
3565+
// testFlutterPlatformViewBlockGestureUnderEagerPolicyShouldDisableAndReEnableTouchEventsGestureRecognizerForSimpleWebView.
3566+
} else if (@available(iOS 18.2, *)) {
3567+
shouldReAddDelayingRecognizer = YES;
3568+
}
3569+
if (shouldReAddDelayingRecognizer) {
3570+
// Since we remove and add back delayingRecognizer, it would be reordered to the last.
3571+
XCTAssertEqual(touchInteceptorView.gestureRecognizers[0], forwardingRecognizer);
3572+
XCTAssertEqual(touchInteceptorView.gestureRecognizers[1], delayingRecognizer);
3573+
} else {
3574+
XCTAssertEqual(touchInteceptorView.gestureRecognizers[0], delayingRecognizer);
3575+
XCTAssertEqual(touchInteceptorView.gestureRecognizers[1], forwardingRecognizer);
3576+
}
35623577
}
35633578

35643579
- (void)

0 commit comments

Comments
 (0)