Skip to content

[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

Merged
merged 14 commits into from
Jun 26, 2023

Conversation

GwonHyeok
Copy link
Contributor

@GwonHyeok GwonHyeok commented May 14, 2023

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

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the relevant style guides and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use dart format.)
  • I signed the CLA.
  • The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
  • I listed at least one issue that this PR fixes in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.
  • I updated CHANGELOG.md to add a description of the change, following repository CHANGELOG style.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

…wController`.

Starting from iOS version 16.4, it is now possible to change the value of the isInspectable property in WKWebView.
@stuartmorgan-g
Copy link
Contributor

Thanks for the contribution!

  • I listed at least one issue that this PR fixes in the description above.

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.

@cyanglaz cyanglaz self-assigned this May 15, 2023
@GwonHyeok
Copy link
Contributor Author

GwonHyeok commented May 16, 2023

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

@cyanglaz
Copy link
Contributor

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

@GwonHyeok
Copy link
Contributor Author

@cyanglaz

I have created an issue for this PR and assigned it.

@cyanglaz
Copy link
Contributor

I have created an issue for this PR and assigned it.

Awesome, thank you!

Please also do this:

This will also need native unit tests that validate that it's actually calling the relevant API.

@GwonHyeok
Copy link
Contributor Author

@cyanglaz

I added native unit tests for this PR. 👍🏻

Copy link
Contributor

@bparrishMines bparrishMines left a 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!

@GwonHyeok
Copy link
Contributor Author

@bparrishMines

I have reviewed all the feedback you provided and have committed the changes.

Copy link
Contributor

@bparrishMines bparrishMines left a 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

GwonHyeok and others added 3 commits May 24, 2023 09:38
…_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
@GwonHyeok
Copy link
Contributor Author

@stuartmorgan @cyanglaz

resolved the issue caused by a conflict with the release of version 3.4.4.

@GwonHyeok
Copy link
Contributor Author

@bparrishMines @cyanglaz @stuartmorgan

Hello,
Why hasn't the this PR been reviewed yet?

@stuartmorgan-g
Copy link
Contributor

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.

Copy link
Contributor

@cyanglaz cyanglaz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM % nits

Copy link
Contributor

@cyanglaz cyanglaz left a 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, *)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWFUnsupportedVersionError has been added.

Copy link
Contributor

@bparrishMines bparrishMines left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@bparrishMines bparrishMines added the autosubmit Merge PR when tree becomes green via auto submit App label Jun 26, 2023
@auto-submit auto-submit bot merged commit 96c8e2d into flutter:main Jun 26, 2023
@tall-nuts
Copy link

ARC Semantic Issue (Xcode): No visible @interface for 'FWFWebView' declares the selector 'setInspectable:'
/Users/xxx/.pub-cache/hosted/pub.dev/webview_flutter_wkwebview-3.6.0/ios/Classes/FWFWebViewHostApi.m:203:44

@stuartmorgan-g
Copy link
Contributor

@PengfeiGao In the future, please use the issue tracker to report issues. That's flutter/flutter#129587

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 27, 2023
stuartmorgan-g pushed a commit to flutter/flutter that referenced this pull request Jun 27, 2023
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
@sagarshende23

This comment was marked as off-topic.

@Aulig

This comment was marked as off-topic.

@sagarshende23

This comment was marked as off-topic.

@wubaibin

This comment was marked as off-topic.

@flutter flutter locked as off-topic and limited conversation to collaborators Jun 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
autosubmit Merge PR when tree becomes green via auto submit App p: webview_flutter platform-ios
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[webview_flutter_wkwebview] Add toggle for the Web Inspector feature
8 participants