[webview_flutter] Add interface for showing javascript dialog message #4704

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`.

…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.

… message (flutter/packages#4704)

flutter/packages@29d8cc0...11152d2 2024-02-09 [email protected] [webview_flutter] Add interface for showing javascript dialog message (flutter/packages#4704) 2024-02-09 [email protected] [pigeon] Implement Screaming Snake Case Conversion for Kotlin Enum Cases (flutter/packages#5918) 2024-02-09 [email protected] [camerax] Small fixes to starting/stopping video capture (flutter/packages#6068) 2024-02-08 [email protected] Roll Flutter from 8431cae to eb5d0a4 (33 revisions) (flutter/packages#6079) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

…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: flutter/packages#5670 * The PR that contains all changes can be found at flutter/packages#4704

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 added 4 commits February 5, 2024 19:23

Update changelog, pubspec and add test case

0f6fb5f

Update minimum flutter and dart version

aff2aa8

Remove warning

203c627

Update change log

2b35ddd

jsharp83 force-pushed the add-javascript-panel-interface branch from d18d501 to 2b35ddd Compare February 5, 2024 10:23

bparrishMines added 7 commits February 8, 2024 12:16

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

bparrishMines added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 9, 2024

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Feb 9, 2024

bparrishMines requested a review from stuartmorgan-g February 9, 2024 01:21

stuartmorgan-g approved these changes Feb 9, 2024

View reviewed changes

stuartmorgan-g added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 9, 2024

auto-submit bot merged commit 11152d2 into flutter:main Feb 9, 2024

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

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

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 9, 2024

11152d2 [webview_flutter] Add interface for showing javascript dialog…

6ffcc78

… message (flutter/packages#4704)

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`.

[webview_flutter] Add interface for showing javascript dialog message #4704

[webview_flutter] Add interface for showing javascript dialog message #4704

Uh oh!

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!

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

Development

Uh oh!

5 participants

jsharp83 commented Aug 15, 2023 •

edited

Loading

jsharp83 Sep 28, 2023 •

edited

Loading