[Enhancement] Add EvaluateJavaScript method to WebView #2140

alanag13 · 2018-03-20T02:24:55Z

Description of Change

fixes #1699 .

Adds the ability to execute and return the result of a passed javascript string from a platform's webview.

Platform support:

API Changes

Added:
Task WebView.EvaluateJavaScriptAsync

PR Checklist

Has tests (if omitted, state reason in description)
Rebased on top of master at time of PR
Changes adhere to coding standard
Consolidate commits as makes sense

alanag13 · 2018-03-20T02:25:39Z

Question: should WebView.Eval be deprecated in favor of this?

alanag13 · 2018-03-20T02:32:40Z

Xamarin.Forms.Core/WebView.cs

+			if (Xamarin.Forms.Device.RuntimePlatform != "Android")
+			{
+				script = EscapeJsString(script);
+				script = "try{JSON.stringify(eval('" + script + "'))}catch(e){'null'};";


A fair amount of effort was involved in finding a way to be able to pass a js string that would resolve the same on all platforms due to some inconsistencies:

UWP/WPF only returns a value if the result of the function is a string and throws a generic exception otherwise (even for what I would think should be easily convertible types like bool and int). It also throws if there is an error in the script.

iOS/macOS will return String.Empty when the result is an array, object, or there is a script error.

Android was the only implementation that nicely handled all types. Wrapping the result of the script in JSON.stringify and catching script errors allows all platforms to behave consistently.

Does the use of eval affect CSP?

alanag13 · 2018-03-20T17:01:48Z

@rmarinho can you give me any insight into why this failed (it looks like only on Windows)?

hartez · 2018-03-20T19:20:16Z

can you give me any insight into why this failed (it looks like only on Windows)?

@alanag13 Nothing wrong on your end - the Windows build failure was a VSTS hiccup. That build stage should be green now.

alanag13 · 2018-03-21T05:41:56Z

Xamarin.Forms.Core/WebView.cs

@@ -184,7 +178,7 @@ public void SendNavigating(WebNavigatingEventArgs args)
 			return _platformConfigurationRegistry.Value.On<T>();
 		}

-		private static string EscapeJsString(string js)
+		static string EscapeJsString(string js)


Because we add an extra eval function in the code I added for standardizing platforms, we need to add an extra level of escaping to any single quotes the user may have passed in for their script. I really don't like this but it was all I could come up with to solve this problem. I'm open to suggestions here.

alanag13 · 2018-03-21T06:01:40Z

GitHub is hiding a comment I made on WebView.cs, since I made a change to it after opening this PR, so just make sure you show outdated or go to the full changes to see it.

rmarinho

Just a couple of checks and recomendations

rmarinho · 2018-03-21T11:28:28Z

Xamarin.Forms.Core/WebView.cs

+
+		static string EscapeJsString(string js)
+		{
+			if (!js.Contains("'"))


check if this is null or it will throw NRE

Good catch, will do

rmarinho · 2018-03-21T11:30:58Z

Xamarin.Forms.Platform.GTK/Renderers/WebViewRenderer.cs

+        {
+	       if (Control != null)
+	       {
+		       Control.ExecuteScript(script);


Control?.ExecuteScript(script);

rmarinho · 2018-03-21T11:31:28Z

Xamarin.Forms.Platform.GTK/Renderers/WebViewRenderer.cs

+	       {
+		       Control.ExecuteScript(script);
+	       }
+               return null;


Should we always return null?

Did this per @jassmith's recommendation in the issue thread: #1699

rmarinho · 2018-03-21T11:32:29Z

Xamarin.Forms.Platform.UAP/WebViewRenderer.cs

@@ -140,6 +143,11 @@ void Load()
 			await Control.Dispatcher.RunAsync(CoreDispatcherPriority.Normal, async () => await Control.InvokeScriptAsync("eval", new[] { eventArg.Script }));
 		}

+		async Task<string> OnEvaluateJavaScriptRequested(string script)
+		{
+				return await Control.InvokeScriptAsync("eval", new[] { script });


Code style issue, extra tabs I think

alanag13 · 2018-03-22T04:39:28Z

@rmarinho updated with your changes

alanag13 · 2018-03-22T15:46:41Z

@rmarinho @samhouts are the ui tests failures unrelated? these were all green yesterday before my most recent commit, which seems like it should be pretty harmless . Should I rebase?

jassmith · 2018-03-22T17:20:44Z

Doesn't impact fast renderers, therefor merging :)

alanag13 · 2018-03-22T18:03:02Z

@jassmith do we want to deprecate Eval in favor of this? To users, the only difference is that this returns a value. Only reason I can think of to not do this is that GTK and Tizen always return null , but it still seems potentially confusing to have both Eval and EvaluateJavaScriptAsync.

I can open another PR to do that if you think it's the right call, but let me know the version number you want to appear in the deprecation message.

alanag13 added 6 commits March 19, 2018 21:08

Add EvaluateJavaScript method to return string result of js calls

11cf688

add docs

d162cc0

account for some quirks between platforms

019097b

cleanup

75e424e

edge cases, standardization, and error handling

5c5d408

early escape

09fc1f7

alanag13 commented Mar 20, 2018

View reviewed changes

add null check

e3609ef

rmarinho added t/enhancement ➕ F100 labels Mar 20, 2018

samhouts changed the title ~~Feature/1699 webview evaluate js~~ [Enhancement] Add EvaluateJavaScript method to WebView Mar 20, 2018

xamarin-release-manager added the API-change Heads-up to reviewers that this PR may contain an API change label Mar 20, 2018

fix code styles

a545102

alanag13 commented Mar 21, 2018

View reviewed changes

clarify sort

45cad38

xamarin-release-manager added the API-change Heads-up to reviewers that this PR may contain an API change label Mar 21, 2018

rmarinho suggested changes Mar 21, 2018

View reviewed changes

code styles adjustment and null check

99c46e5

rmarinho approved these changes Mar 22, 2018

View reviewed changes

xamarin-release-manager added the API-change Heads-up to reviewers that this PR may contain an API change label Mar 22, 2018

rmarinho requested a review from samhouts March 22, 2018 14:19

jassmith approved these changes Mar 22, 2018

View reviewed changes

jassmith merged commit 578c0eb into xamarin:master Mar 22, 2018

samhouts added this to the 3.1.0 milestone May 5, 2018

samhouts modified the milestone: 3.1.0 Jun 1, 2018

samhouts added the community-sprint label Nov 2, 2018

hartez mentioned this pull request May 23, 2023

[Windows] WebView EvaluateJavaScriptAsync returns an incorrect result dotnet/maui#15233

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Enhancement] Add EvaluateJavaScript method to WebView #2140

[Enhancement] Add EvaluateJavaScript method to WebView #2140

alanag13 commented Mar 20, 2018 •

edited by samhouts

Loading

alanag13 commented Mar 20, 2018

alanag13 Mar 20, 2018 •

edited

Loading

jerone Mar 24, 2018

alanag13 commented Mar 20, 2018

hartez commented Mar 20, 2018

alanag13 Mar 21, 2018

alanag13 commented Mar 21, 2018

rmarinho left a comment

rmarinho Mar 21, 2018

alanag13 Mar 21, 2018

rmarinho Mar 21, 2018

rmarinho Mar 21, 2018

alanag13 Mar 21, 2018

rmarinho Mar 21, 2018

alanag13 commented Mar 22, 2018

alanag13 commented Mar 22, 2018

jassmith commented Mar 22, 2018

alanag13 commented Mar 22, 2018

[Enhancement] Add EvaluateJavaScript method to WebView #2140

[Enhancement] Add EvaluateJavaScript method to WebView #2140

Conversation

alanag13 commented Mar 20, 2018 • edited by samhouts Loading

Description of Change

API Changes

PR Checklist

alanag13 commented Mar 20, 2018

alanag13 Mar 20, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alanag13 commented Mar 20, 2018

hartez commented Mar 20, 2018

Choose a reason for hiding this comment

alanag13 commented Mar 21, 2018

rmarinho left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alanag13 commented Mar 22, 2018

alanag13 commented Mar 22, 2018

jassmith commented Mar 22, 2018

alanag13 commented Mar 22, 2018

alanag13 commented Mar 20, 2018 •

edited by samhouts

Loading

alanag13 Mar 20, 2018 •

edited

Loading