[webview_flutter] Add interface for showing javascript dialog message by jsharp83 · Pull Request #4704 · flutter/packages

jsharp83 · 2023-08-15T07:54:59Z

there are cases where Web calls System Popup with javascript on webview_flutter
At this time, the message comes in the WKUIDelegate part in iOS.
- https://developer.apple.com/documentation/webkit/wkuidelegate/1537406-webview
- https://developer.apple.com/documentation/webkit/wkuidelegate/1536489-webview
Android also has a interface on WebChromeClient
- https://developer.android.com/reference/android/webkit/WebChromeClient#onJsAlert(android.webkit.WebView,%20java.lang.String,%20java.lang.String,%20android.webkit.JsResult)
It was implemented according to the requirements of the code review of the [webview_flutter_wkwebview] Add webkit interface for showing javascript alert message #4555
Related issue: alert() method doesn't work in WebView flutter#30358 (comment)
Related Interface PR: [webview_flutter] Implement platform interface for JavaScript dialog #5670

Pre-launch Checklist

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

jsharp83 · 2023-08-23T04:52:38Z

@bparrishMines Hi, Can you check this PR? This is my first contribution here so I don't know exact routine for contribution. Help me please. Thanks :)

bparrishMines

Thanks for implementing this for Android and iOS! I left my first comments.

Also, we're working on updating pigeon to handle more of the wrapping code if you would prefer to wait for that before continuing.

bparrishMines · 2023-09-15T16:04:23Z

packages/webview_flutter/webview_flutter_platform_interface/CHANGELOG.md

nit:

Suggested change

* Add support to show JavaScript dialog. See `PlatformWebViewController.setJavaScriptAlertDialogCallback`.

* Adds support to show JavaScript dialog. See `PlatformWebViewController.setJavaScriptAlertDialogCallback`.

This can also list the other two callback methods.

bparrishMines · 2023-09-15T16:13:45Z

.../webview_flutter/webview_flutter_platform_interface/lib/src/platform_webview_controller.dart

Since this is only setting one callback method, the pattern should be

Suggested change

Future<void> setJavaScriptAlertDialogCallback(

Future<void> Function(String message)

javaScriptAlertDialogCallback) async {

Future<void> setOnJavaScriptAlertDialog(

Future<void> Function(String message)

onJavaScriptAlertDialog) async {

Same for the two methods below.

bparrishMines · 2023-09-15T16:14:55Z

packages/webview_flutter/webview_flutter_platform_interface/pubspec.yaml

This will be a minor version bump since it is adding a new feature. (e.g. 2.6.0)

bparrishMines · 2023-09-15T16:18:15Z

...roid/android/src/main/java/io/flutter/plugins/webviewflutter/WebChromeClientHostApiImpl.java

Since the platform interface separates handling the callbacks into three separate methods, you should also have three separate flags for each alert/confirm/prompt. What happens if a user only wants to set the confirm dialog?

bparrishMines · 2023-09-15T16:20:16Z

packages/webview_flutter/webview_flutter_wkwebview/pigeons/web_kit.dart

Since the native WKUIDelegate separates handling dialogs into separate methods, this should also.

jsharp83 · 2023-09-20T13:49:51Z

@bparrishMines It has been modified according to the review. Could you please check the code again?

bparrishMines · 2023-09-25T17:05:03Z

.../webview_flutter/webview_flutter_platform_interface/lib/src/platform_webview_controller.dart

+
+  /// Sets a callback that notifies the host application that the web page
+  /// wants to display a JavaScript alert() dialog.
+  Future<void> setOnJavaScriptAlertDialogCallback(


This still uses Callback at the end. It should be setOnJavaScriptAlertDialog. Same for the confirm and textInputDialog methods below.

bparrishMines · 2023-09-25T17:10:06Z

packages/webview_flutter/webview_flutter_android/lib/src/android_webview.dart

    );
  }

+  Future<void> setHasJavaScriptAlertCallback(


This and the two methods below should follow the same pattern as onShowFileChooser and be called setSynchronousReturnValueForOnJsAlert.

bparrishMines · 2023-09-25T17:11:26Z

packages/webview_flutter/webview_flutter_android/lib/src/android_webview.dart

  /// mode.
  final HideCustomViewCallback? onHideCustomView;

+  final Future<void> Function(String message)? onHandleJavaScriptAlert;


These should have the same name as the corresponding Java method. (e.g. onJsAlert). They should also have a return value of void since they can't return a synchronous value.

@bparrishMines
Sorry, I cannot fully understand on your comment.

They should also have a return value of void since they can't return a synchronous value.

They should have a Future value to pass the value for JSResult and it works well in my test.

FileChooser in android_webview also return a List previously, So I just follow it to implement.

Can you explain more to fix this?
I really appreciate your help :)

Ah, now I understand. Your implementation is correct. I didn't notice the JSResult in the params.

bparrishMines · 2023-09-25T17:16:18Z

packages/webview_flutter/webview_flutter_android/lib/src/android_webview.dart

+  final bool Function()? hasJavaScriptAlertCallback;
+  final bool Function()? hasJavaScriptConfirmCallback;
+  final bool Function()? hasJavaScriptPromptCallback;


These can be removed. I don't believe they are used and they are redundant with the callback methods above.

bparrishMines · 2023-09-25T17:20:05Z

packages/webview_flutter/webview_flutter_android/lib/src/android_webview_controller.dart

    ),
+    onHandleJavaScriptAlert: withWeakReferenceTo(this, (WeakReference<AndroidWebViewController> weakReference) {
+      return (String message) async {
+        final Future<void> Function(String)? callback = _onJavaScriptAlertCallback;


This should use weakReference?.target._onJavaScriptAlertCallback, otherwise this will cause a leak. Same for the other dialog fields below.

bparrishMines · 2023-09-25T17:22:00Z

packages/webview_flutter/webview_flutter_wkwebview/pigeons/web_kit.dart

+    'runJavaScriptAlertPanelForDelegateWithIdentifier:message:',
+  )
+  @async
+  void runJavaScriptAlertPanelWithIdentifier(


This should be runJavaScriptAlertPanel. The withIdentifier should only be added to the @ObjCSelector annotation above and not in the method.

bparrishMines · 2023-09-25T17:22:09Z

packages/webview_flutter/webview_flutter_wkwebview/pigeons/web_kit.dart

+    'runJavaScriptConfirmPanelForDelegateWithIdentifier:message:',
+  )
+  @async
+  bool runJavaScriptConfirmPanelWithIdentifier(


Same for this method

bparrishMines · 2023-09-25T17:23:14Z

packages/webview_flutter/webview_flutter_wkwebview/lib/src/webkit_webview_controller.dart

      },
+      runJavaScriptAlertDialog: (String message) async {
+        final Future<void> Function(String message)? callback =
+            _onJavaScriptAlertDialogCallback;


This needs to be weakThis.target?._onJavaScriptAlertDialogCallback. The methods below also need to be updated.

bparrishMines · 2023-10-03T15:43:11Z

.../webview_flutter/webview_flutter_platform_interface/lib/src/platform_webview_controller.dart

It looks like WebChromeClient.onJsAlert and WKUIDelegate.runJavaScriptAlertPanelWithMessage also provide the URL for these callbacks. We should change String message to a class that contains a message field where we can add additional information at a later point. You could create a class named something like JavaScriptAlertDialogRequest. See JavaScriptConsoleMessage as an example.

This goes for the methods below as well. (e.g. JavaScriptConfirmDialogRequest and JavaScriptTextInputDialogRequest).

I fixed it. Can you check it again?

bparrishMines · 2023-10-03T15:45:00Z

packages/webview_flutter/webview_flutter_platform_interface/CHANGELOG.md

nit:

Suggested change

* Adds support to show JavaScript dialog. Sees `PlatformWebViewController.setOnJavaScriptAlertDialog`, `PlatformWebViewController.setOnJavaScriptConfirmDialog` and `PlatformWebViewController.setOnJavaScriptTextInputDialog`.

* Adds support to show JavaScript dialog. See

`PlatformWebViewController.setOnJavaScriptAlertDialog`,

`PlatformWebViewController.setOnJavaScriptConfirmDialog`, and

`PlatformWebViewController.setOnJavaScriptTextInputDialog`.

…iew (#5795) * There are cases where Web calls System Popup with javascript on webview_flutter * At this time, the message comes in the WKUIDelegate part in iOS. * https://developer.apple.com/documentation/webkit/wkuidelegate/1537406-webview * https://developer.apple.com/documentation/webkit/wkuidelegate/1536489-webview * Related issue: flutter/flutter#30358 (comment) * Related Interface PR: #5670 * The PR that contains all changes can be found at #4704

…5796) * There are cases where Web calls System Popup with javascript on webview_flutter * Android has a interface on WebChromeClient * https://developer.android.com/reference/android/webkit/WebChromeClient#onJsAlert(android.webkit.WebView,%20java.lang.String,%20java.lang.String,%20android.webkit.JsResult) * Related issue: flutter/flutter#30358 (comment) * Related Interface PR: #5670 * The PR that contains all changes can be found at #4704

jsharp83 · 2024-01-26T03:18:03Z

@bparrishMines @stuartmorgan @hellohuanlin

HI, This PR only contains webview_flutter package part now.
I needed to update minimum flutter version on webview_flutter package due to webview_flutter_wkwebview
Please check this again. Thank you.

stuartmorgan-g

LGTM with CHANGELOG nit. @bparrishMines for secondary approval.

stuartmorgan-g · 2024-01-30T15:10:22Z

packages/webview_flutter/webview_flutter/CHANGELOG.md

Nit: "Adds support for custom handling of JavaScript dialogs. See [...]"

(Also, please wrap to ~80 characters.)

This two lines can be removed you have the content now in 4.6.0

rekire · 2024-02-03T10:41:59Z

@jsharp83 I think you missed that line break change request. I cannot await that this goes finally live.

rekire · 2024-02-03T11:40:01Z

packages/webview_flutter/webview_flutter/CHANGELOG.md

I think this line is still too long

…t-panel-interface

bparrishMines

LGTM

auto-submit · 2024-02-09T01:17:13Z

auto label is removed for flutter/packages/4704, due to This PR has not met approval requirements for merging. The PR author is not a member of flutter-hackers and needs 1 more review(s) in order to merge this PR.

Merge guidelines: A PR needs at least one approved review if the author is already part of flutter-hackers or two member reviews if the author is not a flutter-hacker before re-applying the autosubmit label. Reviewers: If you left a comment approving, please use the "approve" review action instead.

bparrishMines · 2024-02-09T01:21:18Z

@stuartmorgan It looks like this needs the approval along with your LGTM

stuartmorgan-g

LGTM; looks like I just pressed the wrong button last time.

jsharp83 requested review from bparrishMines and cyanglaz as code owners August 15, 2023 07:54

github-actions bot added platform-android platform-ios p: webview_flutter labels Aug 15, 2023

jsharp83 force-pushed the add-javascript-panel-interface branch 2 times, most recently from 4788137 to 7dfb120 Compare August 23, 2023 04:48

jsharp83 force-pushed the add-javascript-panel-interface branch from 7dfb120 to 850e9f5 Compare August 28, 2023 00:16

bparrishMines reviewed Sep 15, 2023

View reviewed changes

jsharp83 force-pushed the add-javascript-panel-interface branch 4 times, most recently from 232e186 to 41bf657 Compare September 20, 2023 13:45

bparrishMines reviewed Sep 25, 2023

View reviewed changes

jsharp83 force-pushed the add-javascript-panel-interface branch 11 times, most recently from 54a7f6b to 634c05f Compare September 28, 2023 06:23

gtbluesky mentioned this pull request Sep 28, 2023

[webview_flutter]Adds support to intercept JavaScript alert/confirm/prompt dialog. #5026

Closed

11 tasks

bparrishMines reviewed Oct 3, 2023

View reviewed changes

jsharp83 force-pushed the add-javascript-panel-interface branch from 2e83c14 to 3930794 Compare October 5, 2023 09:18

jsharp83 force-pushed the add-javascript-panel-interface branch from 185378d to 3913e59 Compare January 4, 2024 04:12

This was referenced Jan 4, 2024

[webview_flutter_wkwebview] Add javascript panel interface for wkwebview #5795

Merged

[webview_flutter_android] Add javascript panel interface for android #5796

Merged

[webview_flutter] Add javascript panel interface for flutter parts #5797

Closed

bparrishMines added the federated: all_changes PR that contains changes for all packages for a federated plugin change label Jan 16, 2024

jsharp83 force-pushed the add-javascript-panel-interface branch from 3913e59 to 41253b3 Compare January 25, 2024 01:49

stuartmorgan-g reviewed Jan 30, 2024

View reviewed changes

rekire reviewed Feb 3, 2024

View reviewed changes

packages/webview_flutter/webview_flutter/CHANGELOG.md Outdated

Copy link

rekire Feb 3, 2024

Choose a reason for hiding this comment

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

I think this line is still too long

rekire and others added 12 commits February 5, 2024 19:23

Add missing exports from the platform interface

ade23fe

Update changelog, pubspec and add test case

0f6fb5f

Update minimum flutter and dart version

aff2aa8

Remove warning

203c627

Update change log

2b35ddd

undo dart and flutter bumps

28f3eda

Merge branch 'main' of github.com:flutter/packages into add-javascrip…

a9bb4fb

…t-panel-interface

format changelog

f06bfc2

fix test

39ff8de

my bad redo version change

2bbea8c

add to changelog about version bump

d50d7ae

also bump example pubspec version

2c3dc83

bparrishMines approved these changes Feb 9, 2024

View reviewed changes

stuartmorgan-g approved these changes Feb 9, 2024

View reviewed changes

engine-flutter-autoroll mentioned this pull request Feb 9, 2024

Roll Packages from 29d8cc006b0e to 11152d2bc8f3 (4 revisions) flutter/flutter#143217

Merged

reidbaker mentioned this pull request Feb 19, 2024

[in_app_purchase_android] Add UserChoiceBilling mode. #6162

Merged

16 tasks

	* Add support to show JavaScript dialog. See `PlatformWebViewController.setJavaScriptAlertDialogCallback`.
	* Adds support to show JavaScript dialog. See `PlatformWebViewController.setJavaScriptAlertDialogCallback`.

-  Future<void> setJavaScriptAlertDialogCallback(
-      Future<void> Function(String message)
-          javaScriptAlertDialogCallback) async {
+  Future<void> setOnJavaScriptAlertDialog(
+      Future<void> Function(String message)
+          onJavaScriptAlertDialog) async {

-* Adds support to show JavaScript dialog. Sees `PlatformWebViewController.setOnJavaScriptAlertDialog`, `PlatformWebViewController.setOnJavaScriptConfirmDialog` and `PlatformWebViewController.setOnJavaScriptTextInputDialog`.
+* Adds support to show JavaScript dialog. See
+  `PlatformWebViewController.setOnJavaScriptAlertDialog`,
+  `PlatformWebViewController.setOnJavaScriptConfirmDialog`, and
+  `PlatformWebViewController.setOnJavaScriptTextInputDialog`.

Conversation

jsharp83 commented Aug 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

jsharp83 commented Aug 23, 2023

Uh oh!

bparrishMines left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jsharp83 commented Sep 20, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jsharp83 Sep 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jsharp83 commented Jan 26, 2024

Uh oh!

stuartmorgan-g left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rekire commented Feb 3, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bparrishMines left a comment

Choose a reason for hiding this comment

Uh oh!

auto-submit bot commented Feb 9, 2024

Uh oh!

bparrishMines commented Feb 9, 2024

Uh oh!

stuartmorgan-g left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

jsharp83 commented Aug 15, 2023 •

edited

Loading

jsharp83 Sep 28, 2023 •

edited

Loading