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

Conversation

@hellohuanlin
Copy link
Contributor

@hellohuanlin hellohuanlin commented Nov 26, 2024

This is pretty tricky bug - I suspect that because Apple's internal recognizer caching an outdated state of our delaying recognizer.

The conflict happens between WebKit and platform view's recognizers. It happens to all plugins that uses a WKWebView, or any view that has a similar (unknown) gesture setup.

This fix has to be in the engine (rather than the plugin), because the plugin itself knows nothing about the existence of our delaying recognizer.

Here are the steps of my research for future reference:

  1. The bug only happens when the overlay flutter widgets blocks the gestures for the platform view (e.g. tap on the platform view area when a flutter context menu is displayed). When the bug happens, WKWebView's link doesn't work anymore, however, the link can still be highlighted when tapped.

  2. When I remove the delaying recognizer from platform view, the link became clickable again. This means that the bug is related to some conflict between WKWebView's internal recognizers and our delaying recognizer. This also means that it is not possible to fix this issue at plugin level, which knows nothing about the delaying recognizer.

  3. When we tap on the web view when context menu is displayed, blockGesture will be called, which simply toggles delaying recognizer's state from possible to ended state, meaning it should block all recognizers from the current gesture (and it correctly did so). Then I use dispatch_async and check the state again, and confirmed the state is correctly reset to possible state.

  4. Subsequent tap on web view triggers acceptGesture, which turns the possible state into failed state. This subsequent tap only highlights the link, but not activate the link. This suggests that some internal web kit recognizer that handles the highlight sees the failed state of delaying recognizer (which is correct), but the recognizer that handles the link activation probably sees the stale state of possible or ended (which is outdated old state).

  5. So the solution is trying to make the recognizer "see" the updated state rather than the cached old state.

  6. I tried recreating a new delaying recognizer when blockGesture is called:

- blockGesture {
  delayingRecognizer.state = .ended
  dispatch_async {
    // force re-create the delaying recognizer
  }
}

This fixed the link activation bug, however, when opening the context menu again, the gesture is not blocked anymore. This means web kit internal recognizers likely cache the old delaying recognizer for state update, thus the new instance of delaying recognizer won't work anymore. So we can't change the instance. However, it's a good experiment that confirms my hypothesis that some internal webkit recognizer caches the outdated state of delaying recognizer.

  1. For the above code, rather than using dispatch_async, I also tried dispatch_after, and it turns out that it only works if the dispatch_after delay is 0 - even if the delay is much smaller than 1 frame's time (16.7ms), it doesn't work. This means the state checking happens either at the end of the current run loop, or beginning of the next run loop. (not too important information, but it helps me better understand how UIKit works).

  2. So from 6, we know that we have to keep the original instance of delaying recognizer. I tried toggling recognizer.enabled, it didn't work. I also tried inserting a dummy recognizer, it didn't work. Neither approach triggers the state "refresh" for those webkit internal recognizers.

  3. I tried removing and adding back the delaying recognizer, and it just worked! This means that removing and adding back the delaying recognizer probably triggered UIKit to refresh the states for all its related recognizers (i.e. those recognizers either blocking or being blocked by the delaying recognizer), hence getting the updated state.

List which issues are fixed by this PR. You must list at least one issue.

Fixes flutter/flutter#158961

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

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Copy link
Member

@jmagman jmagman left a comment

Choose a reason for hiding this comment

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

The code LGTM, the behavior RSLGTM but it sounds like you thoroughly tested. Hopefully this gets fixed in a future version of iOS and we can fallback to the old behavior.

Also, you have a merge conflict.

@hellohuanlin hellohuanlin force-pushed the pv_webview_not_tappable branch from 6e9ef31 to 4118f01 Compare December 4, 2024 23:37
@hellohuanlin
Copy link
Contributor Author

@cbracken kind reminder, in case any comments before i land

@hellohuanlin
Copy link
Contributor Author

i think i will verify with 18.2 RC which is out today before landing it

Copy link
Member

@cbracken cbracken left a comment

Choose a reason for hiding this comment

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

LGTM stamp from a Japanese personal seal

@hellohuanlin hellohuanlin added the autosubmit Merge PR when tree becomes green via auto submit App label Dec 6, 2024
@auto-submit auto-submit bot merged commit cc2b613 into flutter:main Dec 6, 2024
30 checks passed
@hellohuanlin hellohuanlin added the cp: beta cherry pick to the beta release candidate branch label Dec 6, 2024
flutteractionsbot pushed a commit to flutteractionsbot/engine that referenced this pull request Dec 6, 2024
This is pretty tricky bug - I suspect that because Apple's internal recognizer caching an outdated state of our delaying recognizer. 

The conflict happens between WebKit and platform view's recognizers. It happens to all plugins that uses a WKWebView, or any view that has a similar (unknown) gesture setup. 

This fix has to be in the engine (rather than the plugin), because the plugin itself knows nothing about the existence of our delaying recognizer. 

Here are the steps of my research for future reference: 

1. The bug only happens when the overlay flutter widgets blocks the gestures for the platform view (e.g. tap on the platform view area when a flutter context menu is displayed). When the bug happens, WKWebView's link doesn't work anymore, however, the link can still be highlighted when tapped. 

2. When I remove the delaying recognizer from platform view, the link became clickable again. This means that the bug is related to some conflict between WKWebView's internal recognizers and our delaying recognizer. This also means that it is not possible to fix this issue at plugin level, which knows nothing about the delaying recognizer. 

3. When we tap on the web view when context menu is displayed, `blockGesture` will be called, which simply toggles delaying recognizer's state from `possible` to `ended` state, meaning it should block all recognizers from the current gesture (and it correctly did so). Then I use `dispatch_async` and check the state again, and confirmed the state is correctly reset to `possible` state. 

4. Subsequent tap on web view triggers `acceptGesture`, which turns the `possible` state into `failed` state. This subsequent tap only highlights the link, but not activate the link. This suggests that some internal web kit recognizer that handles the highlight sees the `failed` state of delaying recognizer (which is correct), but the recognizer that handles the link activation probably sees the stale state of `possible` or `ended` (which is outdated old state). 

5. So the solution is trying to make the recognizer "see" the updated state rather than the cached old state. 

6. I tried recreating a new delaying recognizer when `blockGesture` is called: 
```
- blockGesture {
  delayingRecognizer.state = .ended
  dispatch_async {
    // force re-create the delaying recognizer
  }
}
```
This fixed the link activation bug, however, when opening the context menu again, the gesture is not blocked anymore. This means web kit internal recognizers likely cache the old delaying recognizer for state update, thus the new instance of delaying recognizer won't work anymore. So we can't change the instance. However, it's a good experiment that confirms my hypothesis that some internal webkit recognizer caches the outdated state of delaying recognizer. 

7. For the above code, rather than using `dispatch_async`, I also tried `dispatch_after`, and it turns out that it only works if the dispatch_after delay is `0` - even if the delay is much smaller than 1 frame's time (16.7ms), it doesn't work. This means the state checking happens either at the end of the current run loop, or beginning of the next run loop. (not too important information, but it helps me better understand how UIKit works). 

8. So from 6, we know that we have to keep the original instance of delaying recognizer. I tried toggling `recognizer.enabled`, it didn't work. I also tried inserting a dummy recognizer, it didn't work. Neither approach triggers the state "refresh" for those webkit internal recognizers. 

9. I tried removing and adding back the delaying recognizer, and it just worked! This means that removing and adding back the delaying recognizer probably triggered UIKit to refresh the states for all its related recognizers (i.e. those recognizers either blocking or being blocked by the delaying recognizer), hence getting the updated state. 

*List which issues are fixed by this PR. You must list at least one issue.*

Fixes flutter/flutter#158961

*If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
hellohuanlin added a commit to hellohuanlin/engine that referenced this pull request Dec 6, 2024
This is pretty tricky bug - I suspect that because Apple's internal recognizer caching an outdated state of our delaying recognizer. 

The conflict happens between WebKit and platform view's recognizers. It happens to all plugins that uses a WKWebView, or any view that has a similar (unknown) gesture setup. 

This fix has to be in the engine (rather than the plugin), because the plugin itself knows nothing about the existence of our delaying recognizer. 

Here are the steps of my research for future reference: 

1. The bug only happens when the overlay flutter widgets blocks the gestures for the platform view (e.g. tap on the platform view area when a flutter context menu is displayed). When the bug happens, WKWebView's link doesn't work anymore, however, the link can still be highlighted when tapped. 

2. When I remove the delaying recognizer from platform view, the link became clickable again. This means that the bug is related to some conflict between WKWebView's internal recognizers and our delaying recognizer. This also means that it is not possible to fix this issue at plugin level, which knows nothing about the delaying recognizer. 

3. When we tap on the web view when context menu is displayed, `blockGesture` will be called, which simply toggles delaying recognizer's state from `possible` to `ended` state, meaning it should block all recognizers from the current gesture (and it correctly did so). Then I use `dispatch_async` and check the state again, and confirmed the state is correctly reset to `possible` state. 

4. Subsequent tap on web view triggers `acceptGesture`, which turns the `possible` state into `failed` state. This subsequent tap only highlights the link, but not activate the link. This suggests that some internal web kit recognizer that handles the highlight sees the `failed` state of delaying recognizer (which is correct), but the recognizer that handles the link activation probably sees the stale state of `possible` or `ended` (which is outdated old state). 

5. So the solution is trying to make the recognizer "see" the updated state rather than the cached old state. 

6. I tried recreating a new delaying recognizer when `blockGesture` is called: 
```
- blockGesture {
  delayingRecognizer.state = .ended
  dispatch_async {
    // force re-create the delaying recognizer
  }
}
```
This fixed the link activation bug, however, when opening the context menu again, the gesture is not blocked anymore. This means web kit internal recognizers likely cache the old delaying recognizer for state update, thus the new instance of delaying recognizer won't work anymore. So we can't change the instance. However, it's a good experiment that confirms my hypothesis that some internal webkit recognizer caches the outdated state of delaying recognizer. 

7. For the above code, rather than using `dispatch_async`, I also tried `dispatch_after`, and it turns out that it only works if the dispatch_after delay is `0` - even if the delay is much smaller than 1 frame's time (16.7ms), it doesn't work. This means the state checking happens either at the end of the current run loop, or beginning of the next run loop. (not too important information, but it helps me better understand how UIKit works). 

8. So from 6, we know that we have to keep the original instance of delaying recognizer. I tried toggling `recognizer.enabled`, it didn't work. I also tried inserting a dummy recognizer, it didn't work. Neither approach triggers the state "refresh" for those webkit internal recognizers. 

9. I tried removing and adding back the delaying recognizer, and it just worked! This means that removing and adding back the delaying recognizer probably triggered UIKit to refresh the states for all its related recognizers (i.e. those recognizers either blocking or being blocked by the delaying recognizer), hence getting the updated state. 

*List which issues are fixed by this PR. You must list at least one issue.*

Fixes flutter/flutter#158961

*If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Dec 6, 2024
github-merge-queue bot pushed a commit to flutter/flutter that referenced this pull request Dec 7, 2024
…159913)

flutter/engine@d42645f...de53ed5

2024-12-06 [email protected] [Impeller] store vertex buffers in
render pass for gles. (flutter/engine#56991)
2024-12-06 [email protected] [Impeller] Add keep alive for 4
frames in render target cache. (flutter/engine#57020)
2024-12-06 [email protected]
[ios][platform_view] workaround for non-tappable webview
(flutter/engine#56804)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC [email protected],[email protected] on the revert to ensure that a
human
is aware of the problem.

To file a bug in Flutter:
https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
hellohuanlin added a commit to hellohuanlin/engine that referenced this pull request Dec 9, 2024
This is pretty tricky bug - I suspect that because Apple's internal recognizer caching an outdated state of our delaying recognizer. 

The conflict happens between WebKit and platform view's recognizers. It happens to all plugins that uses a WKWebView, or any view that has a similar (unknown) gesture setup. 

This fix has to be in the engine (rather than the plugin), because the plugin itself knows nothing about the existence of our delaying recognizer. 

Here are the steps of my research for future reference: 

1. The bug only happens when the overlay flutter widgets blocks the gestures for the platform view (e.g. tap on the platform view area when a flutter context menu is displayed). When the bug happens, WKWebView's link doesn't work anymore, however, the link can still be highlighted when tapped. 

2. When I remove the delaying recognizer from platform view, the link became clickable again. This means that the bug is related to some conflict between WKWebView's internal recognizers and our delaying recognizer. This also means that it is not possible to fix this issue at plugin level, which knows nothing about the delaying recognizer. 

3. When we tap on the web view when context menu is displayed, `blockGesture` will be called, which simply toggles delaying recognizer's state from `possible` to `ended` state, meaning it should block all recognizers from the current gesture (and it correctly did so). Then I use `dispatch_async` and check the state again, and confirmed the state is correctly reset to `possible` state. 

4. Subsequent tap on web view triggers `acceptGesture`, which turns the `possible` state into `failed` state. This subsequent tap only highlights the link, but not activate the link. This suggests that some internal web kit recognizer that handles the highlight sees the `failed` state of delaying recognizer (which is correct), but the recognizer that handles the link activation probably sees the stale state of `possible` or `ended` (which is outdated old state). 

5. So the solution is trying to make the recognizer "see" the updated state rather than the cached old state. 

6. I tried recreating a new delaying recognizer when `blockGesture` is called: 
```
- blockGesture {
  delayingRecognizer.state = .ended
  dispatch_async {
    // force re-create the delaying recognizer
  }
}
```
This fixed the link activation bug, however, when opening the context menu again, the gesture is not blocked anymore. This means web kit internal recognizers likely cache the old delaying recognizer for state update, thus the new instance of delaying recognizer won't work anymore. So we can't change the instance. However, it's a good experiment that confirms my hypothesis that some internal webkit recognizer caches the outdated state of delaying recognizer. 

7. For the above code, rather than using `dispatch_async`, I also tried `dispatch_after`, and it turns out that it only works if the dispatch_after delay is `0` - even if the delay is much smaller than 1 frame's time (16.7ms), it doesn't work. This means the state checking happens either at the end of the current run loop, or beginning of the next run loop. (not too important information, but it helps me better understand how UIKit works). 

8. So from 6, we know that we have to keep the original instance of delaying recognizer. I tried toggling `recognizer.enabled`, it didn't work. I also tried inserting a dummy recognizer, it didn't work. Neither approach triggers the state "refresh" for those webkit internal recognizers. 

9. I tried removing and adding back the delaying recognizer, and it just worked! This means that removing and adding back the delaying recognizer probably triggered UIKit to refresh the states for all its related recognizers (i.e. those recognizers either blocking or being blocked by the delaying recognizer), hence getting the updated state. 

*List which issues are fixed by this PR. You must list at least one issue.*

Fixes flutter/flutter#158961

*If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
hellohuanlin added a commit to hellohuanlin/engine that referenced this pull request Dec 9, 2024
This is pretty tricky bug - I suspect that because Apple's internal recognizer caching an outdated state of our delaying recognizer. 

The conflict happens between WebKit and platform view's recognizers. It happens to all plugins that uses a WKWebView, or any view that has a similar (unknown) gesture setup. 

This fix has to be in the engine (rather than the plugin), because the plugin itself knows nothing about the existence of our delaying recognizer. 

Here are the steps of my research for future reference: 

1. The bug only happens when the overlay flutter widgets blocks the gestures for the platform view (e.g. tap on the platform view area when a flutter context menu is displayed). When the bug happens, WKWebView's link doesn't work anymore, however, the link can still be highlighted when tapped. 

2. When I remove the delaying recognizer from platform view, the link became clickable again. This means that the bug is related to some conflict between WKWebView's internal recognizers and our delaying recognizer. This also means that it is not possible to fix this issue at plugin level, which knows nothing about the delaying recognizer. 

3. When we tap on the web view when context menu is displayed, `blockGesture` will be called, which simply toggles delaying recognizer's state from `possible` to `ended` state, meaning it should block all recognizers from the current gesture (and it correctly did so). Then I use `dispatch_async` and check the state again, and confirmed the state is correctly reset to `possible` state. 

4. Subsequent tap on web view triggers `acceptGesture`, which turns the `possible` state into `failed` state. This subsequent tap only highlights the link, but not activate the link. This suggests that some internal web kit recognizer that handles the highlight sees the `failed` state of delaying recognizer (which is correct), but the recognizer that handles the link activation probably sees the stale state of `possible` or `ended` (which is outdated old state). 

5. So the solution is trying to make the recognizer "see" the updated state rather than the cached old state. 

6. I tried recreating a new delaying recognizer when `blockGesture` is called: 
```
- blockGesture {
  delayingRecognizer.state = .ended
  dispatch_async {
    // force re-create the delaying recognizer
  }
}
```
This fixed the link activation bug, however, when opening the context menu again, the gesture is not blocked anymore. This means web kit internal recognizers likely cache the old delaying recognizer for state update, thus the new instance of delaying recognizer won't work anymore. So we can't change the instance. However, it's a good experiment that confirms my hypothesis that some internal webkit recognizer caches the outdated state of delaying recognizer. 

7. For the above code, rather than using `dispatch_async`, I also tried `dispatch_after`, and it turns out that it only works if the dispatch_after delay is `0` - even if the delay is much smaller than 1 frame's time (16.7ms), it doesn't work. This means the state checking happens either at the end of the current run loop, or beginning of the next run loop. (not too important information, but it helps me better understand how UIKit works). 

8. So from 6, we know that we have to keep the original instance of delaying recognizer. I tried toggling `recognizer.enabled`, it didn't work. I also tried inserting a dummy recognizer, it didn't work. Neither approach triggers the state "refresh" for those webkit internal recognizers. 

9. I tried removing and adding back the delaying recognizer, and it just worked! This means that removing and adding back the delaying recognizer probably triggered UIKit to refresh the states for all its related recognizers (i.e. those recognizers either blocking or being blocked by the delaying recognizer), hence getting the updated state. 

*List which issues are fixed by this PR. You must list at least one issue.*

Fixes flutter/flutter#158961

*If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
hellohuanlin added a commit to hellohuanlin/engine that referenced this pull request Dec 13, 2024
This is pretty tricky bug - I suspect that because Apple's internal recognizer caching an outdated state of our delaying recognizer. 

The conflict happens between WebKit and platform view's recognizers. It happens to all plugins that uses a WKWebView, or any view that has a similar (unknown) gesture setup. 

This fix has to be in the engine (rather than the plugin), because the plugin itself knows nothing about the existence of our delaying recognizer. 

Here are the steps of my research for future reference: 

1. The bug only happens when the overlay flutter widgets blocks the gestures for the platform view (e.g. tap on the platform view area when a flutter context menu is displayed). When the bug happens, WKWebView's link doesn't work anymore, however, the link can still be highlighted when tapped. 

2. When I remove the delaying recognizer from platform view, the link became clickable again. This means that the bug is related to some conflict between WKWebView's internal recognizers and our delaying recognizer. This also means that it is not possible to fix this issue at plugin level, which knows nothing about the delaying recognizer. 

3. When we tap on the web view when context menu is displayed, `blockGesture` will be called, which simply toggles delaying recognizer's state from `possible` to `ended` state, meaning it should block all recognizers from the current gesture (and it correctly did so). Then I use `dispatch_async` and check the state again, and confirmed the state is correctly reset to `possible` state. 

4. Subsequent tap on web view triggers `acceptGesture`, which turns the `possible` state into `failed` state. This subsequent tap only highlights the link, but not activate the link. This suggests that some internal web kit recognizer that handles the highlight sees the `failed` state of delaying recognizer (which is correct), but the recognizer that handles the link activation probably sees the stale state of `possible` or `ended` (which is outdated old state). 

5. So the solution is trying to make the recognizer "see" the updated state rather than the cached old state. 

6. I tried recreating a new delaying recognizer when `blockGesture` is called: 
```
- blockGesture {
  delayingRecognizer.state = .ended
  dispatch_async {
    // force re-create the delaying recognizer
  }
}
```
This fixed the link activation bug, however, when opening the context menu again, the gesture is not blocked anymore. This means web kit internal recognizers likely cache the old delaying recognizer for state update, thus the new instance of delaying recognizer won't work anymore. So we can't change the instance. However, it's a good experiment that confirms my hypothesis that some internal webkit recognizer caches the outdated state of delaying recognizer. 

7. For the above code, rather than using `dispatch_async`, I also tried `dispatch_after`, and it turns out that it only works if the dispatch_after delay is `0` - even if the delay is much smaller than 1 frame's time (16.7ms), it doesn't work. This means the state checking happens either at the end of the current run loop, or beginning of the next run loop. (not too important information, but it helps me better understand how UIKit works). 

8. So from 6, we know that we have to keep the original instance of delaying recognizer. I tried toggling `recognizer.enabled`, it didn't work. I also tried inserting a dummy recognizer, it didn't work. Neither approach triggers the state "refresh" for those webkit internal recognizers. 

9. I tried removing and adding back the delaying recognizer, and it just worked! This means that removing and adding back the delaying recognizer probably triggered UIKit to refresh the states for all its related recognizers (i.e. those recognizers either blocking or being blocked by the delaying recognizer), hence getting the updated state. 

*List which issues are fixed by this PR. You must list at least one issue.*

Fixes flutter/flutter#158961

*If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
auto-submit bot pushed a commit that referenced this pull request Dec 13, 2024
…recursively (#57168)

The original workaround ([PR](#56804)) works for the official web view plugin, but it doesn't work for a third party plugin `flutter_inappwebview` ([issue](https://github.com/pichillilorenzo/flutter_inappwebview)). Upon discussion with the author of that plugin, it turns out that their platform view is not a WKWebView, but rather a wrapper of WKWebView. 

This PR performs a DFS search of the view hierarchy, and enable the workaround as long as there's a WKWebView inside. 

TODO: pending sample project:
I am quite positive that it should work, but **I haven't tried it since I don't have a sample project yet**. I have requested a sample project with them so I can verify the solution. 

*List which issues are fixed by this PR. You must list at least one issue.*

 pichillilorenzo/flutter_inappwebview#2415

*If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
auto-submit bot pushed a commit that referenced this pull request Dec 17, 2024
… AND "enable the webview non tappable workaround by checking subviews recursively #57168" (#57176)

CP for 3 PRs: #56804 and #57168 and  #57193

This is for 3.28.

Since the previous PR was not merged, so I combined the 2 PRs to make it easier to merge. 

*List which issues are fixed by this PR. You must list at least one issue.*
pichillilorenzo/flutter_inappwebview#2415

*If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
nick9822 pushed a commit to nick9822/flutter that referenced this pull request Dec 18, 2024
…ine#56804)

This is pretty tricky bug - I suspect that because Apple's internal recognizer caching an outdated state of our delaying recognizer. 

The conflict happens between WebKit and platform view's recognizers. It happens to all plugins that uses a WKWebView, or any view that has a similar (unknown) gesture setup. 

This fix has to be in the engine (rather than the plugin), because the plugin itself knows nothing about the existence of our delaying recognizer. 

Here are the steps of my research for future reference: 

1. The bug only happens when the overlay flutter widgets blocks the gestures for the platform view (e.g. tap on the platform view area when a flutter context menu is displayed). When the bug happens, WKWebView's link doesn't work anymore, however, the link can still be highlighted when tapped. 

2. When I remove the delaying recognizer from platform view, the link became clickable again. This means that the bug is related to some conflict between WKWebView's internal recognizers and our delaying recognizer. This also means that it is not possible to fix this issue at plugin level, which knows nothing about the delaying recognizer. 

3. When we tap on the web view when context menu is displayed, `blockGesture` will be called, which simply toggles delaying recognizer's state from `possible` to `ended` state, meaning it should block all recognizers from the current gesture (and it correctly did so). Then I use `dispatch_async` and check the state again, and confirmed the state is correctly reset to `possible` state. 

4. Subsequent tap on web view triggers `acceptGesture`, which turns the `possible` state into `failed` state. This subsequent tap only highlights the link, but not activate the link. This suggests that some internal web kit recognizer that handles the highlight sees the `failed` state of delaying recognizer (which is correct), but the recognizer that handles the link activation probably sees the stale state of `possible` or `ended` (which is outdated old state). 

5. So the solution is trying to make the recognizer "see" the updated state rather than the cached old state. 

6. I tried recreating a new delaying recognizer when `blockGesture` is called: 
```
- blockGesture {
  delayingRecognizer.state = .ended
  dispatch_async {
    // force re-create the delaying recognizer
  }
}
```
This fixed the link activation bug, however, when opening the context menu again, the gesture is not blocked anymore. This means web kit internal recognizers likely cache the old delaying recognizer for state update, thus the new instance of delaying recognizer won't work anymore. So we can't change the instance. However, it's a good experiment that confirms my hypothesis that some internal webkit recognizer caches the outdated state of delaying recognizer. 

7. For the above code, rather than using `dispatch_async`, I also tried `dispatch_after`, and it turns out that it only works if the dispatch_after delay is `0` - even if the delay is much smaller than 1 frame's time (16.7ms), it doesn't work. This means the state checking happens either at the end of the current run loop, or beginning of the next run loop. (not too important information, but it helps me better understand how UIKit works). 

8. So from 6, we know that we have to keep the original instance of delaying recognizer. I tried toggling `recognizer.enabled`, it didn't work. I also tried inserting a dummy recognizer, it didn't work. Neither approach triggers the state "refresh" for those webkit internal recognizers. 

9. I tried removing and adding back the delaying recognizer, and it just worked! This means that removing and adding back the delaying recognizer probably triggered UIKit to refresh the states for all its related recognizers (i.e. those recognizers either blocking or being blocked by the delaying recognizer), hence getting the updated state. 

*List which issues are fixed by this PR. You must list at least one issue.*

Fixes flutter#158961

*If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
nick9822 pushed a commit to nick9822/flutter that referenced this pull request Dec 18, 2024
…recursively (flutter/engine#57168)

The original workaround ([PR](flutter/engine#56804)) works for the official web view plugin, but it doesn't work for a third party plugin `flutter_inappwebview` ([issue](https://github.com/pichillilorenzo/flutter_inappwebview)). Upon discussion with the author of that plugin, it turns out that their platform view is not a WKWebView, but rather a wrapper of WKWebView. 

This PR performs a DFS search of the view hierarchy, and enable the workaround as long as there's a WKWebView inside. 

TODO: pending sample project:
I am quite positive that it should work, but **I haven't tried it since I don't have a sample project yet**. I have requested a sample project with them so I can verify the solution. 

*List which issues are fixed by this PR. You must list at least one issue.*

 pichillilorenzo/flutter_inappwebview#2415

*If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

autosubmit Merge PR when tree becomes green via auto submit App cp: beta cherry pick to the beta release candidate branch platform-ios

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tap interactions not working on iOS 18.2 beta for webview_flutter 4.10.0

3 participants