Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@cyanglaz
Copy link
Contributor

@cyanglaz cyanglaz commented Sep 19, 2020

Description

The iOS platform view's mask view are sometimes physically added to the view hierarchy which resulting blocking the touch events. It is not documented and I'm not sure what is iOS's intention of doing that. (Could be an iOS bug) The mask view is only for determining the alpha channel for the platform view (for clipping), and we can always ignore the touch events.

Related Issues

Fixes flutter/flutter#66044

Tests

I added the following tests:

  • testGestureWithMaskViewBlockingPlatformView
    Also updated the PlatformViewForTouchIOSScenario to produce a second frame which reproduces this issue.

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the contributor guide and followed the process outlined there for submitting PRs.
  • I signed the CLA.
  • I read and followed the C++, Objective-C, Java style guides for the engine.
  • I read the tree hygiene wiki page, which explains my responsibilities.
  • I updated/added relevant documentation.
  • All existing and new tests are passing.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Did any tests fail when you ran them? Please read handling breaking changes.

Comment on lines 93 to 98
// // In some scenarios, when we add this view as a maskView of the ChildClippingView, iOS added
// this view as a
// // subview of the ChildClippingView. This results this view blocking touch events on the
// ChildClippingView.
// // So we should always ignore any touch events sent to this view.
// // See https://github.com/flutter/flutter/issues/66044
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: comment needs to be re-formatted.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done! Thanks!

@"-gestureTouchesBegan-gestureTouchesEnded-platformViewTapped");
}

- (void)testGestureWithMaskViewBlockingPlatformView {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this test fail if you comment out FlutterPlatformViews_Internal.mm #L99-102?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

@override
void onDrawFrame() {
// Some iOS gesture recognizers bugs are introduced in the second frame (with a different platform view rect) after laying out the platform view.
// So in this test, we pop 2 frames to ensure that we cover those cases.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pop->push?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, updated to load

}

// In some scenarios, when we add this view as a maskView of the ChildClippingView, iOS added
// this view as a
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you want to cut sentences like this, or does it make sense to manually break the lines below the limit so the autoformater doesn't do something weird?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm this is weird. Fixed

Copy link

@blasten blasten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@cyanglaz cyanglaz merged commit 678653b into flutter:master Sep 21, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 22, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 22, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 22, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 22, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 22, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 22, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 22, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 22, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 22, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 22, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 22, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 22, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 22, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 22, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 22, 2020
christopherfujino added a commit that referenced this pull request Sep 24, 2020
* Update 1.22 engine to use Dart 2.10.0-110.5.beta
* Dark mode friendly iOS debugging message (#21277)
* Update FlutterViewController.mm (#21362)
* Tweaked the label here a little for style ("apps", not "application" -- plural and shorter); and "launching" rather than "re-launching"?
* Fix iOS platform view's mask view blocking touch events. (#21286)
* Disconnect the view's AndroidKeyProcessor when detaching from the engine (#21307)
* Remove extraneous window inset call on IME animation (#21213)
* Retain the WindowInsetsAnimation callback if code shrinking is enabled (#21330)

Co-authored-by: Jenn Magder <[email protected]>
Co-authored-by: Tim Sneath <[email protected]>
Co-authored-by: Chris Yang <[email protected]>
Co-authored-by: Jason Simmons <[email protected]>
Co-authored-by: Gary Qian <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[webview_flutter] Gesture regression on iOS from 1.20.2

3 participants