-
Notifications
You must be signed in to change notification settings - Fork 3.4k
[webview_flutter_wkwebview] Adds the isInspectable
to WebKitWebViewController
.
#3984
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
Conversation
…wController`. Starting from iOS version 16.4, it is now possible to change the value of the isInspectable property in WKWebView.
Thanks for the contribution!
Could you file an issue providing more context on this, especially since the Apple docs don't explain it? That would be helpful for reviewers, and for future understanding of this PR in context. This will also need native unit tests that validate that it's actually calling the relevant API. |
Since iOS 16.4, the Web Inspector feature is disabled by default. Here is a blog post from WebKit that discusses this Apple Developer Forums thread (iOS 16.4 webview can not debug in safari web inspector) |
@GwonHyeok Thanks for the explanation! This makes sense. Please follow the instructions in #3984 (comment) before we can review and land this. I think your comment is clear enough so you can copy it over to the issue that you will create. |
I have created an issue for this PR and assigned it. |
Awesome, thank you! Please also do this:
|
I added native unit tests for this PR. 👍🏻 |
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.
Thanks for the contribution!
packages/webview_flutter/webview_flutter_wkwebview/CHANGELOG.md
Outdated
Show resolved
Hide resolved
packages/webview_flutter/webview_flutter_wkwebview/CHANGELOG.md
Outdated
Show resolved
Hide resolved
packages/webview_flutter/webview_flutter_wkwebview/pubspec.yaml
Outdated
Show resolved
Hide resolved
packages/webview_flutter/webview_flutter_wkwebview/lib/src/web_kit/web_kit.dart
Outdated
Show resolved
Hide resolved
packages/webview_flutter/webview_flutter_wkwebview/lib/src/webkit_webview_controller.dart
Outdated
Show resolved
Hide resolved
I have reviewed all the feedback you provided and have committed the changes. |
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.
LGTM
cc @stuartmorgan / @cyanglaz for secondary review
packages/webview_flutter/webview_flutter_wkwebview/lib/src/web_kit/web_kit.dart
Outdated
Show resolved
Hide resolved
packages/webview_flutter/webview_flutter_wkwebview/lib/src/webkit_webview_controller.dart
Outdated
Show resolved
Hide resolved
…_kit/web_kit.dart Co-authored-by: Maurice Parrish <10687576+bparrishMines@users.noreply.github.com>
…kit_webview_controller.dart Co-authored-by: Maurice Parrish <10687576+bparrishMines@users.noreply.github.com>
# Conflicts: # packages/webview_flutter/webview_flutter_wkwebview/CHANGELOG.md # packages/webview_flutter/webview_flutter_wkwebview/pubspec.yaml
resolved the issue caused by a conflict with the release of version 3.4.4. |
@bparrishMines @cyanglaz @stuartmorgan Hello, |
Because I was out of office for a significant portion of the time since I was added to the review list, and this review is only one of many things I have on my plate. |
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.
LGTM % nits
packages/webview_flutter/webview_flutter_wkwebview/lib/src/web_kit/web_kit.dart
Outdated
Show resolved
Hide resolved
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.
LGTM
- (void)setInspectableForWebViewWithIdentifier:(NSNumber *)identifier | ||
inspectable:(NSNumber *)inspectable | ||
error:(FlutterError *_Nullable *_Nonnull)error { | ||
if (@available(macOS 13.3, iOS 16.4, tvOS 16.4, *)) { |
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 should also provide a FlutterError
for unsupported versions. See https://github.com/flutter/packages/blob/main/packages/webview_flutter/webview_flutter_wkwebview/ios/Classes/FWFUserContentControllerHostApi.m#L55
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.
FWFUnsupportedVersionError
has been added.
…portedVersionError
…bview_inspectable
# Conflicts: # packages/webview_flutter/webview_flutter_wkwebview/CHANGELOG.md
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.
LGTM
ARC Semantic Issue (Xcode): No visible @interface for 'FWFWebView' declares the selector 'setInspectable:' |
@PengfeiGao In the future, please use the issue tracker to report issues. That's flutter/flutter#129587 |
…itWebViewController`. (flutter/packages#3984)
flutter/packages@6b70804...f89ce02 2023-06-27 hansmuller@google.com Updated rfw/test/material_widgets_test.dart for M3 (flutter/packages#4316) 2023-06-27 tarrinneal@gmail.com [shared_preferences] Adds new `clearWithParameters` and `getAllWithParameters` methods to platform interface. (flutter/packages#4261) 2023-06-26 engine-flutter-autoroll@skia.org Roll Flutter from 042c036 to 96a2c05 (60 revisions) (flutter/packages#4313) 2023-06-26 10687576+bparrishMines@users.noreply.github.com [file_selector_android] Create initial Android implementation of the file_selector package (flutter/packages#3814) 2023-06-26 49699333+dependabot[bot]@users.noreply.github.com [in_app_pur]: Bump org.json:json from 20230227 to 20230618 in /packages/in_app_purchase/in_app_purchase/example/android/app (flutter/packages#4244) 2023-06-26 me@ghyeok.io [webview_flutter_wkwebview] Adds the `isInspectable` to `WebKitWebViewController`. (flutter/packages#3984) 2023-06-26 stuartmorgan@google.com [ci] Remove jcenter from legacy project (flutter/packages#4306) 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 flutter-ecosystem@google.com,rmistry@google.com 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://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Fixes: flutter/flutter#126899
Starting from iOS version 16.4, it is now possible to change the value of the isInspectable property in WKWebView.
https://developer.apple.com/documentation/webkit/wkwebview/4111163-isinspectable
Since iOS 16.4, the Web Inspector feature is disabled by default.
Therefore, to use Web Inspector, need to change the value of isInspectable.
Here is a blog post from WebKit that discusses this
https://webkit.org/blog/13936/enabling-the-inspection-of-web-content-in-apps/
Apple Developer Forums thread (iOS 16.4 webview can not debug in safari web inspector)
https://developer.apple.com/forums/thread/727049
Pre-launch Checklist
dart format
.)[shared_preferences]
pubspec.yaml
with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.md
to add a description of the change, following repository CHANGELOG style.///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.