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

[url_launcher] Fix PlatformException introduced in nnbd release #3307

Closed
wants to merge 2 commits into from
Closed

[url_launcher] Fix PlatformException introduced in nnbd release #3307

wants to merge 2 commits into from

Conversation

nohli
Copy link
Contributor

@nohli nohli commented Dec 8, 2020

Description

Fixes PlatformException NOT_A_WEB_SCHEME

forceSafariVC has previously been null (not true) by default.

Since the description (below) still says that it is false (unset) by default, I think, it is desired to set it to false (keeping it as true by default would need updates to the description as well as the if clause causing the PlatformException:

/// [forceSafariVC] is only used in iOS with iOS version >= 9.0. By default (when unset), the launcher
/// opens web URLs in the Safari View Controller, anything else is opened
/// using the default handler on the platform.
  if ((forceSafariVC == true || forceWebView == true) && !isWebURL) {
    throw PlatformException(
        code: 'NOT_A_WEB_SCHEME',
        message: 'To use webview or safariVC, you need to pass'
            'in a web URL. This $urlString is not a web URL.');
  }

Related Issues

flutter/flutter#71725

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.

@nohli nohli changed the base branch from master to nnbd December 8, 2020 14:23
@google-cla google-cla bot added the cla: yes label Dec 8, 2020
@nohli nohli changed the title [url_launcher] Pull request [url_launcher] Fix PlatformException introduced in nnbd release Dec 8, 2020
@mvanbeusekom
Copy link
Contributor

Can you add a unit-test which simply tests if the default values are correct? This will prevent similar mistake in the future.

@blasten
Copy link

blasten commented Dec 14, 2020

this branch has been merged into master. You might want to reopen this PR, but against master.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants