-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
[webview_flutter] Support for handling basic authentication requests (Android) #5454
[webview_flutter] Support for handling basic authentication requests (Android) #5454
Conversation
…/packages into webview-auth-request
…b/src/types/http_auth_request.dart
…s into webview-auth-request
FYI: There is a similar PR for other dialogs #4704 |
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.
Everything looks good. Just need to update the pubspecs.
|
||
# FOR TESTING AND INITIAL REVIEW ONLY. DO NOT MERGE. | ||
# See https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#changing-federated-plugins | ||
dependency_overrides: | ||
{webview_flutter_android: {path: ../../../webview_flutter/webview_flutter_android}, webview_flutter_platform_interface: {path: ../../../webview_flutter/webview_flutter_platform_interface}} |
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 can be removed and the minimum version of the webview_flutter_platform_interface
should be bumped to ^2.7.0
.
# FOR TESTING AND INITIAL REVIEW ONLY. DO NOT MERGE. | |
# See https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#changing-federated-plugins | |
dependency_overrides: | |
{webview_flutter_android: {path: ../../../webview_flutter/webview_flutter_android}, webview_flutter_platform_interface: {path: ../../../webview_flutter/webview_flutter_platform_interface}} |
|
||
# FOR TESTING AND INITIAL REVIEW ONLY. DO NOT MERGE. | ||
# See https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#changing-federated-plugins | ||
dependency_overrides: | ||
{webview_flutter_platform_interface: {path: ../../webview_flutter/webview_flutter_platform_interface}} |
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 be removed too.
# FOR TESTING AND INITIAL REVIEW ONLY. DO NOT MERGE. | |
# See https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#changing-federated-plugins | |
dependency_overrides: | |
{webview_flutter_platform_interface: {path: ../../webview_flutter/webview_flutter_platform_interface}} |
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.
LGTM
@stuartmorgan for secondary review
) { | ||
final void Function(HttpAuthRequest)? callback = | ||
weakThis.target?._onHttpAuthRequest; | ||
if (callback != null) { |
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.
Don't we need a default reply when there's no handler like we have on iOS?
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.
That's a good catch. I updated the PR
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.
LGTM
It would be good to have a test that the default handler is there and works (i.e., one that would have failed before the fix made for my previous comment), but that could be a follow-up.
…(Android) (flutter#5454) Adds the Android implementation for basic http authentication. This PR is part of a series of PRs that aim to close flutter/flutter#83556. The PR that contains all changes can be found at flutter#4140.
flutter/packages@b5958e2...1151191 2023-12-14 49699333+dependabot[bot]@users.noreply.github.com Bump github/codeql-action from 2.22.8 to 3.22.11 (flutter/packages#5674) 2023-12-13 ditman@gmail.com [ci][web] Ignore always_specify_types for JSArray. (flutter/packages#5669) 2023-12-13 mikemcguiness@protonmail.com [tool] Add support for `.java`, `.gradle`, `.sh`, and `.m` files� (flutter/packages#5567) 2023-12-13 ditman@gmail.com [google_sign_in] Update (web) example app. (flutter/packages#5634) 2023-12-13 34871572+gmackall@users.noreply.github.com [path_provider] De-flake getExternalStorageDirectories test (flutter/packages#5628) 2023-12-13 ditman@gmail.com [google_sign_in_web] Stop relying on framework internals. (flutter/packages#5660) 2023-12-13 43054281+camsim99@users.noreply.github.com [Android] Bump Gradle version to 7.6.3 (flutter/packages#5522) 2023-12-13 mikemcguiness@protonmail.com [google_sign_in] Adopt code excerpts in README (flutter/packages#5521) 2023-12-13 mikemcguiness@protonmail.com [css_colors] Adopt code excerpts in README (flutter/packages#5478) 2023-12-13 JeroenWeener@users.noreply.github.com [webview_flutter] Support for handling basic authentication requests (Android) (flutter/packages#5454) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC flutter-ecosystem@google.com,rmistry@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
…s cancelled by default (#5723) Added test in response to feedback from #5454 (review) Also regenerates mock files.
…5727) ## Description This pull request exposes the Android and iOS HTTP Basic Authentication feature to users of the `webview_flutter` plugin. It is the final PR in a sequence of PRs. Previous PRs are #5362, #5454 and #5455. Issues fixed by PR: Closes flutter/flutter#83556
…(Android) (flutter#5454) Adds the Android implementation for basic http authentication. This PR is part of a series of PRs that aim to close flutter/flutter#83556. The PR that contains all changes can be found at flutter#4140.
…s cancelled by default (flutter#5723) Added test in response to feedback from flutter#5454 (review) Also regenerates mock files.
…lutter#5727) ## Description This pull request exposes the Android and iOS HTTP Basic Authentication feature to users of the `webview_flutter` plugin. It is the final PR in a sequence of PRs. Previous PRs are flutter#5362, flutter#5454 and flutter#5455. Issues fixed by PR: Closes flutter/flutter#83556
Adds the Android implementation for basic http authentication.
This PR is part of a series of PRs that aim to close flutter/flutter#83556.
The PR that contains all changes can be found at #4140.
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.///
).