Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Enhancement] Improved WebView That Supports Async Cancellation + Extensibility #3388

Open
Axemasta opened this issue Nov 14, 2021 · 1 comment
Labels
area-controls-webview WebView legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor proposal/open
Milestone

Comments

@Axemasta
Copy link
Contributor

Summary

In Xamarin.Forms I had raised the following issue & PR:

[Bug] WebView Navigating Race Condition #12720
WebView Navigation Now Uses Deferral Token #14137

A requirement for the app I was developing at the time was to confirm ability to navigate to a site within the webview (if I click microsoft.com, is the user allowed to visit the site). This functionality was restricted by the implementation of the webview the cancellation result could not be delayed.

I raised an initial PR converting the implementation to use a task. @PureWeen had a great suggestion of using a DeferralToken to control the navigation, making the whole cancellation process easier for multiple subscribers to halt. I have raised that PR but it contains a break to the API, so is not suitable for Xamarin Forms.

I had business needs to implement this feature as any hacks / work arounds carried other bugs which were not ideal. I forked the WebView control and currently use an alternate implementation of WebView which contains the changes I made in this PR + extra nice to haves.

I was reading the implementation of WebView in MAUI and its the same as in forms, meaning that the issue will persist in MAUI. Since MAUI is in development, I think its the perfect time to revisit the API of WebView, make the control more powerful aswell as addressing some pain points I experienced refactoring the code for my control.

API Changes

Change WebViewNavigatingArgs:

  • Remove Cancel boolean
  • Add DeferralToken property
  • Cancellation is performed in the following manor:
private async void OnNavigating(object sender, WebNavigatingEventArgs e)
{
    if (e.CanCancel)
    {
        var token = e.GetDeferral();

        bool canBrowse = await CanBrowse(e.Url);

        if (!canBrowse)
        {
            e.Cancel();
        }

        token.Complete();
    }
}

The API changes I made in my XF PR ended up refactoring the deferral token code from Shell & making it more generic. I also changed the navigating args to implement an interface instead of inheriting an abstract base:

public interface IWebNavigationEventArgs
{
    WebNavigationEvent NavigationEvent { get; }

    WebViewSource Source { get; }

    string Url { get; }
}

This was so I could make the DeferralToken available to the event args by inheriting a base class.

Intended Use Case

This will generally improve the experience developers have with WebView. Developers can confirm navigation without having a race condition present when navigating. The only way I could get around this issue in XF without my custom SuperWebView was to cancel navigation immediately, run a task then re-navigate (this was hacky & caused other issues).

Proof Of Concept

I have raised this issue & will put up a PR with my first cut of what I think it should look like. I'm very happy to accept and and all advice for design & implementation + I would like ideas on the best way of testing this functionality.

I've maintained the code for iOS & Android and the implementations vary WILDY between them, so it will end up affecting a lot of code. I think there will need to be a lot of testing to make this feature a success.

@jfversluis jfversluis added the legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor label Mar 30, 2022
@jfversluis jfversluis added this to the Backlog milestone Aug 12, 2022
@ghost
Copy link

ghost commented Aug 12, 2022

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-controls-webview WebView legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor proposal/open
Projects
None yet
Development

No branches or pull requests

3 participants