Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[webview_flutter] Adds option to register JavaScript console callback #4541

Closed
wants to merge 24 commits into from
Closed

[webview_flutter] Adds option to register JavaScript console callback #4541

wants to merge 24 commits into from

Conversation

mvanbeusekom
Copy link
Contributor

Adds an options to register a JavaScript console callback, which will allow developers to receive JavaScript console messages in a Dart callback.

Resolves flutter/flutter#32908

@bparrishMines, could you please add the necessary code to bridge the Android WebChromeClient.onConsoleMessage callback to Dart (Android API documentation: https://developer.android.com/reference/android/webkit/WebChromeClient#onConsoleMessage(android.webkit.ConsoleMessage))?

If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.

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.

@mvanbeusekom mvanbeusekom force-pushed the feature/forward_javascript_console branch from 18b1053 to cd2a81c Compare July 21, 2023 15:48
@mvanbeusekom mvanbeusekom force-pushed the feature/forward_javascript_console branch from c0ac04e to c1091ac Compare July 24, 2023 08:32
@bparrishMines
Copy link
Contributor

@mvanbeusekom Has this PR approved Maintainers are allowed to edit this pull request.. I haven't been able push to it.

@mvanbeusekom mvanbeusekom marked this pull request as ready for review July 25, 2023 19:16
@mvanbeusekom mvanbeusekom marked this pull request as draft July 25, 2023 19:16
@mvanbeusekom
Copy link
Contributor Author

@mvanbeusekom Has this PR approved Maintainers are allowed to edit this pull request.. I haven't been able push to it.

The option seems to be missing, most likely since I am pushing from the Baseflow organizational account. I will try another way to provide you with write access.

@mvanbeusekom mvanbeusekom force-pushed the feature/forward_javascript_console branch 2 times, most recently from 6a3a91b to bdb30a7 Compare July 27, 2023 09:31
@mvanbeusekom mvanbeusekom marked this pull request as ready for review July 27, 2023 11:22
@mvanbeusekom
Copy link
Contributor Author

@bparrishMines, I have completed the Apple, Android and App facing implementations. Could you do an initial review before we split this PR up in separate PRs per package?

@bparrishMines bparrishMines added the federated: all_changes PR that contains changes for all packages for a federated plugin change label Jul 27, 2023
@mvanbeusekom
Copy link
Contributor Author

@bparrishMines, @stuartmorgan,

During additional testing I think I discovered a bug in Androids mapping of the JavaScript console.debug and console.info methods, these are mapped to the ConsoleMessage.MessageLevel.tip and ConsoleMessage.MessageLevel.log respectively.

The mapping I expected was as follows:

JavaScript method Expected Actual
console.error MessageLevel.ERROR MessageLevel.ERROR
console.warn MessageLevel.WARNING MessageLevel.WARNING
console.info MessageLevel.TIP MessageLevel.LOG
console.debug MessageLevel.DEBUG MessageLevel.TIP
console.log MessageLevel.LOG MessageLevel.LOG

Question is, how should I treat this in the plugin? I can remap MessageLevel.TIP to MessageLevel.DEBUG, but I cannot differentiate between console.info and console.log as both will result in the MessageLevel.LOG value. I can also copy the current behavior to Dart but that might result in changes later on if this is addressed in Android side.

@stuartmorgan
Copy link
Contributor

I think it's fine to have our enum have the 5 JS options, and just comment that platforms may not preserve all the level information so clients should not rely on a 1:1 mapping between the JS calls. Then map TIP to debug in the implementation, with a comment explaining why, and pass along the info/log conflations as a platform limitation.

This was referenced Aug 15, 2023
Merged
Merged
Merged
Merged
auto-submit bot pushed a commit that referenced this pull request Sep 7, 2023
@mvanbeusekom
[webview_flutter_platform_interface] Adds option to override console …
1598ccd 
…log (#4701)

Adds an option to the `webview_flutter_platform_interface` to register a JavaScript console callback. This will allow developers to receive JavaScript console messages in a Dart callback.

This PR contains the `webview_flutter_platform_interface` changes from PR #4541.

Related issue: flutter/flutter#32908

*If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*
@stuartmorgan
Copy link
Contributor

Update from triage: interface landed, implementation PRs are just waiting for review.

auto-submit bot pushed a commit that referenced this pull request Sep 21, 2023
@mvanbeusekom
[webview_flutter_wkwebview] Adds WKWebView implementation to override…
7eee49a 
… console log (#4703)

Adds the WKWebView implementation for registering a JavaScript console callback. This will allow developers to receive JavaScript console messages in a Dart callback.

This PR contains the `webview_flutter_wkwebview` specific changes from PR #4541.

Related issue: flutter/flutter#32908

*If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*
@mvanbeusekom
Copy link
Contributor Author

Closing this PR in favor of PRs separated per platform.

auto-submit bot pushed a commit that referenced this pull request Sep 29, 2023
@mvanbeusekom
[webview_flutter_android] Adds Android implementation to override con…
eac45de 
…sole log (#4702)

Adds the Android implementation for registering a JavaScript console callback. This will allow developers to receive JavaScript console messages in a Dart callback.

This PR contains the `webview_flutter_android` specific changes from PR #4541.

Related issue: flutter/flutter#32908

*If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*
auto-submit bot pushed a commit that referenced this pull request Oct 1, 2023
@mvanbeusekom
[webview_flutter] Adds app facing implementation to override console …
c933398 
…log (#4705)

Adds the app facing implementation for registering a JavaScript console callback. This will allow developers to receive JavaScript console messages in a Dart callback.

This PR contains the `webview_flutter` specific changes from PR #4541.

Fixes flutter/flutter#32908

*If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*
HugoOlthof pushed a commit to moneybird/packages that referenced this pull request Dec 13, 2023
@mvanbeusekom @HugoOlthof
[webview_flutter_wkwebview] Adds WKWebView implementation to override…
a0b1de3 
… console log (flutter#4703)

Adds the WKWebView implementation for registering a JavaScript console callback. This will allow developers to receive JavaScript console messages in a Dart callback.

This PR contains the `webview_flutter_wkwebview` specific changes from PR flutter#4541.

Related issue: flutter/flutter#32908

*If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*
HugoOlthof pushed a commit to moneybird/packages that referenced this pull request Dec 13, 2023
@mvanbeusekom @HugoOlthof
[webview_flutter_android] Adds Android implementation to override con…
e41b5bb 
…sole log (flutter#4702)

Adds the Android implementation for registering a JavaScript console callback. This will allow developers to receive JavaScript console messages in a Dart callback.

This PR contains the `webview_flutter_android` specific changes from PR flutter#4541.

Related issue: flutter/flutter#32908

*If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*
HugoOlthof pushed a commit to moneybird/packages that referenced this pull request Dec 13, 2023
@mvanbeusekom @HugoOlthof
[webview_flutter] Adds app facing implementation to override console …
eeafcfe 
…log (flutter#4705)

Adds the app facing implementation for registering a JavaScript console callback. This will allow developers to receive JavaScript console messages in a Dart callback.

This PR contains the `webview_flutter` specific changes from PR flutter#4541.

Fixes flutter/flutter#32908

*If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@navaronbracke navaronbracke navaronbracke left review comments

@cyanglaz cyanglaz Awaiting requested review from cyanglaz

@ditman ditman Awaiting requested review from ditman

@bparrishMines bparrishMines Awaiting requested review from bparrishMines bparrishMines is a code owner

Assignees
No one assigned
Labels
federated: all_changes PR that contains changes for all packages for a federated plugin change p: webview_flutter platform-android platform-ios
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Provide method to view webview console logs
4 participants
@mvanbeusekom @bparrishMines @stuartmorgan @navaronbracke