-
Notifications
You must be signed in to change notification settings - Fork 9.8k
[webview_flutter] [url_launcher] Handle Multiwindows in WebViews #2991
Changes from 6 commits
68f8848
7337002
3fa09aa
39b030b
bb8154b
625f673
9f8014f
ef55f08
9bd470a
3c6de98
833bb0a
dde8c7f
c12449a
c5bf9d1
7657a2c
380c7dd
9bfc1d2
dd4deec
022e717
9d6a594
2a6d766
94c7b5a
fab93d7
f6bf112
6d53eda
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,17 +1,21 @@ | ||
package io.flutter.plugins.urllauncher; | ||
|
||
import android.annotation.TargetApi; | ||
import android.app.Activity; | ||
import android.content.BroadcastReceiver; | ||
import android.content.Context; | ||
import android.content.Intent; | ||
import android.content.IntentFilter; | ||
import android.os.Build; | ||
import android.os.Bundle; | ||
import android.os.Message; | ||
import android.provider.Browser; | ||
import android.view.KeyEvent; | ||
import android.webkit.WebChromeClient; | ||
import android.webkit.WebResourceRequest; | ||
import android.webkit.WebView; | ||
import android.webkit.WebViewClient; | ||
import androidx.annotation.NonNull; | ||
import java.util.HashMap; | ||
import java.util.Map; | ||
|
||
|
@@ -60,6 +64,48 @@ public boolean shouldOverrideUrlLoading(WebView view, WebResourceRequest request | |
|
||
private IntentFilter closeIntentFilter = new IntentFilter(ACTION_CLOSE); | ||
|
||
// Verifies that a url opened by `Window.open` has a secure url. | ||
private class FlutterWebChromeClient extends WebChromeClient { | ||
@Override | ||
public boolean onCreateWindow( | ||
final WebView view, boolean isDialog, boolean isUserGesture, Message resultMsg) { | ||
final WebViewClient webViewClient = | ||
new WebViewClient() { | ||
@TargetApi(Build.VERSION_CODES.LOLLIPOP) | ||
@Override | ||
public boolean shouldOverrideUrlLoading( | ||
@NonNull WebView view, @NonNull WebResourceRequest request) { | ||
final String url = request.getUrl().toString(); | ||
if (isSecure(url)) { | ||
webview.loadUrl(url); | ||
} | ||
return true; | ||
} | ||
|
||
@Override | ||
public boolean shouldOverrideUrlLoading(WebView view, String url) { | ||
if (isSecure(url)) { | ||
webview.loadUrl(url); | ||
} | ||
return true; | ||
} | ||
}; | ||
|
||
final WebView newWebView = new WebView(webview.getContext()); | ||
newWebView.setWebViewClient(webViewClient); | ||
|
||
final WebView.WebViewTransport transport = (WebView.WebViewTransport) resultMsg.obj; | ||
transport.setWebView(newWebView); | ||
resultMsg.sendToTarget(); | ||
|
||
return true; | ||
} | ||
|
||
private boolean isSecure(String url) { | ||
return url.startsWith("https://") || url.startsWith("http://"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would suggest we rename this as: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would only do this after we get an ok from the internal team. This method is currently just following their suggestion. |
||
} | ||
} | ||
|
||
@Override | ||
public void onCreate(Bundle savedInstanceState) { | ||
super.onCreate(savedInstanceState); | ||
|
@@ -81,6 +127,10 @@ public void onCreate(Bundle savedInstanceState) { | |
// Open new urls inside the webview itself. | ||
webview.setWebViewClient(webViewClient); | ||
|
||
// Multi windows is set with FlutterWebChromeClient by default to handle internal bug: b/159892679. | ||
webview.getSettings().setSupportMultipleWindows(true); | ||
webview.setWebChromeClient(new FlutterWebChromeClient()); | ||
|
||
// Register receiver that may finish this Activity. | ||
registerReceiver(broadcastReceiver, closeIntentFilter); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,9 +9,14 @@ | |
import android.hardware.display.DisplayManager; | ||
import android.os.Build; | ||
import android.os.Handler; | ||
import android.os.Message; | ||
import android.view.View; | ||
import android.webkit.WebChromeClient; | ||
import android.webkit.WebResourceRequest; | ||
import android.webkit.WebStorage; | ||
import android.webkit.WebView; | ||
import android.webkit.WebViewClient; | ||
import androidx.annotation.NonNull; | ||
import io.flutter.plugin.common.BinaryMessenger; | ||
import io.flutter.plugin.common.MethodCall; | ||
import io.flutter.plugin.common.MethodChannel; | ||
|
@@ -29,6 +34,48 @@ public class FlutterWebView implements PlatformView, MethodCallHandler { | |
private final FlutterWebViewClient flutterWebViewClient; | ||
private final Handler platformThreadHandler; | ||
|
||
// Verifies that a url opened by `Window.open` has a secure url. | ||
private class FlutterWebChromeClient extends WebChromeClient { | ||
blasten marked this conversation as resolved.
Show resolved
Hide resolved
|
||
@Override | ||
public boolean onCreateWindow( | ||
final WebView view, boolean isDialog, boolean isUserGesture, Message resultMsg) { | ||
final WebViewClient webViewClient = | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure I understand this part, it seems like we're creating a new WebViewClient type and delegate some of the calls to the existing flutterWebViewClient, do we not want all functionality implemented by FlutterWebViewClient to work for the new WebView as well? (e.g should we not just use FlutterWebViewClient as the client for the new webview?) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I assumed that we only wanted the new |
||
new WebViewClient() { | ||
@TargetApi(Build.VERSION_CODES.LOLLIPOP) | ||
@Override | ||
public boolean shouldOverrideUrlLoading( | ||
@NonNull WebView view, @NonNull WebResourceRequest request) { | ||
final String url = request.getUrl().toString(); | ||
if (isSecure(url)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure I follow the logic here, can you comment on why we only delegate calls for http and https URLs? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this can be addressed by resolving https://github.com/flutter/plugins/pull/2991/files#r483177693 |
||
flutterWebViewClient.shouldOverrideUrlLoading(FlutterWebView.this.webView, request); | ||
} | ||
return true; | ||
} | ||
|
||
@Override | ||
public boolean shouldOverrideUrlLoading(WebView view, String url) { | ||
if (isSecure(url)) { | ||
flutterWebViewClient.shouldOverrideUrlLoading(FlutterWebView.this.webView, url); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does invoking There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added a test, so it does still work without a navigation delegate. |
||
} | ||
return true; | ||
} | ||
}; | ||
|
||
final WebView newWebView = new WebView(view.getContext()); | ||
amirh marked this conversation as resolved.
Show resolved
Hide resolved
|
||
newWebView.setWebViewClient(webViewClient); | ||
|
||
final WebView.WebViewTransport transport = (WebView.WebViewTransport) resultMsg.obj; | ||
transport.setWebView(newWebView); | ||
resultMsg.sendToTarget(); | ||
|
||
return true; | ||
} | ||
|
||
private boolean isSecure(String url) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was a little confused by the name |
||
return url.startsWith("https://") || url.startsWith("http://"); | ||
} | ||
} | ||
|
||
@TargetApi(Build.VERSION_CODES.JELLY_BEAN_MR1) | ||
@SuppressWarnings("unchecked") | ||
FlutterWebView( | ||
|
@@ -50,6 +97,10 @@ public class FlutterWebView implements PlatformView, MethodCallHandler { | |
webView.getSettings().setDomStorageEnabled(true); | ||
webView.getSettings().setJavaScriptCanOpenWindowsAutomatically(true); | ||
|
||
// Multi windows is set with FlutterWebChromeClient by default to handle internal bug: b/159892679. | ||
webView.getSettings().setSupportMultipleWindows(true); | ||
webView.setWebChromeClient(new FlutterWebChromeClient()); | ||
|
||
methodChannel = new MethodChannel(messenger, "plugins.flutter.io/webview_" + id); | ||
methodChannel.setMethodCallHandler(this); | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what API needs this particular version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method should only be called in Android API 24+: https://developer.android.com/reference/android/webkit/WebViewClient#shouldOverrideKeyEvent(android.webkit.WebView,%20android.view.KeyEvent)
However,
WebResourceRequest.getUrl()
is only available in Android API 21: https://developer.android.com/reference/android/webkit/WebViewClient#shouldOverrideKeyEvent(android.webkit.WebView,%20android.view.KeyEvent)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
understood