From 2b382d12495f2fea7ee01c0d0ac841091980f297 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Tue, 16 Aug 2022 15:54:32 +0100 Subject: [PATCH 1/8] allowing the combined login flow to have state loss as the transaction occurs asynchronusly --- .../app/features/onboarding/ftueauth/FtueAuthVariant.kt | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt index 150ab74ec2..97628f81d9 100644 --- a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt +++ b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt @@ -260,7 +260,7 @@ class FtueAuthVariant( } private fun onStartCombinedLogin() { - addRegistrationStageFragmentToBackstack(FtueAuthCombinedLoginFragment::class.java) + addRegistrationStageFragmentToBackstack(FtueAuthCombinedLoginFragment::class.java, allowStateLoss = true) } private fun openStartCombinedRegister() { @@ -519,13 +519,14 @@ class FtueAuthVariant( ) } - private fun addRegistrationStageFragmentToBackstack(fragmentClass: Class, params: Parcelable? = null) { + private fun addRegistrationStageFragmentToBackstack(fragmentClass: Class, params: Parcelable? = null, allowStateLoss: Boolean = false) { activity.addFragmentToBackstack( views.loginFragmentContainer, fragmentClass, params, tag = FRAGMENT_REGISTRATION_STAGE_TAG, - option = commonOption + option = commonOption, + allowStateLoss = allowStateLoss, ) } From 9a5b21d8f104caac2c0785e0a076950c8c0b1054 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Tue, 16 Aug 2022 16:15:35 +0100 Subject: [PATCH 2/8] allowing the combined registration flow to have state loss as the transaction occurs asynchronusly --- .../vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt index 97628f81d9..1f7a26b829 100644 --- a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt +++ b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt @@ -264,7 +264,7 @@ class FtueAuthVariant( } private fun openStartCombinedRegister() { - addRegistrationStageFragmentToBackstack(FtueAuthCombinedRegisterFragment::class.java) + addRegistrationStageFragmentToBackstack(FtueAuthCombinedRegisterFragment::class.java, allowStateLoss = true) } private fun displayFallbackWebDialog() { From 1d03460aee8471666b665adf389d1aa3af502fe9 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Wed, 17 Aug 2022 11:38:30 +0100 Subject: [PATCH 3/8] removing ability to shortcut registration to waiting for email if the 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 --- .../features/onboarding/OnboardingViewModel.kt | 16 +--------------- .../onboarding/OnboardingViewModelTest.kt | 15 --------------- 2 files changed, 1 insertion(+), 30 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt b/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt index 73288bd6d5..6106d84a8d 100644 --- a/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt @@ -71,7 +71,7 @@ import java.util.concurrent.CancellationException * */ class OnboardingViewModel @AssistedInject constructor( - @Assisted initialState: OnboardingViewState, + @Assisted private val initialState: OnboardingViewState, private val applicationContext: Context, private val authenticationService: AuthenticationService, private val activeSessionHolder: ActiveSessionHolder, @@ -123,9 +123,6 @@ class OnboardingViewModel @AssistedInject constructor( private val registrationWizard: RegistrationWizard get() = authenticationService.getRegistrationWizard() - val currentThreePid: String? - get() = registrationWizard.getCurrentThreePid() - // True when login and password has been sent with success to the homeserver val isRegistrationStarted: Boolean get() = authenticationService.isRegistrationStarted() @@ -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 { - if (registrationWizard.isRegistrationStarted()) { - currentThreePid?.let { - handle(OnboardingAction.PostViewEvent(OnboardingViewEvents.OnSendEmailSuccess(it, isRestoredSession = true))) - } - } - } catch (e: Throwable) { - // NOOP. API is designed to use wizards in a login/registration flow, - // but we need to check the state anyway. - } } private fun handleResetPassword(action: OnboardingAction.ResetPassword) { diff --git a/vector/src/test/java/im/vector/app/features/onboarding/OnboardingViewModelTest.kt b/vector/src/test/java/im/vector/app/features/onboarding/OnboardingViewModelTest.kt index b505d05944..93195e2f12 100644 --- a/vector/src/test/java/im/vector/app/features/onboarding/OnboardingViewModelTest.kt +++ b/vector/src/test/java/im/vector/app/features/onboarding/OnboardingViewModelTest.kt @@ -191,21 +191,6 @@ class OnboardingViewModelTest { .finish() } - @Test - fun `given registration started with currentThreePid, when handling InitWith, then emits restored session OnSendEmailSuccess`() = runTest { - val test = viewModel.test() - fakeAuthenticationService.givenRegistrationWizard(FakeRegistrationWizard().also { - it.givenRegistrationStarted(hasStarted = true) - it.givenCurrentThreePid(AN_EMAIL) - }) - - viewModel.handle(OnboardingAction.InitWith(LoginConfig(A_HOMESERVER_URL, identityServerUrl = null))) - - test - .assertEvents(OnboardingViewEvents.OnSendEmailSuccess(AN_EMAIL, isRestoredSession = true)) - .finish() - } - @Test fun `given registration not started, when handling InitWith, then does nothing`() = runTest { val test = viewModel.test() From ca10109a655d7f82383f2eacddf05fa3f7981451 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Wed, 17 Aug 2022 13:51:58 +0100 Subject: [PATCH 4/8] coverting captcha webview to a viewstub in order to catch errors when the system webview is missing or has been disabled --- .../ftueauth/FtueAuthCaptchaFragment.kt | 62 ++++++++++++++++++- .../layout/fragment_ftue_login_captcha.xml | 18 +++++- .../src/main/res/layout/view_stub_webview.xml | 4 ++ 3 files changed, 78 insertions(+), 6 deletions(-) create mode 100644 vector/src/main/res/layout/view_stub_webview.xml diff --git a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthCaptchaFragment.kt b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthCaptchaFragment.kt index 3720a41455..0562378ba5 100644 --- a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthCaptchaFragment.kt +++ b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthCaptchaFragment.kt @@ -16,15 +16,22 @@ package im.vector.app.features.onboarding.ftueauth +import android.os.Bundle import android.os.Parcelable import android.view.LayoutInflater +import android.view.View import android.view.ViewGroup +import android.view.ViewStub import com.airbnb.mvrx.args +import com.google.android.material.dialog.MaterialAlertDialogBuilder +import im.vector.app.R import im.vector.app.databinding.FragmentFtueLoginCaptchaBinding +import im.vector.app.databinding.ViewStubWebviewBinding import im.vector.app.features.onboarding.OnboardingAction import im.vector.app.features.onboarding.OnboardingViewState import im.vector.app.features.onboarding.RegisterAction import kotlinx.parcelize.Parcelize +import org.matrix.android.sdk.api.extensions.orFalse import javax.inject.Inject @Parcelize @@ -40,10 +47,32 @@ class FtueAuthCaptchaFragment @Inject constructor( ) : AbstractFtueAuthFragment() { private val params: FtueAuthCaptchaFragmentArgument by args() + private var webViewBinding: ViewStubWebviewBinding? = null private var isWebViewLoaded = false override fun getBinding(inflater: LayoutInflater, container: ViewGroup?): FragmentFtueLoginCaptchaBinding { - return FragmentFtueLoginCaptchaBinding.inflate(inflater, container, false) + return FragmentFtueLoginCaptchaBinding.inflate(inflater, container, false).also { + it.loginCaptchaWebViewStub.setOnInflateListener { _, inflated -> + webViewBinding = ViewStubWebviewBinding.bind(inflated) + } + } + } + + override fun onViewCreated(view: View, savedInstanceState: Bundle?) { + super.onViewCreated(view, savedInstanceState) + inflateWebViewOrShowError() + } + + private fun inflateWebViewOrShowError() { + views.loginCaptchaWebViewStub.inflateWebView(onError = { + MaterialAlertDialogBuilder(requireActivity()) + .setTitle(R.string.dialog_title_error) + .setMessage(it.localizedMessage) + .setPositiveButton(R.string.ok) { _, _ -> + requireActivity().recreate() + } + .show() + }) } override fun resetViewModel() { @@ -51,11 +80,38 @@ class FtueAuthCaptchaFragment @Inject constructor( } override fun updateWithState(state: OnboardingViewState) { - if (!isWebViewLoaded) { - captchaWebview.setupWebView(this, views.loginCaptchaWevView, views.loginCaptchaProgress, params.siteKey, state) { + if (!isWebViewLoaded && webViewBinding != null) { + captchaWebview.setupWebView(this, webViewBinding!!.root, views.loginCaptchaProgress, params.siteKey, state) { viewModel.handle(OnboardingAction.PostRegisterAction(RegisterAction.CaptchaDone(it))) } isWebViewLoaded = true } } } + +private fun ViewStub.inflateWebView(onError: (Throwable) -> Unit) { + try { + inflate() + } catch (e: Exception) { + val isMissingWebView = e.crawlCausesFor { it.message?.contains("MissingWebViewPackageException").orFalse() } + if (isMissingWebView) { + onError(MissingWebViewException(e)) + } else { + onError(e) + } + } +} + +private fun Throwable?.crawlCausesFor(predicate: (Throwable) -> Boolean): Boolean { + return when { + this == null -> false + else -> { + when (predicate(this)) { + true -> true + else -> this.cause.crawlCausesFor(predicate) + } + } + } +} + +private class MissingWebViewException(cause: Throwable) : IllegalStateException("Failed to load WebView provider: No WebView installed", cause) diff --git a/vector/src/main/res/layout/fragment_ftue_login_captcha.xml b/vector/src/main/res/layout/fragment_ftue_login_captcha.xml index 2f6970c785..fc93d0f990 100644 --- a/vector/src/main/res/layout/fragment_ftue_login_captcha.xml +++ b/vector/src/main/res/layout/fragment_ftue_login_captcha.xml @@ -64,15 +64,27 @@ android:id="@+id/titleContentSpacing" android:layout_width="match_parent" android:layout_height="0dp" - app:layout_constraintBottom_toTopOf="@id/loginCaptchaWevView" + app:layout_constraintBottom_toTopOf="@id/loginWebViewBarrier" app:layout_constraintHeight_percent="0.03" app:layout_constraintTop_toBottomOf="@id/captchaHeaderTitle" /> - + + + From a1a8ccae3821717b17e4de7fb78fd275ea800583 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Wed, 17 Aug 2022 14:15:35 +0100 Subject: [PATCH 5/8] reverting scope of init only variable --- .../im/vector/app/features/onboarding/OnboardingViewModel.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt b/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt index 6106d84a8d..d5ed20c89c 100644 --- a/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt @@ -71,7 +71,7 @@ import java.util.concurrent.CancellationException * */ class OnboardingViewModel @AssistedInject constructor( - @Assisted private val initialState: OnboardingViewState, + @Assisted initialState: OnboardingViewState, private val applicationContext: Context, private val authenticationService: AuthenticationService, private val activeSessionHolder: ActiveSessionHolder, From cdeea219175d30cd5e2640343080bb05d6509196 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Wed, 17 Aug 2022 14:43:03 +0100 Subject: [PATCH 6/8] adding changelog entries --- changelog.d/6855.bugfix | 1 + changelog.d/6860.bugfix | 1 + changelog.d/6861.bugfix | 1 + 3 files changed, 3 insertions(+) create mode 100644 changelog.d/6855.bugfix create mode 100644 changelog.d/6860.bugfix create mode 100644 changelog.d/6861.bugfix diff --git a/changelog.d/6855.bugfix b/changelog.d/6855.bugfix new file mode 100644 index 0000000000..63a62de986 --- /dev/null +++ b/changelog.d/6855.bugfix @@ -0,0 +1 @@ +Fixes onboarding captcha crashing when no WebView is available by showing an error with information instead diff --git a/changelog.d/6860.bugfix b/changelog.d/6860.bugfix new file mode 100644 index 0000000000..22e287c0b3 --- /dev/null +++ b/changelog.d/6860.bugfix @@ -0,0 +1 @@ +Removes ability to continue registration after the app has been destroyed, fixes the next steps crashing due to missing information from the previous steps diff --git a/changelog.d/6861.bugfix b/changelog.d/6861.bugfix new file mode 100644 index 0000000000..508325acc6 --- /dev/null +++ b/changelog.d/6861.bugfix @@ -0,0 +1 @@ +Fixes crash when exiting the login or registration entry screens whilst they're loading From 8b70f3a3b98bb10f78ad55df7b056ca712aca32f Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Wed, 17 Aug 2022 16:16:44 +0100 Subject: [PATCH 7/8] extracting throwable crawling to extension --- .../vector/app/core/extensions/Throwable.kt | 34 +++++++++++++++++++ .../ftueauth/FtueAuthCaptchaFragment.kt | 13 +------ 2 files changed, 35 insertions(+), 12 deletions(-) create mode 100644 vector/src/main/java/im/vector/app/core/extensions/Throwable.kt diff --git a/vector/src/main/java/im/vector/app/core/extensions/Throwable.kt b/vector/src/main/java/im/vector/app/core/extensions/Throwable.kt new file mode 100644 index 0000000000..e1688124fa --- /dev/null +++ b/vector/src/main/java/im/vector/app/core/extensions/Throwable.kt @@ -0,0 +1,34 @@ +/* + * Copyright (c) 2022 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.core.extensions + +/** + * Recursive through the throwable and its causes for the given predicate + * + * @return true when the predicate finds a match + */ +tailrec fun Throwable?.crawlCausesFor(predicate: (Throwable) -> Boolean): Boolean { + return when { + this == null -> false + else -> { + when (predicate(this)) { + true -> true + else -> this.cause.crawlCausesFor(predicate) + } + } + } +} diff --git a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthCaptchaFragment.kt b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthCaptchaFragment.kt index 0562378ba5..1f44922b3b 100644 --- a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthCaptchaFragment.kt +++ b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthCaptchaFragment.kt @@ -25,6 +25,7 @@ import android.view.ViewStub import com.airbnb.mvrx.args import com.google.android.material.dialog.MaterialAlertDialogBuilder import im.vector.app.R +import im.vector.app.core.extensions.crawlCausesFor import im.vector.app.databinding.FragmentFtueLoginCaptchaBinding import im.vector.app.databinding.ViewStubWebviewBinding import im.vector.app.features.onboarding.OnboardingAction @@ -102,16 +103,4 @@ private fun ViewStub.inflateWebView(onError: (Throwable) -> Unit) { } } -private fun Throwable?.crawlCausesFor(predicate: (Throwable) -> Boolean): Boolean { - return when { - this == null -> false - else -> { - when (predicate(this)) { - true -> true - else -> this.cause.crawlCausesFor(predicate) - } - } - } -} - private class MissingWebViewException(cause: Throwable) : IllegalStateException("Failed to load WebView provider: No WebView installed", cause) From 106fa1b1d5b3be24a340af4c8d7404150ca349f5 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 18 Aug 2022 11:51:23 +0100 Subject: [PATCH 8/8] adding missing fullstops in to docs --- .../src/main/java/im/vector/app/core/extensions/Throwable.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vector/src/main/java/im/vector/app/core/extensions/Throwable.kt b/vector/src/main/java/im/vector/app/core/extensions/Throwable.kt index e1688124fa..0aa9039dcb 100644 --- a/vector/src/main/java/im/vector/app/core/extensions/Throwable.kt +++ b/vector/src/main/java/im/vector/app/core/extensions/Throwable.kt @@ -17,9 +17,9 @@ package im.vector.app.core.extensions /** - * Recursive through the throwable and its causes for the given predicate + * Recursive through the throwable and its causes for the given predicate. * - * @return true when the predicate finds a match + * @return true when the predicate finds a match. */ tailrec fun Throwable?.crawlCausesFor(predicate: (Throwable) -> Boolean): Boolean { return when {