-
Notifications
You must be signed in to change notification settings - Fork 9.8k
[webview_flutter] [url_launcher] Handle Multiwindows in WebViews #2991
Changes from 18 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 |
---|---|---|
|
@@ -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,46 @@ 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 (!flutterWebViewClient.shouldOverrideUrlLoading( | ||
FlutterWebView.this.webView, request)) { | ||
webView.loadUrl(url); | ||
} | ||
return true; | ||
} | ||
|
||
@Override | ||
public boolean shouldOverrideUrlLoading(WebView view, String url) { | ||
if (!flutterWebViewClient.shouldOverrideUrlLoading( | ||
FlutterWebView.this.webView, url)) { | ||
webView.loadUrl(url); | ||
} | ||
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; | ||
} | ||
} | ||
|
||
@TargetApi(Build.VERSION_CODES.JELLY_BEAN_MR1) | ||
@SuppressWarnings("unchecked") | ||
FlutterWebView( | ||
|
@@ -50,6 +95,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); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -828,6 +828,84 @@ void main() { | |
final String currentUrl = await controller.currentUrl(); | ||
expect(currentUrl, 'about:blank'); | ||
}); | ||
|
||
testWidgets( | ||
'can open new window and go back', | ||
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 one is good, but what about a test case like the one in the internal doc? 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'm not sure how to do a test like that. I would need to load a link that tries to open a window and test to see if javascript ran? |
||
(WidgetTester tester) async { | ||
final Completer<WebViewController> controllerCompleter = | ||
Completer<WebViewController>(); | ||
final Completer<void> pageLoaded = Completer<void>(); | ||
await tester.pumpWidget( | ||
Directionality( | ||
textDirection: TextDirection.ltr, | ||
child: WebView( | ||
key: GlobalKey(), | ||
onWebViewCreated: (WebViewController controller) { | ||
controllerCompleter.complete(controller); | ||
}, | ||
javascriptMode: JavascriptMode.unrestricted, | ||
onPageFinished: (String url) { | ||
pageLoaded.complete(); | ||
}, | ||
initialUrl: 'https://flutter.dev', | ||
), | ||
), | ||
); | ||
final WebViewController controller = await controllerCompleter.future; | ||
await controller | ||
.evaluateJavascript('window.open("https://www.google.com")'); | ||
await pageLoaded.future; | ||
expect(controller.currentUrl(), completion('https://www.google.com/')); | ||
|
||
await controller.goBack(); | ||
expect(controller.currentUrl(), completion('https://www.flutter.dev')); | ||
}, | ||
skip: !Platform.isAndroid, | ||
); | ||
|
||
testWidgets( | ||
'javascript does not run in parent window', | ||
(WidgetTester tester) async { | ||
final String openWindowTest = ''' | ||
<!DOCTYPE html><html> | ||
<head><title>Resize test</title> | ||
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. uber nit: resize test -> XSS test |
||
<script> | ||
setTimeout(function() { | ||
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 document needs to be inside the |
||
window.open('javascript:var elem = document.createElement("p");elem.innerHTML = "<b>Executed JS in parent origin: "+window.location.origin+"</b>"; document.body.append(elem);alert("XSS in doc.domain: "+document.domain+", win.origin: "+window.location.origin)'); | ||
}, 0); | ||
</script> | ||
</head> | ||
<body onload="onLoad();" bgColor="blue"> | ||
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.
function onLoad() {
window.open('javascript:var elem = document.createElement("p");elem.innerHTML = "<b>Executed JS in parent origin: "+window.location.origin+"</b>"; document.body.append(elem);alert("XSS in doc.domain: "+document.domain+", win.origin: "+window.location.origin)');
} |
||
</body> | ||
</html> | ||
'''; | ||
final String openWindowTestBase64 = | ||
base64Encode(const Utf8Encoder().convert(openWindowTest)); | ||
final Completer<WebViewController> controllerCompleter = | ||
Completer<WebViewController>(); | ||
|
||
await tester.pumpWidget( | ||
Directionality( | ||
textDirection: TextDirection.ltr, | ||
child: WebView( | ||
key: GlobalKey(), | ||
onWebViewCreated: (WebViewController controller) { | ||
controllerCompleter.complete(controller); | ||
}, | ||
javascriptMode: JavascriptMode.unrestricted, | ||
initialUrl: | ||
'<iframe src="data:text/html;charset=utf-8;base64,$openWindowTestBase64"</iframe>', | ||
), | ||
), | ||
); | ||
|
||
final WebViewController controller = await controllerCompleter.future; | ||
final String result = await controller.evaluateJavascript( | ||
'document.querySelector("p") && document.querySelector("p").textContent'); | ||
expect(result, isEmpty); | ||
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 might be racy. How do we know that the code inside the iframe has run at this point? One solution is to add a |
||
}, | ||
skip: !Platform.isAndroid, | ||
); | ||
} | ||
|
||
// JavaScript booleans evaluate to different string values on Android and iOS. | ||
|
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