-
Notifications
You must be signed in to change notification settings - Fork 3.3k
[webview_flutter_wkwebview] Extended Web View API on iOS to add flexibility when working with local HTML content #8787
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
base: main
Are you sure you want to change the base?
Conversation
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
5e0c659
to
3417b52
Compare
3417b52
to
bc4128f
Compare
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.
@agavrilko To follow the pattern of other methods, we should introduce a new platform interface method to webview_flutter_platform_interface
. Something like PlatformWebViewController.loadFileWithParams(LoadFileParams)
. LoadFileParams
would contain a url
parameter. Then the webview_flutter_wkwebview
package can create its own implementation of LoadFileParams
with the readAccessUrl
parameter because it is unique to iOS/macOS.
Hello @bparrishMines, I initially aimed to introduce this minor update without making any API changes, focusing solely on exposing the functionality that was already present but not accessible to users. Based on my understanding, your suggestion would require:
While this design would make the API more consistent and extensible, it would also introduce a breaking change for current users, requiring them to update their code. And if my understanding is correct, should I just update an existing |
@agavrilko I think you should create a new platform interface method named something like: Everything else you stated seems correct. Additionally, after this feature lands, |
bc4128f
to
afad597
Compare
afad597
to
e3639a2
Compare
Hello @bparrishMines I'm seeing that the pipeline fails due to a version resolution issue:
Could you please advise on the proper workflow for updating versions across the packages? Thank you! |
@agavrilko Yes, you can follow https://github.com/flutter/flutter/blob/master/docs/ecosystem/contributing/README.md#changing-federated-plugins for the process. |
d08642a
to
bffcb4e
Compare
bffcb4e
to
f3090f0
Compare
Hello @bparrishMines, I need further assistance with this PR. But it looks like changing
Should this change be split into 2 different PRs? |
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.
@agavrilko Yes, this PR won't be published as is. The first step is to get this PR approved first, then individual PRs for each package will be created. This allows us to review all the changes together before we start landing seaprate PRs.
/// such as headers (on Android) or resource paths (on iOS/macOS). | ||
/// | ||
/// Throws a `PlatformException` if the [absoluteFilePath] does not exist. | ||
Future<void> loadFileWithParams(LoadFileParams params) { |
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 method shouldn't be necessary. A user can now use the platform specific loadFileWithParams
to add platform specific parameters.
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.
I am not sure I am property understand this comment.
Do you suggest to remove this method from the WebViewController
class?
The loading of the file with custom parameters would look like the following:
switch (controller.platform) {
case final WebKitWebViewController platform:
platform.loadFileWithParams(...)default:
...
}
Just want to ensure I understand it right.
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.
Correct. WebViewController.loadFileWithParams
should be removed and users could use the code snippet you provided to access the platform-specific params. The goal of the app-facing package is to provide a simple and clean API while the platform interface provides a more robust and extendable API.
@@ -131,10 +131,25 @@ class WebViewController { | |||
/// `/Users/username/Documents/www/index.html`. | |||
/// | |||
/// Throws a `PlatformException` if the [absoluteFilePath] does not exist. | |||
@Deprecated('Use loadFileWithParams(LocalFileParams params) instead. ' |
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 shouldn't need deprecation. This can just change the platform
call from platform.loadFile
to platform.loadFileWithParams
.
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.
Updated.
/// Platform-specific implementations can add additional fields by extending | ||
/// this class. | ||
/// | ||
/// {@tool sample} |
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.
/// {@tool sample} |
This doesn't actually do anything right now. We just haven't removed them from other classes yet.
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.
Removed.
/// final String readAccessPath; | ||
/// } | ||
/// ``` | ||
/// {@end-tool} |
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.
/// {@end-tool} |
Same here
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.
Removed.
/// this class. | ||
/// | ||
/// {@tool sample} | ||
/// This example demonstrates how to extend [LocalFileParams] to provide |
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 example demonstrates how to extend [LocalFileParams] to provide | |
/// This example demonstrates how to extend [LoadFileParams] to provide |
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.
Fixed.
class WebKitLoadFileParams extends LoadFileParams { | ||
/// Constructs a [WebKitLoadFileParams], the subclass of a [LoadFileParams]. | ||
/// | ||
/// The optional [readAccessPath] defines the directory that the WebView is |
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.
I think this information should be moved to the documentation for the field.
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.
Moved.
class AndroidLoadFileParams extends LoadFileParams { | ||
/// Constructs a [AndroidLoadFileParams], the subclass of a [LoadFileParams]. | ||
/// | ||
/// Optionally, [headers] map may be included, which contains key-value pairs |
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.
I think this information should be moved to the field.
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.
Moved.
/// that will be passed as additional HTTP headers when loading the file. | ||
AndroidLoadFileParams({ | ||
required String absoluteFilePath, | ||
this.headers = _defaultHeaders, |
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 work right?
this.headers = _defaultHeaders, | |
this.headers = const <String, String>{}, |
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.
Updated.
@@ -383,15 +425,19 @@ class AndroidWebViewController extends PlatformWebViewController { | |||
_webView.pigeon_instanceManager.getIdentifier(_webView)!; | |||
|
|||
@override | |||
Future<void> loadFile( |
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.
As stated in the iOS implementation as well, we keep loadFile
and it should just call loadFileWithParams
.
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.
Updated.
@@ -274,7 +274,7 @@ void main() { | |||
)); | |||
} | |||
|
|||
test('loadFile without file prefix', () async { | |||
test('Initializing WebView settings on controller creation', () async { |
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.
Since the loadFile
method will be kept in AndroidWebViewController
, one of the tests for loadFile
should be kept.
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.
'loadFile' tests restored under 'loadFile' group.
NOTE: the test on 277 line was not related to the 'loadFile' method, this change just fixes the label.
flutter/flutter#136479 • Deprecates `WebViewController.loadFile(String)` in favor of `WebViewController.loadFileWithParams(LoadFileParams)`. • Introduces `AndroidLoadFileParams` and `WebKitLoadFileParams` to support platform-specific parameters when loading local HTML files on Android and iOS/macOS.
f3090f0
to
9531f5c
Compare
Overview
The
readAccessURLProvider
property has been added to theWebKitWebViewControllerCreationParams
class. This function allows customization of the path to associated resources when loading a local HTML page usingloadFile
on iOS.What Is the Issue
[#136479] readAccessUrl of the webview_flutter_wkwebview should be accessible for customization
The native
WKWebView
takes two arguments when loading local HTML pages:WKWebView
can access (e.g., .css, .js files).Currently, the Flutter implementation always sets the parent folder of the specified HTML file as the read access directory. This behavior is not configurable, which limits how local web content can be structured.
For example, the following structure does not work because the .css and .js files are not in the parent directory of the HTML file:
With the existing behavior,
WKWebView
cannotaccess styles/main.css
andscripts/app.js
because the parent folder ofindex.html
(/pages/
) does not contain them.How This Resolves the Issue
The
readAccessURLProvider
property has been added to theWebKitWebViewControllerCreationParams
class. This property accepts a function that takes the path of the HTML file being loaded and returns the path to the associated resources.Each time
loadFile
is called, the controller invokes this function and passes its result as the second argument to the underlyingWKWebView
implementation.By default, the function returns the parent directory of the specified HTML file, preserving the existing behavior when no custom provider is set.
Impact on End Users
WKWebView
should allow access to when loading a local HTML page, providing greater flexibility in organizing assets.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, or this PR is exempt from CHANGELOG changes.///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.