Skip to content
This repository has been archived by the owner on Feb 22, 2023. It is now read-only.

[webview_flutter] Update webview packages for Android and iOS to implement runJavascript and runJavascriptReturningResult. #4402

Merged
merged 47 commits into from Nov 1, 2021

Conversation

BeMacized
Copy link
Member

This PR implements the newly added runJavaScript and runJavaScriptForResult methods added to the platform interface in #4401.

This is meant as a solution to the issue mentioned in flutter/flutter#66318, and was discussed offline with @mvanbeusekom and @stuartmorgan.

As this PR also depends on and includes the changes from #4401, this PR will remain in draft state until that PR is merged and published.

Relevant issue:

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]. (Note that unlike the flutter/flutter repo, the flutter/plugins repo does use dart format.)
  • I signed the [CLA].
  • The title of the PR starts with the name of the plugin 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].
  • I updated CHANGELOG.md to add a description of the change.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test exempt.
  • All existing and new tests are passing.

@google-cla google-cla bot added the cla: yes label Oct 1, 2021
@github-actions github-actions bot added p: webview_flutter Edits files for a webview_flutter plugin platform-android platform-ios labels Oct 1, 2021
@BeMacized BeMacized changed the title [webview_flutter] Update webview packages for Android and iOS to implement runJavaScript and runJavaScriptForResult. [webview_flutter] Update webview packages for Android and iOS to implement runJavascript and runJavascriptReturningResult. Oct 4, 2021
@BeMacized
Copy link
Member Author

I think I've fixed every case now where "JavaScript" could be used, while still using "Javascript" where it was needed to either stay consistent or prevent a breaking change.

If you still find any cases I missed, let me know!

@BeMacized BeMacized marked this pull request as ready for review October 8, 2021 12:31
@BeMacized
Copy link
Member Author

@stuartmorgan Would you be able to take another look at this please?

Copy link
Contributor

@stuartmorgan stuartmorgan left a comment

Choose a reason for hiding this comment

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

Small things, otherwise LG

@BeMacized
Copy link
Member Author

@stuartmorgan All PR feedback has been implemented.

I however came across one issue with either the formatter or the format check: they seem to disagree. When formatting the webview plugin with the included script (dart run ./script/tool/lib/src/main.dart format --plugins webview_flutter), it formats FlutterWebView.m in such a way that it does not pass the check. @mvanbeusekom has been able to reproduce the issue as well.

I've fixed it now based on the output from cirrus so that this PR passes the check, but it does not fix the problem with the formatter.

@stuartmorgan
Copy link
Contributor

I however came across one issue with either the formatter or the format check: they seem to disagree. When formatting the webview plugin with the included script (dart run ./script/tool/lib/src/main.dart format --plugins webview_flutter), it formats FlutterWebView.m in such a way that it does not pass the check. @mvanbeusekom has been able to reproduce the issue as well.

flutter/flutter#39767

The script and the format check are exactly the same thing; what disagrees is the version of clang-format on the bots and the version you have locally.

Copy link
Contributor

@stuartmorgan stuartmorgan left a comment

Choose a reason for hiding this comment

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

LGTM with one nit. Thanks!

@@ -110,11 +110,11 @@ void main() {
await pageLoads.stream.firstWhere((String url) => url == currentUrl);

final String content = await controller
.evaluateJavascript('document.documentElement.innerText');
Copy link
Contributor

Choose a reason for hiding this comment

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

I just realized that in both implementation packages you've replaced all instances of evaluateJavascript with one of the new methods, which means we no longer have any test coverage that the legacy method still works.

Please add a new test to both the Android and iOS packages that does a simple test of evaluateJavascript (including its return result) just to make sure we have sanity coverage that it's not broken. (It doesn't need to cover edge cases, it's mostly to make sure that, e.g., we'd notice if the case statement for mapping "evaluateJavascript" to the right helper method were dropped.)

Copy link
Contributor

@stuartmorgan stuartmorgan left a comment

Choose a reason for hiding this comment

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

New tests LGTM, thanks

@BeMacized BeMacized merged commit 0c23f8f into flutter:master Nov 1, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 3, 2021
… to implement `runJavascript` and `runJavascriptReturningResult`. (flutter/plugins#4402)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 3, 2021
… to implement `runJavascript` and `runJavascriptReturningResult`. (flutter/plugins#4402)
NickalasB added a commit to NickalasB/plugins that referenced this pull request Nov 4, 2021
* master:
  [webview_flutter] Deprecate evaluateJavascript in favour of runJavaScript and runJavaScriptForResult. (flutter#4403)
  [webview_flutter] Add interface methods to load local files and HTML strings. (flutter#4446)
  [ci] add pedantic dep to new in_app_purchase pkgs (flutter#4471)
  [ci] Remove unused dep from file_selector. (flutter#4468)
  [ci] update build_runner dep in android_intent and file_selector example (flutter#4467)
  [webview_flutter] Add platform interface method `loadRequest`. (flutter#4450)
  Remove -Werror from deprecated plugin Android builds (flutter#4466)
  [webview_flutter] Update webview packages for Android and iOS to implement `runJavascript` and `runJavascriptReturningResult`. (flutter#4402)
  [camera] Fix CamcorderProfile Usages (flutter#4423)
  [google_sign_in] Use a transparent image as a placeholder for the `GoogleUserCircleAvatar` (flutter#4391)
  [in_app_purchase]IAP/platform interface add cancel status (flutter#4093)
  [image_picker] doc:readme image picker misprints (flutter#4421)
  Support Hybrid Composition through the GoogleMaps Widget (flutter#4082)
  [path_provider] started supporting background platform channels (flutter#4443)
  [device_info] started using Background Platform Channels (flutter#4456)

# Conflicts:
#	packages/webview_flutter/webview_flutter/CHANGELOG.md
#	packages/webview_flutter/webview_flutter/pubspec.yaml
amantoux pushed a commit to amantoux/plugins that referenced this pull request Dec 11, 2021
…ement `runJavascript` and `runJavascriptReturningResult`. (flutter#4402)

* [webview_flutter_platform_interface] Update webview platform interface with new methods for running JavaScript

* Fix tests

* [webview_flutter] Implemented `runJavaScript` and `runJavaScriptForResult` in iOS and Android packages.

* Remove accidental development team inclusion from project.pbxproj

* Implemented PR feedback

* Updated runJavaScriptForResult behaviour

* Implemented PR feedback partially

* Implement PR feedback

* Update changelog

* Implement PR feedback from interface PR

* Update changelog

* Revert inclusion of development team

* Implement PR feedback

* Implemented platform interface PR feedback

* Update pubspec dependency

* Fixed capitalisation

* Fixed capitalisation

* Fix warning

* Partially implement PR feedback

* Partially implement PR feedback

* Format

* Update podfile

* Update podfile

* Update podfiles

* Update iOS project files

* Update podspec

* Remove unnecessary podfile configuration

* Implemented PR feedback

* Format

* Revert podfile changes

* Implemented PR feedback

* Fix formatting

* Fix formatting

* Fixed test.

* Re-add integration tests for deprecated evaluateJavascript method.

* Fix merge conflicts
KyleFin pushed a commit to KyleFin/plugins that referenced this pull request Dec 21, 2021
…ement `runJavascript` and `runJavascriptReturningResult`. (flutter#4402)

* [webview_flutter_platform_interface] Update webview platform interface with new methods for running JavaScript

* Fix tests

* [webview_flutter] Implemented `runJavaScript` and `runJavaScriptForResult` in iOS and Android packages.

* Remove accidental development team inclusion from project.pbxproj

* Implemented PR feedback

* Updated runJavaScriptForResult behaviour

* Implemented PR feedback partially

* Implement PR feedback

* Update changelog

* Implement PR feedback from interface PR

* Update changelog

* Revert inclusion of development team

* Implement PR feedback

* Implemented platform interface PR feedback

* Update pubspec dependency

* Fixed capitalisation

* Fixed capitalisation

* Fix warning

* Partially implement PR feedback

* Partially implement PR feedback

* Format

* Update podfile

* Update podfile

* Update podfiles

* Update iOS project files

* Update podspec

* Remove unnecessary podfile configuration

* Implemented PR feedback

* Format

* Revert podfile changes

* Implemented PR feedback

* Fix formatting

* Fix formatting

* Fixed test.

* Re-add integration tests for deprecated evaluateJavascript method.

* Fix merge conflicts
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
2 participants