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

[webview_flutter] Implementations of loadFile and loadHtmlString for WKWebView #4486

Merged
merged 8 commits into from
Nov 12, 2021
Merged

[webview_flutter] Implementations of loadFile and loadHtmlString for WKWebView #4486

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
8e05ac2
Merge remote-tracking branch 'upstream/master' into webview_load_file…
mvanbeusekom Nov 5, 2021
c1dc83a
Implementation of loadFile for WKWebView.
mvanbeusekom Nov 5, 2021
f1343c2
Implementation of loadHtmlString for WKWebView.
mvanbeusekom Nov 8, 2021
0351601
Merge remote-tracking branch 'upstream/master' into webview_load_file…
mvanbeusekom Nov 8, 2021
b945839
Bump version number
mvanbeusekom Nov 9, 2021
5218c0a
Merge remote-tracking branch 'upstream/master' into webview_load_file…
mvanbeusekom Nov 11, 2021
d5cf9f9
Apply feedback on PR
mvanbeusekom Nov 11, 2021
98eb236
Fix final nits
mvanbeusekom Nov 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions packages/webview_flutter/webview_flutter_wkwebview/example/lib/main.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,11 @@ const String kLocalFileExamplePage = '''
<body>

<h1>Local demo page</h1>
<p>This is an example page used to demonstrate how to load a local file or HTML string using the <a href="https://pub.dev/packages/webview_flutter">Flutter webview</a> plugin.</p>
<p>
This is an example page used to demonstrate how to load a local file or HTML
string using the <a href="https://pub.dev/packages/webview_flutter">Flutter
webview</a> plugin.
</p>

</body>
</html>
Expand Down Expand Up @@ -343,10 +347,6 @@ class _SampleMenu extends StatelessWidget {
final String tmpDir = (await getTemporaryDirectory()).path;
File indexFile = File('$tmpDir/www/index.html');

if (await indexFile.exists()) {
return indexFile.path;
}

await Directory('$tmpDir/www').create(recursive: true);
await indexFile.writeAsString(kLocalFileExamplePage);

Expand Down
2 changes: 0 additions & 2 deletions packages/webview_flutter/webview_flutter_wkwebview/example/lib/web_view.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,6 @@ class WebViewController {
Future<void> loadFile(
String absoluteFilePath,
) {
assert(absoluteFilePath != null || absoluteFilePath.isNotEmpty);
Copy link
Contributor

@stuartmorgan-g stuartmorgan-g Nov 11, 2021

Choose a reason for hiding this comment

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

Don't you still want the non-emtpy assert here and below?

return _webViewPlatformController.loadFile(absoluteFilePath);
}

Expand All @@ -327,7 +326,6 @@ class WebViewController {
String html, {
String? baseUrl,
}) {
assert(html != null || html.isNotEmpty);
return _webViewPlatformController.loadHtmlString(
html,
baseUrl: baseUrl,
Expand Down
21 changes: 20 additions & 1 deletion packages/webview_flutter/webview_flutter_wkwebview/ios/Classes/FlutterWebView.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,17 @@ - (void)onLoadFile:(FlutterMethodCall*)call result:(FlutterResult)result {
}

NSURL* url = [NSURL fileURLWithPath:[call arguments] isDirectory:NO];
Copy link
Contributor

@stuartmorgan-g stuartmorgan-g Nov 11, 2021

Choose a reason for hiding this comment

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

You should validate that this is non-nil before passing it below (returning an error if it is nil) in case someone passes a garbage string that doesn't parse as a path.

Copy link
Contributor Author

@mvanbeusekom mvanbeusekom Nov 11, 2021

Choose a reason for hiding this comment

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

Added additional check to ensure the url variable is not nil, return an error if the variable is nil.


if (!url) {
NSString* errorDetails = [NSString stringWithFormat:@"Initializing NSURL with the supplied "
@"'%@' path resulted in a nil value.",
[call arguments]];
result([FlutterError errorWithCode:@"loadFile_failed"
message:@"Failed parsing file path."
details:errorDetails]);
return;
}

NSURL* baseUrl = [url URLByDeletingLastPathComponent];

[_webView loadFileURL:url allowingReadAccessToURL:baseUrl];
Expand All @@ -214,7 +225,7 @@ - (void)onLoadFile:(FlutterMethodCall*)call result:(FlutterResult)result {

- (void)onLoadHtmlString:(FlutterMethodCall*)call result:(FlutterResult)result {
NSDictionary* arguments = [call arguments];
if (!arguments || ![arguments isKindOfClass:NSDictionary.class]) {
if (![arguments isKindOfClass:NSDictionary.class]) {
result([FlutterError
errorWithCode:@"loadHtmlString_failed"
message:@"Failed parsing arguments."
Expand Down Expand Up @@ -603,6 +614,14 @@ - (void)updateUserAgent:(NSString*)userAgent {
}
}

/**
* Validates if the given `argument` is a non-null non-empty string.
*
* @param argument The argument that should be validated.
* @param errorDetails An optional NSString variable which will contain a detailed error message in
* case the supplied argument is not valid.
* @return `true` if the given `argument` is a valid non-null, non-empty string; otherwise `false`.
Copy link
Contributor

@stuartmorgan-g stuartmorgan-g Nov 11, 2021

Choose a reason for hiding this comment

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

Nit: "YES" and "NO", not "true" and "false"

*/
+ (bool)isValidStringArgument:(id)argument withErrorMessage:(NSString**)errorDetails {
Copy link
Contributor

@stuartmorgan-g stuartmorgan-g Nov 11, 2021

Choose a reason for hiding this comment

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

Needs a declaration comment, per style guide.

Copy link
Contributor Author

@mvanbeusekom mvanbeusekom Nov 11, 2021

Choose a reason for hiding this comment

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

Done.

Copy link
Contributor

@stuartmorgan-g stuartmorgan-g Nov 11, 2021

Choose a reason for hiding this comment

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

Oops, and I missed that this is bool; it should be BOOL.

if (!argument) {
if (errorDetails) {
Expand Down