-
Notifications
You must be signed in to change notification settings - Fork 169
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[desktop-webview-window] Control URL requests made by the webview (Breaking!) #296
Changes from 9 commits
70a4c5a
6edcd69
1e0cd33
a085b42
41c80c2
4e154cd
e7dd272
60e61cd
c0b8fcf
91e9201
d6b7783
1c2d6e7
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 |
---|---|---|
|
@@ -22,9 +22,9 @@ class WebviewImpl extends Webview { | |
|
||
OnHistoryChangedCallback? _onHistoryChanged; | ||
|
||
final ValueNotifier<bool> _isNaivgating = ValueNotifier<bool>(false); | ||
final ValueNotifier<bool> _isNavigating = ValueNotifier<bool>(false); | ||
|
||
final Set<OnUrlRequestCallback> _onUrlRequestCallbacks = {}; | ||
OnUrlRequestCallback? _onUrlRequestCallback = null; | ||
|
||
final Set<OnWebMessageReceivedCallback> _onWebMessageReceivedCallbacks = {}; | ||
|
||
|
@@ -57,12 +57,14 @@ class WebviewImpl extends Webview { | |
} | ||
|
||
void onNavigationStarted() { | ||
_isNaivgating.value = true; | ||
_isNavigating.value = true; | ||
} | ||
|
||
void notifyUrlChanged(String url) { | ||
for (final callback in _onUrlRequestCallbacks) { | ||
callback(url); | ||
bool notifyUrlChanged(String url) { | ||
if(_onUrlRequestCallback != null) { | ||
return _onUrlRequestCallback!(url); | ||
} else { | ||
return true; | ||
} | ||
} | ||
|
||
|
@@ -73,11 +75,11 @@ class WebviewImpl extends Webview { | |
} | ||
|
||
void onNavigationCompleted() { | ||
_isNaivgating.value = false; | ||
_isNavigating.value = false; | ||
} | ||
|
||
@override | ||
ValueListenable<bool> get isNavigating => _isNaivgating; | ||
ValueListenable<bool> get isNavigating => _isNavigating; | ||
|
||
@override | ||
void registerJavaScriptMessageHandler( | ||
|
@@ -222,13 +224,13 @@ class WebviewImpl extends Webview { | |
} | ||
|
||
@override | ||
void addOnUrlRequestCallback(OnUrlRequestCallback callback) { | ||
_onUrlRequestCallbacks.add(callback); | ||
void setOnUrlRequestCallback(OnUrlRequestCallback callback) { | ||
_onUrlRequestCallback = callback; | ||
} | ||
|
||
@override | ||
void removeOnUrlRequestCallback(OnUrlRequestCallback callback) { | ||
_onUrlRequestCallbacks.remove(callback); | ||
void removeOnUrlRequestCallback() { | ||
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. we can remove this function. |
||
_onUrlRequestCallback = null; | ||
} | ||
|
||
@override | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,6 +8,8 @@ | |
#include <utility> | ||
#include <thread> | ||
|
||
#include "flutter/method_result_functions.h" | ||
|
||
#include "web_view.h" | ||
#include "utils.h" | ||
#include "strconv.h" | ||
|
@@ -144,14 +146,34 @@ void WebView::OnWebviewControllerCreated() { | |
std::make_unique<flutter::EncodableValue>(flutter::EncodableMap{ | ||
{flutter::EncodableValue("id"), flutter::EncodableValue(web_view_id_)}, | ||
})); | ||
LPWSTR uri; | ||
args->get_Uri(&uri); | ||
method_channel_->InvokeMethod( | ||
"onUrlRequested", | ||
std::make_unique<flutter::EncodableValue>(flutter::EncodableMap{ | ||
{flutter::EncodableValue("id"), flutter::EncodableValue(web_view_id_)}, | ||
{flutter::EncodableValue("url"), flutter::EncodableValue(wide_to_utf8(std::wstring(uri)))}, | ||
})); | ||
|
||
if (triggerOnUrlRequestedEvent) { | ||
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. Is this necessary? It should make more sense to trigger onUrlRequest on any request (including from navigate(String) method). 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. In my opinion its more convenient like this, because otherwise the call of "webview.launch(url)" would trigger the OnUrlRequest event every time and you manually had to grant it every time. If you call "webview.launch(url)" you know already that the webview will navigate to the url now, because you requested it yourself. But I am more interested in navigations triggered by the webview itself. 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. But I can also make it, that also "webview.launch(url)" triggers the event. How would you like to have it? 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. sorry for later. But I still stand by my opinion. for example: 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. Ok seems also reasonable. Then I suggest to add an optional parameter with default value to webview.launch: 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 new commit containing the additional parameter for webview.launch Please review |
||
LPWSTR uri; | ||
args->get_Uri(&uri); | ||
|
||
auto result_handler = std::make_unique<flutter::MethodResultFunctions<>>( | ||
[uri, sender, this](const flutter::EncodableValue* success_value) { | ||
const bool letPass = std::get<bool>(*success_value); | ||
if (letPass) { | ||
this->setTriggerOnUrlRequestedEvent(false); | ||
sender->Navigate(uri); | ||
} | ||
}, | ||
nullptr, nullptr); | ||
|
||
method_channel_->InvokeMethod( | ||
"onUrlRequested", | ||
std::make_unique<flutter::EncodableValue>(flutter::EncodableMap{ | ||
{flutter::EncodableValue("id"), flutter::EncodableValue(web_view_id_)}, | ||
{flutter::EncodableValue("url"), flutter::EncodableValue(wide_to_utf8(std::wstring(uri)))}, | ||
}), std::move(result_handler)); | ||
|
||
// navigation is canceled here and retriggered later from the callback passed to the method channel | ||
args->put_Cancel(true); | ||
} else { | ||
args->put_Cancel(false); | ||
triggerOnUrlRequestedEvent = true; | ||
} | ||
return S_OK; | ||
} | ||
).Get(), nullptr); | ||
|
@@ -323,6 +345,10 @@ void WebView::PostWebMessageAsJson(const std::wstring& webmessage, | |
} | ||
} | ||
|
||
void WebView::setTriggerOnUrlRequestedEvent(const bool value) { | ||
this->triggerOnUrlRequestedEvent = value; | ||
} | ||
|
||
WebView::~WebView() { | ||
if (webview_) { | ||
webview_->Stop(); | ||
|
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.
the
callback
parameter should be nullableThere 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.
Done