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

[local_auth] Fix iOS crash when no localizedReason #3780

Merged
fluttergithubbot merged 7 commits into from
Apr 9, 2021
Merged

[local_auth] Fix iOS crash when no localizedReason #3780

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
25bead8
Fix iOS crash when no localizedReason
enricobenedos Apr 2, 2021
64db76c
Updated versioning files
enricobenedos Apr 2, 2021
52bfb1d
Removed null assertion due to null safety
enricobenedos Apr 8, 2021
60c4e52
Updated authenticate() method documnetation
enricobenedos Apr 8, 2021
4f83842
Merge branch 'master' into local_auth_app_crash
Apr 8, 2021
2f8a09a
Update packages/local_auth/CHANGELOG.md
enricobenedos Apr 9, 2021
8e3fe6e
Update packages/local_auth/lib/local_auth.dart
enricobenedos Apr 9, 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
Prev Previous commit
Next Next commit
Removed null assertion due to null safety
  • Loading branch information
@enricobenedos
enricobenedos committed Apr 8, 2021
commit 52bfb1d3bf303aefd19771a91e8d3df8815d8b4a
1 change: 0 additions & 1 deletion packages/local_auth/lib/local_auth.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,6 @@ class LocalAuthentication {
bool sensitiveTransaction = true,
bool biometricOnly = false,
}) async {
assert(localizedReason != null);
assert(localizedReason.isNotEmpty);
Copy link
Contributor

@cyanglaz cyanglaz Apr 2, 2021

Choose a reason for hiding this comment

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

What's the behavior on android if this is empty?

Copy link
Contributor Author

@enricobenedos enricobenedos Apr 4, 2021

Choose a reason for hiding this comment

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

On Android 11 (simulator) there are no problems if the localizedReason is empty. Is it a good idea to add the assert only on iOS case using Platform.isIOS?

Copy link
Contributor

@cyanglaz cyanglaz Apr 5, 2021

Choose a reason for hiding this comment

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

Is it a good idea to add the assert only on iOS case using Platform.isIOS?

Yes. If android can handle null, especially if null and empty string means different things on android, we should allow null on android. Do you know if there are difference between null and empty string on android?

Copy link
Contributor

@cyanglaz cyanglaz Apr 8, 2021

Choose a reason for hiding this comment

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

Looking at this again. I think you are right, we really shouldn't allow empty strings or null on android either. So the change looks good.

Copy link
Contributor

@cyanglaz cyanglaz Apr 8, 2021

Choose a reason for hiding this comment

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

@enricobenedos Let's make sure to mention the localizedReason must not be an empty String in the documentation.

Copy link
Contributor Author

@enricobenedos enricobenedos Apr 8, 2021

Choose a reason for hiding this comment

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

Hello @cyanglaz, sorry for the delay but I'm a little bit busy. I'm happy that you check that is it a good idea to also not give the possibility to input an empty string on Android. I will update the docs soon.

Copy link
Contributor

@cyanglaz cyanglaz Apr 8, 2021

Choose a reason for hiding this comment

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

Awesome! Thanks, once you update the doc we will land this!

Copy link
Contributor Author

@enricobenedos enricobenedos Apr 8, 2021

Choose a reason for hiding this comment

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

I've updated the authenticate method documentation.


final Map<String, Object> args = <String, Object>{
Expand Down