-
Notifications
You must be signed in to change notification settings - Fork 739
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
FTUE - Crash fixes #6862
FTUE - Crash fixes #6862
Conversation
@@ -260,11 +260,11 @@ class FtueAuthVariant( | |||
} | |||
|
|||
private fun onStartCombinedLogin() { | |||
addRegistrationStageFragmentToBackstack(FtueAuthCombinedLoginFragment::class.java) | |||
addRegistrationStageFragmentToBackstack(FtueAuthCombinedLoginFragment::class.java, allowStateLoss = true) |
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.
allowing state loss for the entry fragments is the fix for #6861
try { | ||
inflate() | ||
} catch (e: Exception) { | ||
val isMissingWebView = e.crawlCausesFor { it.message?.contains("MissingWebViewPackageException").orFalse() } |
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.
we have to crawl through the exception causes to find the missing webview package exception, it's a bit hacky~
} | ||
|
||
private fun inflateWebViewOrShowError() { | ||
views.loginCaptchaWebViewStub.inflateWebView(onError = { |
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.
using the ViewStub
and handling the inflation error is the fix for #6855
@@ -492,17 +489,6 @@ class OnboardingViewModel @AssistedInject constructor( | |||
|
|||
private fun handleInitWith(action: OnboardingAction.InitWith) { | |||
loginConfig = action.loginConfig | |||
// If there is a pending email validation continue on this step | |||
try { |
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.
Removing the attempt to fast forward the onboarding state to the wait for email verification stage when the app has been killed and relaunched is the fix for #6860
The OnboardingViewModel
and OnboardingViewState
already support and are restored by the system restoration process. If the process has been destroyed we need to accumulate the previous view states again otherwise we end up with invalid state.
} | ||
} | ||
} | ||
} |
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.
Could be rewritten to this compact, but maybe less clear:
private fun Throwable.crawlCausesFor(predicate: (Throwable) -> Boolean): Boolean {
return predicate(this) || cause?.crawlCausesFor(predicate) ?: false
}
Also, maybe move this extension to a dedicated file for Throwable extensions?
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.
will extract 👍
as the function is recursive I'd lean towards making the exit conditions as clear as possible, having the recursive call the last line allows us to opt into the tailrec optimisations (although it hasn't been used here since the crash stack is fairly small)
…n occurs asynchronusly
…nsaction occurs asynchronusly
… app is destroyed - this behaviour puts the app in an invalid state as we've lost all the ViewState we've collect from the previous onboarding steps - the app already handles restoring the onboarding state via the system restoration
… the system webview is missing or has been disabled
0b2b28d
to
8b70f3a
Compare
|
SonarCloud Quality Gate failed. |
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
Type of change
Content
OnboardingViewState
to be in an invalid empty state. Fixes Crash when returning to app after closing during email verification #6860WebView
xml declaration with aViewStub
which try/catches the inflation. Allows us to catch the missing webview crash and provide an error dialog with information about the error. Fixes App Crashes On Email Verification (Sign Up) #6855already have an account
oruse case selection
buttons again. Fixes Crash - IllegalStateException: Can not perform this action after onSaveInstanceState #6861Motivation and context
Screenshots / GIFs
WebView disabled
*animations are disabled
Tests
Tested devices