Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

WebView Navigation Can Now Be Cancelled Using A Task #12720 #12756

Closed
wants to merge 7 commits into from

Conversation

Axemasta
Copy link

@Axemasta Axemasta commented Nov 6, 2020

Issues Resolved

API Changes

Added:

  • public Func<Task<bool>> CancelTask { get; set; } to WebNavigatingEventArgs

Platforms Affected

  • iOS
  • Android

Behavioral/Visual Changes

By default there are no behavioural changes, developers would have to opt into this functionality.

I've added the ability for the WebView to cancel navigation asynchronously. In the current implementation the WebView raised WebNavigatingEventArgs which contain public bool Cancel { get; set; }. The value is mutable and will be read by the control to determine whether or not to cancel navigation.

From WkWebViewRenderer, Line 777-781:

var args = new WebNavigatingEventArgs(navEvent, new UrlWebViewSource { Url = lastUrl }, lastUrl);

WebView.SendNavigating(args);
_renderer.UpdateCanGoBackForward();
decisionHandler(args.Cancel ? WKNavigationActionPolicy.Cancel : WKNavigationActionPolicy.Allow);

As you can see if args.Cancel is mutated (in this case by the developer writing their forms app) the navigation will be cancelled. However if it takes too long for the developer to update the args.Cancel value then the native control will have already navigated. This becomes restrictive when trying to use Xamarin.Forms to build a WebView based application as conditional access is an important feature.

I have implemented this functionality on iOS and Android, I had raised an issue (#12720) but theres been no discussion on the issue so I've implemented what I think should be updated and want to see what peoples thoughts are.

Before/After Screenshots

Not applicable - No visual changes have been made & the behaviour is difficult to show in a gif.

Testing Procedure

iOS and Android are currently the only platforms affected.

I used the controls gallery app in the Xamarin.Forms solution to test my changes. I created a page with a standard WebView and verified that the Navigating event was being fired. I then wrote a method that would return whether or not I could navigate to a site (synchronously) and tested it was behaving as expected. I then wrapped that method in. a task (and ran a await Task.Delay(2000)) and ran that method, this allowed me to navigate to a site eventhough when my task completed it reported that navigation to site should be blocked. I implemented my code changes ensuring that when i had updated the code the original behaviour was the same as before I did the changes. I didn't check in any changes to the control app etc, I know the Xamarin Community Toolkit has a sample app where you can put your new feature / change.

Once the code was complete I increased the delay on my async task and tested using the WebView. I ran the following test where the facebook.com domain was banned:

  • Load https://google.co.uk
  • Seach for "facebook"
  • Click any link to '.facebook.com`

Google would load and work normally, when clicking through to facebook nothing would happen, I would see through debug logs that the site was being blocked. Whilst the task is running the UI thread is not blocked so the WebView can be interacted with until the task completes.

PR Checklist

  • Targets the correct branch
  • Tests are passing (or failures are unrelated)

Alex Duffell added 3 commits November 4, 2020 15:48
Added a new property which is a Func that returns a Task<bool>. This can be implemented in the Navigating event fired by the WebView and will allow a developer to pass the native renderer some code to execute which will evaluate whether or not to cancel the current navigation. The WebView will wait for this code to execute before continuing.
Implemented the ability to cancel browser navigation using a task. The code here is not as elegant as iOS namely because of the ShouldOverrideUrlLoading method provided by android making it difficult to go to a background thread without using another method to serve as a fire and forget
Removing excess lines & moving methods around
@rmarinho
Copy link
Member

rmarinho commented Nov 6, 2020

This needs rebase on top of Main.

Alex Duffell added 4 commits November 6, 2020 13:44
Added a new property which is a Func that returns a Task<bool>. This can be implemented in the Navigating event fired by the WebView and will allow a developer to pass the native renderer some code to execute which will evaluate whether or not to cancel the current navigation. The WebView will wait for this code to execute before continuing.
Implemented the ability to cancel browser navigation using a task. The code here is not as elegant as iOS namely because of the ShouldOverrideUrlLoading method provided by android making it difficult to go to a background thread without using another method to serve as a fire and forget
Removing excess lines & moving methods around
@Axemasta Axemasta marked this pull request as draft November 6, 2020 15:38
@Axemasta
Copy link
Author

Axemasta commented Nov 6, 2020

@rmarinho I've rebased on main, I also marked the PR as draft because I want to gauge opinion on what I've proposed here 😁

@rmarinho
Copy link
Member

rmarinho commented Nov 6, 2020

@PureWeen is this similar to what you did for navigation in shell? should we use the same/similar approach ?

@PureWeen
Copy link
Contributor

PureWeen commented Nov 6, 2020

@rmarinho yes!

I think we should go the same approach as shell with this
#12039

  • It's closer to what UWP does
  • The advantage of a deferral token is that multiple subscribers can influence the deferral. If you're relying on a task then coordination between multiple subscribers can be awkward

@Axemasta
Copy link
Author

Axemasta commented Nov 6, 2020

@PureWeen I'll have a look at this PR and update mine accordingly 😄

Nice to know there is a pattern I can follow, I wasn't happy with how mine needed an extra property on the event args, when I was originally spitballing this code I was adding methods to set on the webview to invoke the function etc, quite nasty imho.

@Axemasta
Copy link
Author

Axemasta commented Apr 9, 2021

I'm closing this PR as I've opened up a new one with the new implementation (deferral token) discussed here #14137

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.

4 participants