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

[webview_flutter] Add unimplemented features to the web version. #5808

Closed
wants to merge 17 commits into from
Closed

[webview_flutter] Add unimplemented features to the web version. #5808

wants to merge 17 commits into from

Conversation

FlafyDev
Copy link
Contributor

@FlafyDev FlafyDev commented May 23, 2022 •
edited
Loading

  • Adds support for Javascript channels and raw evaluation of Javascript for the web.
    The methods that were implemented in this PR are the following:
    evaluateJavascript, runJavascript, and runJavascriptReturningResult
    These only work when setting the content with loadHtmlString because of security reasons browsers have with iframes.
    The methods will also not work with loadRequest, because it can load things other than HTML and I don't know how to detect whether it is HTML or not (The HTML needs to be preprocessed.)
  • Fixes the unreliable encoding of the data in loadRequest.
    loadHtmlString also had the unreliable encoding, but it's been changed and now doesn't have any encoding.
    The new encoding is taken from WebViewX.

Tests added

  • testWidgets('loadHtml', ...) - Tests the functionality of loadHtmlString. This test doesn't pass without solving the issue linked above.
  • testWidgets('JavascriptChannel', ...) - Tests the communication between Flutter and the Webview with Javascript channels and raw Javascript evaluation.

Technical Details

Technical Details

The communication between the IFrame's HTML and the global scope is done by preprocessing the HTML to add a script element to expose the IFrame's Window object to the global scope. With the Window object of the IFrame's HTML you can evaluate raw Javascript.
Preprocessing the HTML also handles the Javascript channels, which is why you can't add/remove them after loading the HTML.

The preprocessing of the HTML creates the necessary elements ("", "html", "head", "body") in case they aren't present. It also creates a script element to expose the Window object and handle Javascript channels.

So for example, <div>Hello, world</div> will turn into this after the preprocessing:

<!DOCTYPE html>
<html>
  <head>
    <script>
      parent.webview0_getWindow(window);
      window.Echo = { postMessage: (message) => parent.webview0_channel("Echo", message) };
    </script>
  </head>
  <body>
    <div>Hello, world</div>
  </body>
</html>

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/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, 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.

@FlafyDev FlafyDev requested a review from ditman as a code owner May 23, 2022 17:38
@github-actions github-actions bot added p: webview_flutter Edits files for a webview_flutter plugin platform-web labels May 23, 2022
@FlafyDev
Copy link
Contributor Author

FlafyDev commented May 23, 2022 •
edited
Loading

Why do the Github tests fail? I don't understand the output.

@balvinderz
Copy link
Contributor

These tests are failing:

  1. WebWebViewPlatformController loadHtmlString loads html into iframe
  2. WebWebViewPlatformController loadRequest loadRequest makes request and loads response into iframe

@FlafyDev
Copy link
Contributor Author

Where are these tests located at? I don't see them in webview_flutter_web/example/integration_test/webview_flutter_test.dart and the tests there all pass

@balvinderz
Copy link
Contributor

https://github.com/FlafyDev/plugins/blob/98ce7a8e53b1ee73666eafbfd2219371ee8db16e/packages/webview_flutter/webview_flutter_web/test/webview_flutter_web_test.dart#L57
https://github.com/FlafyDev/plugins/blob/98ce7a8e53b1ee73666eafbfd2219371ee8db16e/packages/webview_flutter/webview_flutter_web/test/webview_flutter_web_test.dart#L89

@FlafyDev FlafyDev mentioned this pull request May 25, 2022
Closed
@stuartmorgan
Copy link
Contributor

Thanks for the submission! Please split this into two separate PR; this is doing two very different things:

  • Fixing a bug, which would would definitely accept a high-quality PR for
  • Adding a (very) partial implementation of a feature in a way that may have very surprising side effects for people, which needs discussion and evaluation to decide if it's something we would move forward with.

@FlafyDev
Copy link
Contributor Author

Thanks for the submission! Please split this into two separate PR; this is doing two very different things:

* Fixing a bug, which would would definitely accept a high-quality PR for

* Adding a (very) partial implementation of a feature in a way that may have very surprising side effects for people, which needs discussion and evaluation to decide if it's something we would move forward with.

I agree, this should be 2 different PRs. I'll revert the bug fix and make a new PR for it.

@FlafyDev FlafyDev mentioned this pull request May 26, 2022
Merged
11 tasks
@stuartmorgan stuartmorgan self-requested a review May 26, 2022 20:18
@ditman
Copy link
Member

ditman commented Jun 2, 2022

I'm not sure that this can be landed as is, too spicy! I'm surprised that the analyzer didn't want more // ignore: unsafe_html around!

I think the most appropriate way of doing this (but I don't have a proper design doc yet) would be to create some JS code that gets distributed alongside the webview_flutter_web plugin, as an asset, that can be loaded by the iframes that users want to embed.

That JS would configure the iframe in a way that "collaborates" with the embedding flutter app, by establishing a proper communication channel through the postMessage API and MessageEvents (without the need to expose window or parent). The flutter app can call different methods on the contentWindow of the IFrameElement.

Read more about postMessage, here: https://developer.mozilla.org/en-US/docs/Web/API/Window/postMessage

@stuartmorgan
Copy link
Contributor

Based on the comments above, I'm going to close this, as it's not an implementation we would move forward with. If you are interesting in supporting message posting in general, using an approach like the one described in @ditman's comment above, the next step would be a design document, which could be posted for discussion in flutter/flutter#101758 once drafted. Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

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

@stuartmorgan stuartmorgan Awaiting requested review from stuartmorgan

Assignees
No one assigned
Labels
p: webview_flutter Edits files for a webview_flutter plugin platform-web
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@FlafyDev @balvinderz @stuartmorgan @ditman