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

[webview_flutter] Only call onWebResourceError for main frame#3078

Merged
fluttergithubbot merged 9 commits intoflutter:masterfrom
bparrishMines:webresource
Aug 10, 2021
Merged

[webview_flutter] Only call onWebResourceError for main frame#3078
fluttergithubbot merged 9 commits intoflutter:masterfrom
bparrishMines:webresource

Conversation

@bparrishMines
Copy link
Copy Markdown
Contributor

@bparrishMines bparrishMines commented Sep 24, 2020

Description

onWebResourceError is called only for main frame on iOS and Android version below 23. This makes Android versions 23+ only use this callback for main frame as well.

Related Issues

Fixes flutter/flutter#64925

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • My PR includes unit or integration tests for all changed/updated/fixed behaviors (See Contributor Guide).
  • All existing and new tests are passing.
  • I updated/added relevant documentation (doc comments with ///).
  • The analyzer (flutter analyze) does not report any problems on my PR.
  • I read and followed the Flutter Style Guide.
  • The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • 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 signed the CLA.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

  • Yes, this is a breaking change (please indicate a breaking change in CHANGELOG.md and increment major revision).
  • No, this is not a breaking change.

@stuartmorgan-g stuartmorgan-g added the p: webview_flutter Edits files for a webview_flutter plugin label Jan 29, 2021
@stuartmorgan-g
Copy link
Copy Markdown
Contributor

Looks like this fell through the cracks. Could you add a version bump and resolve the conflict here? Other than that, it looks good to go.

);

expect(errorCompleter.future, doesNotComplete);
await Future.delayed(Duration(seconds: 5));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I missed this when looking at the PR before; why is there a 5-second sleep at the end of these tests? At the very least this needs a clear comment, but is there really not some reliable thing we can wait for to do whatever this is intended to do, rather than relying on timing?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think I believed there was a chance for an error to occur after the page finished loading, but I just tested it and that doesn't look like the case for the url provided. I changed it to wait for onPageFinished.

Copy link
Copy Markdown
Contributor

@stuartmorgan-g stuartmorgan-g left a comment

Choose a reason for hiding this comment

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

LGTM!

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

Labels

cla: yes p: webview_flutter Edits files for a webview_flutter plugin platform-android waiting for tree to go green (Use "autosubmit") This PR is approved and tested, but waiting for the tree to be green to land.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

onWebResourceError is called overzealously on Android

5 participants