[webview_flutter] Webiew post url support #1970

charafau · 2019-08-11T03:24:56Z

Description

Added initial post parameters - first request to the webpage will be POST request

Related Issues

flutter/flutter#27730
flutter/flutter#29930

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.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

No, this is not a breaking change.

charafau · 2019-08-15T16:38:58Z

I would like to ask e2e test but that would require adding support for POST headers to https://flutter-header-echo.herokuapp.com/ would that be possible?

amirh · 2019-08-15T22:07:38Z

(Haven't reviewed yet, just replying on your testing comment:)

What I'd really want to experiment with is launching a local HTTP server as part of the test, this would have unblocked other scenarios we couldn't test as well. It may as well be too much out of scope for this PR though.

amirh

Thanks left a few comments, as said we would need an e2e test before this can land.

amirh · 2019-10-10T15:34:13Z

.../webview_flutter/android/src/main/java/io/flutter/plugins/webviewflutter/FlutterWebView.java

@@ -135,6 +149,20 @@ public void onMethodCall(MethodCall methodCall, Result result) {
    }
  }

+  private static String initialParametersToString(Map<String, Object> parameters)


Expecting a Map -> application/x-www-form-urlencoded encoding in each platform implementation sounds a little fragile in case these implementations don't exactly match.

My preference would be to take a similar approach to Android's WebView and WkWebView where the encoding is the responsibility of the app developer, which allows some more flexibility for developers.

I think this is how you're suppose to do that, webview send form by post with this.

amirh · 2019-10-10T15:37:24Z

packages/webview_flutter/ios/Classes/FlutterWebView.m

+  NSMutableString* resultString = [NSMutableString string];
+  for (NSString* key in [params allKeys]) {
+    if ([resultString length] > 0) [resultString appendString:@"&"];
+    [resultString appendFormat:@"%@=%@", key, [params objectForKey:key]];


These values would have to be url encoded as well to match the Android implementation.
Anyway as mentioned in previous comment I think we better not make this the responsibility of the platform implementation.

I changed implementation so developer can send encoded string by himself

amirh · 2019-10-10T15:38:29Z

packages/webview_flutter/ios/Classes/FlutterWebView.m

+  [request setHTTPMethod:@"POST"];
+  [request setHTTPBody:postData];
+  [request setValue:contentLength forHTTPHeaderField:@"Content-Length"];
+  [request setValue:@"application/x-www-form-urlencoded" forHTTPHeaderField:@"Content-Type"];


Seems like the Android webview forces this encoding, do you know if multipart/form-data is allowed with WkWebView?

I don't know if you can send multipart like that, but code I've seen uses form encoded only

amirh · 2019-10-10T15:40:52Z

packages/webview_flutter/lib/webview_flutter.dart

@@ -134,6 +134,7 @@ class WebView extends StatefulWidget {
    Key key,
    this.onWebViewCreated,
    this.initialUrl,
+    this.initialPostParameters,


I think we better start with a WebViewController.postUrl which covers the more generic use case (posting at any point). The initialUrl is a convenience mechanism for simple cases, I'd be hesitant to add complexity to it.

Moved method to controller as suggested :)

charafau · 2019-10-14T21:06:22Z

Thank you for review, need to prepare for presentation now, but will come back to it in 2 weeks.

Moved encoding to client side

charafau · 2019-11-21T23:48:54Z

It took me longer than I thought it would. Sorry for delay. Still I don't have e2e test for that, would it be possible to add endpoint to server?

stuartmorgan · 2021-04-06T20:31:06Z

We apologize for the long delay in triaging this PR. We’re in the process of overhauling our PR triage system to respond much more quickly, as well as working through the backlog.

Can you clarify your last question? What server are you wanting to add an endpoint to? (I suspect the best way to unblock an end-to-end test here would be a local webserver as discussed above, but if there are other options we can certainly explore them.)

charafau · 2021-04-08T13:00:17Z

@stuartmorgan That was some time ago but as far as I remeber, I wanted to write test to check if webview actually make that POST request. My idea was to integration test to this server https://github.com/flutter/plugins/blob/master/packages/webview_flutter/example/integration_test/webview_flutter_test.dart#L95 and check if it returns different response for POST

stuartmorgan · 2021-04-27T20:12:27Z

@iskakaushik Do you know what https://flutter-header-echo.herokuapp.com/ is? Do we own it?

It seems problematic to have our integration test depending on a service that we don't have a clearly documented handling of (ownership, update process, etc.). I'm curious if there is a reason we did that originally vs. using a local server.

charafau · 2021-04-28T01:49:01Z

@stuartmorgan I tried to start dart http server inside test but from what I find, you'd have to run it in different isolate and that's not possible to do when test run

stuartmorgan · 2021-05-04T20:19:00Z

@iskakaushik Ping on the server question above?

stuartmorgan · 2021-05-04T20:22:00Z

you'd have to run it in different isolate and that's not possible to do when test run

Can you elaborate on this? I.e., do you have docs or other resources to link to? (I'm just not familiar with this restriction, and would like to investigate further.)

charafau · 2021-06-02T09:50:19Z

@stuartmorgan sorry for long reply, checked if I have my old code but it's lost. I checked flutter issues and other people have the same problems with isolates in flutter driver tests:

flutter/flutter#24703
I'm fine with adding this as isolate but I guess server is there for some reason

stuartmorgan · 2021-06-02T15:50:09Z

@stuartmorgan sorry for long reply, checked if I have my old code but it's lost. I checked flutter issues and other people have the same problems with isolates in flutter driver tests:

flutter/flutter#24703
I'm fine with adding this as isolate but I guess server is there for some reason

It sounds like it wouldn't have worked as an isolate at the time the server was added. If that's no longer the case though, and we can make this self-contained, that would be far better than having an outside server; we're seeing here the issues with the server approach, where it's non-obvious how to update the one that's there (or even whether we still can).

charafau · 2021-06-02T18:06:38Z

makes sense. I will check if that's still an issue (PR is quite old) and come back with the results.

stuartmorgan · 2021-10-12T15:42:53Z

@charafau Are you still planning on moving forward with this?

charafau · 2021-10-13T09:12:38Z

@stuartmorgan I completely forgot about it. I don't have time right now so I will leave the issue to implement for someone else.

Hixie · 2021-10-26T23:53:05Z

I'm going to close this and leave a note on the issue; if anyone wants to pick this back up please don't hesitate to do so.

charafau added 3 commits July 25, 2019 02:19

added postUrl for Android webview

9f701dd

Merge branch 'master' into webiew_post_url_support

c2606df

added ios implementation

4a65213

charafau requested a review from amirh as a code owner August 11, 2019 03:24

googlebot added the cla: yes label Aug 11, 2019

escamoteur added the needs love label Sep 24, 2019

amirh suggested changes Oct 10, 2019

View reviewed changes

collinjackson added the in review label Oct 24, 2019

Merge branch 'master' into webiew_post_url_support

9dfdeee

liang530 pushed a commit to huosdk/plugins that referenced this pull request Nov 12, 2019

Android webview add post flutter#1970

a61d57c

charafau added 2 commits November 20, 2019 13:18

Changed initial post parameters to controller postUrl

6a913c8

Moved encoding to client side

Merge branch 'master' into webiew_post_url_support

27260e0

charafau force-pushed the webiew_post_url_support branch from 772ae65 to 27260e0 Compare November 20, 2019 13:52

reformatted code

0596136

charafau requested a review from amirh November 21, 2019 23:47

stuartmorgan added the p: webview_flutter Edits files for a webview_flutter plugin label Jan 29, 2021

stuartmorgan added the needs tests label Sep 2, 2021

stuartmorgan removed the needs love label Sep 20, 2021

stuartmorgan added the needs new owner label Oct 15, 2021

Hixie mentioned this pull request Oct 26, 2021

WebView for Flutter - Allow the POST of data on initial URL (instead of a GET) flutter/flutter#27730

Closed

Hixie closed this Oct 26, 2021

github-actions bot added platform-android platform-ios labels Oct 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[webview_flutter] Webiew post url support #1970

[webview_flutter] Webiew post url support #1970

charafau commented Aug 11, 2019

charafau commented Aug 15, 2019

amirh commented Aug 15, 2019

amirh left a comment

amirh Oct 10, 2019

charafau Nov 21, 2019

amirh Oct 10, 2019

charafau Nov 21, 2019

amirh Oct 10, 2019

charafau Nov 21, 2019

amirh Oct 10, 2019

charafau Nov 21, 2019

charafau commented Oct 14, 2019

charafau commented Nov 21, 2019

stuartmorgan commented Apr 6, 2021

charafau commented Apr 8, 2021

stuartmorgan commented Apr 27, 2021

charafau commented Apr 28, 2021

stuartmorgan commented May 4, 2021

stuartmorgan commented May 4, 2021

charafau commented Jun 2, 2021

stuartmorgan commented Jun 2, 2021

charafau commented Jun 2, 2021 •

edited

Loading

stuartmorgan commented Oct 12, 2021

charafau commented Oct 13, 2021

Hixie commented Oct 26, 2021

[webview_flutter] Webiew post url support #1970

[webview_flutter] Webiew post url support #1970

Conversation

charafau commented Aug 11, 2019

Description

Related Issues

Checklist

Breaking Change

charafau commented Aug 15, 2019

amirh commented Aug 15, 2019

amirh 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

charafau commented Oct 14, 2019

charafau commented Nov 21, 2019

stuartmorgan commented Apr 6, 2021

charafau commented Apr 8, 2021

stuartmorgan commented Apr 27, 2021

charafau commented Apr 28, 2021

stuartmorgan commented May 4, 2021

stuartmorgan commented May 4, 2021

charafau commented Jun 2, 2021

stuartmorgan commented Jun 2, 2021

charafau commented Jun 2, 2021 • edited Loading

stuartmorgan commented Oct 12, 2021

charafau commented Oct 13, 2021

Hixie commented Oct 26, 2021

charafau commented Jun 2, 2021 •

edited

Loading