From e37e0bb47656d5fc992bfe07e0bbe9700df1a196 Mon Sep 17 00:00:00 2001 From: Cristian Monforte Date: Thu, 7 May 2020 09:12:44 +0200 Subject: [PATCH 01/11] (wip) receive event isAboutToLoad --- .../com/duckduckgo/app/browser/BrowserTabFragment.kt | 1 + .../com/duckduckgo/app/browser/BrowserTabViewModel.kt | 9 +++++++++ .../com/duckduckgo/app/browser/BrowserWebViewClient.kt | 3 ++- .../com/duckduckgo/app/browser/WebViewClientListener.kt | 1 + 4 files changed, 13 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt index f30c1a232129..664694883dc4 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt @@ -638,6 +638,7 @@ class BrowserTabFragment : Fragment(), FindListener, CoroutineScope, DaxDialogLi is Command.ShowErrorWithAction -> showErrorSnackbar(it) is Command.DaxCommand.FinishTrackerAnimation -> finishTrackerAnimation() is Command.DaxCommand.HideDaxDialog -> showHideTipsDialog(it.cta) + is Command.Blank -> webView?.hide() } } diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt index e4e0735dd60c..d9e89314bc7b 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt @@ -194,6 +194,8 @@ class BrowserTabViewModel( class SaveCredentials(val request: BasicAuthenticationRequest, val credentials: BasicAuthenticationCredentials) : Command() object GenerateWebViewPreviewImage : Command() object LaunchTabSwitcher : Command() + object Blank : Command() + class ShowErrorWithAction(val action: () -> Unit) : Command() sealed class DaxCommand : Command() { object FinishTrackerAnimation : DaxCommand() @@ -1014,7 +1016,14 @@ class BrowserTabViewModel( showErrorWithAction() } + override fun onPageIsAboutToLoad(url: String) { + omnibarViewState.value = currentOmnibarViewState().copy(omnibarText = url) + } + override fun requiresAuthentication(request: BasicAuthenticationRequest) { + if (request.host != site?.uri?.host) { + command.value = Blank + } command.value = RequiresAuthentication(request) } diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt index 005fdf1919d0..9b5233afad77 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt @@ -99,7 +99,8 @@ class BrowserWebViewClient( webView.loadUrl(newUri.toString()) return true } - false + webViewClientListener?.onPageIsAboutToLoad(url.toString()) + return false } } } catch (e: Throwable) { diff --git a/app/src/main/java/com/duckduckgo/app/browser/WebViewClientListener.kt b/app/src/main/java/com/duckduckgo/app/browser/WebViewClientListener.kt index 750c9323503c..112f3c7f2e37 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/WebViewClientListener.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/WebViewClientListener.kt @@ -30,6 +30,7 @@ interface WebViewClientListener { fun navigationStateChanged(newWebNavigationState: WebNavigationState) fun pageRefreshed(refreshedUrl: String) fun progressChanged(newProgress: Int) + fun onPageIsAboutToLoad(url: String) fun titleReceived(newTitle: String) fun trackerDetected(event: TrackingEvent) From 9aad0d7b352e2cf5f06080fd246b5b2b805e85a5 Mon Sep 17 00:00:00 2001 From: Cristian Monforte Date: Thu, 7 May 2020 17:17:36 +0200 Subject: [PATCH 02/11] revert introducing new method on listener --- .../java/com/duckduckgo/app/browser/BrowserTabViewModel.kt | 5 +---- .../java/com/duckduckgo/app/browser/BrowserWebViewClient.kt | 3 +-- .../java/com/duckduckgo/app/browser/WebViewClientListener.kt | 1 - 3 files changed, 2 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt index d9e89314bc7b..9a66b768ede1 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt @@ -1016,12 +1016,9 @@ class BrowserTabViewModel( showErrorWithAction() } - override fun onPageIsAboutToLoad(url: String) { - omnibarViewState.value = currentOmnibarViewState().copy(omnibarText = url) - } - override fun requiresAuthentication(request: BasicAuthenticationRequest) { if (request.host != site?.uri?.host) { + omnibarViewState.value = currentOmnibarViewState().copy(omnibarText = request.site) command.value = Blank } command.value = RequiresAuthentication(request) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt index 9b5233afad77..005fdf1919d0 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt @@ -99,8 +99,7 @@ class BrowserWebViewClient( webView.loadUrl(newUri.toString()) return true } - webViewClientListener?.onPageIsAboutToLoad(url.toString()) - return false + false } } } catch (e: Throwable) { diff --git a/app/src/main/java/com/duckduckgo/app/browser/WebViewClientListener.kt b/app/src/main/java/com/duckduckgo/app/browser/WebViewClientListener.kt index 112f3c7f2e37..750c9323503c 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/WebViewClientListener.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/WebViewClientListener.kt @@ -30,7 +30,6 @@ interface WebViewClientListener { fun navigationStateChanged(newWebNavigationState: WebNavigationState) fun pageRefreshed(refreshedUrl: String) fun progressChanged(newProgress: Int) - fun onPageIsAboutToLoad(url: String) fun titleReceived(newTitle: String) fun trackerDetected(event: TrackingEvent) From d5ba5309b1431bddb253da96e1412e66c73da1ec Mon Sep 17 00:00:00 2001 From: Cristian Monforte Date: Thu, 7 May 2020 17:33:11 +0200 Subject: [PATCH 03/11] show/hide background webview while showing auth popup --- .../java/com/duckduckgo/app/browser/BrowserTabFragment.kt | 3 ++- .../com/duckduckgo/app/browser/BrowserTabViewModel.kt | 8 +++++--- .../app/browser/ui/HttpAuthenticationDialogFragment.kt | 5 +++++ 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt index 664694883dc4..7abd52ed031a 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt @@ -638,7 +638,8 @@ class BrowserTabFragment : Fragment(), FindListener, CoroutineScope, DaxDialogLi is Command.ShowErrorWithAction -> showErrorSnackbar(it) is Command.DaxCommand.FinishTrackerAnimation -> finishTrackerAnimation() is Command.DaxCommand.HideDaxDialog -> showHideTipsDialog(it.cta) - is Command.Blank -> webView?.hide() + is Command.HideBrowser -> webView?.hide() + is Command.ShowBrowser -> webView?.show() } } diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt index 9a66b768ede1..988f5b1648ab 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt @@ -77,7 +77,6 @@ import com.jakewharton.rxrelay2.PublishRelay import io.reactivex.android.schedulers.AndroidSchedulers import io.reactivex.disposables.Disposable import io.reactivex.schedulers.Schedulers -import kotlinx.android.synthetic.main.include_omnibar_toolbar.* import kotlinx.coroutines.Job import kotlinx.coroutines.launch import kotlinx.coroutines.withContext @@ -194,7 +193,8 @@ class BrowserTabViewModel( class SaveCredentials(val request: BasicAuthenticationRequest, val credentials: BasicAuthenticationCredentials) : Command() object GenerateWebViewPreviewImage : Command() object LaunchTabSwitcher : Command() - object Blank : Command() + object HideBrowser : Command() + object ShowBrowser : Command() class ShowErrorWithAction(val action: () -> Unit) : Command() sealed class DaxCommand : Command() { @@ -1019,17 +1019,19 @@ class BrowserTabViewModel( override fun requiresAuthentication(request: BasicAuthenticationRequest) { if (request.host != site?.uri?.host) { omnibarViewState.value = currentOmnibarViewState().copy(omnibarText = request.site) - command.value = Blank + command.value = HideBrowser } command.value = RequiresAuthentication(request) } override fun handleAuthentication(request: BasicAuthenticationRequest, credentials: BasicAuthenticationCredentials) { request.handler.proceed(credentials.username, credentials.password) + command.value = ShowBrowser command.value = SaveCredentials(request, credentials) } override fun cancelAuthentication(request: BasicAuthenticationRequest) { + command.value = ShowBrowser request.handler.cancel() } diff --git a/app/src/main/java/com/duckduckgo/app/browser/ui/HttpAuthenticationDialogFragment.kt b/app/src/main/java/com/duckduckgo/app/browser/ui/HttpAuthenticationDialogFragment.kt index e78f48008fe7..98c1f43086d2 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/ui/HttpAuthenticationDialogFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/ui/HttpAuthenticationDialogFragment.kt @@ -18,6 +18,7 @@ package com.duckduckgo.app.browser.ui import androidx.appcompat.app.AlertDialog import android.app.Dialog +import android.content.DialogInterface import android.os.Bundle import android.view.View import android.view.WindowManager @@ -71,7 +72,11 @@ class HttpAuthenticationDialogFragment : DialogFragment() { showKeyboard(usernameInput, alert) return alert + } + override fun onDismiss(dialog: DialogInterface) { + super.onDismiss(dialog) + listener?.cancelAuthentication(request) } private fun validateBundleArguments() { From 9d0603e5496acaa8f5a01887e3e38e0e332dd55b Mon Sep 17 00:00:00 2001 From: Cristian Monforte Date: Thu, 7 May 2020 22:58:07 +0200 Subject: [PATCH 04/11] dismiss dialog when sending to background --- .../java/com/duckduckgo/app/browser/BrowserTabFragment.kt | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt index 7abd52ed031a..dc8272306717 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt @@ -446,9 +446,17 @@ class BrowserTabFragment : Fragment(), FindListener, CoroutineScope, DaxDialogLi override fun onPause() { logoHidingListener.onPause() dismissDownloadFragment() + dismissAuthenticationDialog() super.onPause() } + private fun dismissAuthenticationDialog() { + if (isAdded) { + val fragment = parentFragmentManager.findFragmentByTag(AUTHENTICATION_DIALOG_TAG) as? HttpAuthenticationDialogFragment + fragment?.dismiss() + } + } + private fun dismissDownloadFragment() { val fragment = fragmentManager?.findFragmentByTag(DOWNLOAD_CONFIRMATION_TAG) as? DownloadConfirmationFragment fragment?.dismiss() From 57ab9b5d98d6fd3644439e9206bfbb21996a5c54 Mon Sep 17 00:00:00 2001 From: Cristian Monforte Date: Thu, 7 May 2020 23:06:23 +0200 Subject: [PATCH 05/11] rename commands --- .../com/duckduckgo/app/browser/BrowserTabFragment.kt | 4 ++-- .../com/duckduckgo/app/browser/BrowserTabViewModel.kt | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt index dc8272306717..61abd0d02943 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt @@ -646,8 +646,8 @@ class BrowserTabFragment : Fragment(), FindListener, CoroutineScope, DaxDialogLi is Command.ShowErrorWithAction -> showErrorSnackbar(it) is Command.DaxCommand.FinishTrackerAnimation -> finishTrackerAnimation() is Command.DaxCommand.HideDaxDialog -> showHideTipsDialog(it.cta) - is Command.HideBrowser -> webView?.hide() - is Command.ShowBrowser -> webView?.show() + is Command.HideWebView -> webView?.hide() + is Command.ShowWebView -> webView?.show() } } diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt index 988f5b1648ab..ffe412d096fc 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt @@ -193,8 +193,8 @@ class BrowserTabViewModel( class SaveCredentials(val request: BasicAuthenticationRequest, val credentials: BasicAuthenticationCredentials) : Command() object GenerateWebViewPreviewImage : Command() object LaunchTabSwitcher : Command() - object HideBrowser : Command() - object ShowBrowser : Command() + object HideWebView : Command() + object ShowWebView : Command() class ShowErrorWithAction(val action: () -> Unit) : Command() sealed class DaxCommand : Command() { @@ -1019,19 +1019,19 @@ class BrowserTabViewModel( override fun requiresAuthentication(request: BasicAuthenticationRequest) { if (request.host != site?.uri?.host) { omnibarViewState.value = currentOmnibarViewState().copy(omnibarText = request.site) - command.value = HideBrowser + command.value = HideWebView } command.value = RequiresAuthentication(request) } override fun handleAuthentication(request: BasicAuthenticationRequest, credentials: BasicAuthenticationCredentials) { request.handler.proceed(credentials.username, credentials.password) - command.value = ShowBrowser + command.value = ShowWebView command.value = SaveCredentials(request, credentials) } override fun cancelAuthentication(request: BasicAuthenticationRequest) { - command.value = ShowBrowser + command.value = ShowWebView request.handler.cancel() } From e107e657ad251b40b1c1b98c1b7acc61cb88b2fd Mon Sep 17 00:00:00 2001 From: Cristian Monforte Date: Thu, 7 May 2020 23:15:26 +0200 Subject: [PATCH 06/11] moving to Hide/Show WebContent. --- .../com/duckduckgo/app/browser/BrowserTabFragment.kt | 4 ++-- .../com/duckduckgo/app/browser/BrowserTabViewModel.kt | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt index 61abd0d02943..0d7da1027094 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt @@ -646,8 +646,8 @@ class BrowserTabFragment : Fragment(), FindListener, CoroutineScope, DaxDialogLi is Command.ShowErrorWithAction -> showErrorSnackbar(it) is Command.DaxCommand.FinishTrackerAnimation -> finishTrackerAnimation() is Command.DaxCommand.HideDaxDialog -> showHideTipsDialog(it.cta) - is Command.HideWebView -> webView?.hide() - is Command.ShowWebView -> webView?.show() + is Command.HideWebContent -> webView?.hide() + is Command.ShowWebContent -> webView?.show() } } diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt index ffe412d096fc..e263258be0b2 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt @@ -193,8 +193,8 @@ class BrowserTabViewModel( class SaveCredentials(val request: BasicAuthenticationRequest, val credentials: BasicAuthenticationCredentials) : Command() object GenerateWebViewPreviewImage : Command() object LaunchTabSwitcher : Command() - object HideWebView : Command() - object ShowWebView : Command() + object HideWebContent : Command() + object ShowWebContent : Command() class ShowErrorWithAction(val action: () -> Unit) : Command() sealed class DaxCommand : Command() { @@ -1019,19 +1019,19 @@ class BrowserTabViewModel( override fun requiresAuthentication(request: BasicAuthenticationRequest) { if (request.host != site?.uri?.host) { omnibarViewState.value = currentOmnibarViewState().copy(omnibarText = request.site) - command.value = HideWebView + command.value = HideWebContent } command.value = RequiresAuthentication(request) } override fun handleAuthentication(request: BasicAuthenticationRequest, credentials: BasicAuthenticationCredentials) { request.handler.proceed(credentials.username, credentials.password) - command.value = ShowWebView + command.value = ShowWebContent command.value = SaveCredentials(request, credentials) } override fun cancelAuthentication(request: BasicAuthenticationRequest) { - command.value = ShowWebView + command.value = ShowWebContent request.handler.cancel() } From 2d2a20fa5f3656ac6ff1add21aaa21c28a2b8840 Mon Sep 17 00:00:00 2001 From: Cristian Monforte Date: Thu, 7 May 2020 23:39:10 +0200 Subject: [PATCH 07/11] test cases for updating url and hiding web content when authentication requested --- .../app/browser/BrowserTabViewModelTest.kt | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt index e35704dc2e56..f66aa883114b 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt @@ -1243,6 +1243,31 @@ class BrowserTabViewModelTest { assertSame(authenticationRequest, requiresAuthCommand.request) } + @Test + fun whenAuthenticationIsRequiredForSameHostThenNoChangesOnBrowser() { + val mockHandler = mock() + val siteURL = "http://example.com/requires-auth" + val authenticationRequest = BasicAuthenticationRequest(mockHandler, "example.com", "test realm", siteURL) + + loadUrl(url = "http://example.com", isBrowserShowing = true) + testee.requiresAuthentication(authenticationRequest) + + assertEquals("http://example.com", omnibarViewState().omnibarText) + } + + @Test + fun whenAuthenticationIsRequiredForDifferentHostThenUpdateUrlAndHideWebContent() { + val mockHandler = mock() + val siteURL = "http://example.com/requires-auth" + val authenticationRequest = BasicAuthenticationRequest(mockHandler, "example.com", "test realm", siteURL) + + loadUrl(url = "http://another.website.com", isBrowserShowing = true) + testee.requiresAuthentication(authenticationRequest) + + assertCommandIssued() + assertEquals(siteURL, omnibarViewState().omnibarText) + } + @Test fun whenHandleAuthenticationThenHandlerCalledWithParameters() { val mockHandler = mock() From 54081d3d90f39902bf1adc8d57316b6d538f5de7 Mon Sep 17 00:00:00 2001 From: Cristian Monforte Date: Fri, 8 May 2020 00:19:46 +0200 Subject: [PATCH 08/11] test case: showing web content after dialog dismissed --- .../app/browser/BrowserTabViewModelTest.kt | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt index f66aa883114b..f1529d6c4155 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt @@ -1280,6 +1280,25 @@ class BrowserTabViewModelTest { verify(mockHandler, atLeastOnce()).proceed(username, password) } + @Test + fun whenAuthenticationDialogAcceptedThenShowWebContent() { + val authenticationRequest = BasicAuthenticationRequest(mock(), "example.com", "test realm", "") + val credentials = BasicAuthenticationCredentials(username = "user", password = "password") + + testee.handleAuthentication(request = authenticationRequest, credentials = credentials) + + assertCommandIssued() + } + + @Test + fun whenAuthenticationDialogCanceledThenShowWebContent() { + val authenticationRequest = BasicAuthenticationRequest(mock(), "example.com", "test realm", "") + + testee.cancelAuthentication(request = authenticationRequest) + + assertCommandIssued() + } + @Test fun whenBookmarkSuggestionSubmittedThenAutoCompleteBookmarkSelectionPixelSent() = runBlocking { whenever(mockBookmarksDao.hasBookmarks()).thenReturn(true) From 600c8137401f449f43bcbf665e24b4ff47fbd54c Mon Sep 17 00:00:00 2001 From: Cristian Monforte Date: Fri, 8 May 2020 08:48:11 +0200 Subject: [PATCH 09/11] notify HttpAuthenticationListener only once from dialog. If dialog dismissed by activity, cancel authentication --- .../java/com/duckduckgo/app/browser/BrowserTabViewModel.kt | 2 +- .../app/browser/ui/HttpAuthenticationDialogFragment.kt | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt index e263258be0b2..d9fb1c8b8760 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt @@ -1031,8 +1031,8 @@ class BrowserTabViewModel( } override fun cancelAuthentication(request: BasicAuthenticationRequest) { - command.value = ShowWebContent request.handler.cancel() + command.value = ShowWebContent } fun userLaunchingTabSwitcher() { diff --git a/app/src/main/java/com/duckduckgo/app/browser/ui/HttpAuthenticationDialogFragment.kt b/app/src/main/java/com/duckduckgo/app/browser/ui/HttpAuthenticationDialogFragment.kt index 98c1f43086d2..1883c0bd90a9 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/ui/HttpAuthenticationDialogFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/ui/HttpAuthenticationDialogFragment.kt @@ -16,7 +16,6 @@ package com.duckduckgo.app.browser.ui -import androidx.appcompat.app.AlertDialog import android.app.Dialog import android.content.DialogInterface import android.os.Bundle @@ -24,6 +23,7 @@ import android.view.View import android.view.WindowManager import android.widget.EditText import android.widget.TextView +import androidx.appcompat.app.AlertDialog import androidx.fragment.app.DialogFragment import com.duckduckgo.app.browser.R import com.duckduckgo.app.browser.model.BasicAuthenticationCredentials @@ -61,21 +61,22 @@ class HttpAuthenticationDialogFragment : DialogFragment() { request, BasicAuthenticationCredentials(username = usernameInput.text.toString(), password = passwordInput.text.toString()) ) - + listener = null }.setNegativeButton(R.string.authenticationDialogNegativeButton) { _, _ -> rootView.hideKeyboard() listener?.cancelAuthentication(request) + listener = null } .setTitle(R.string.authenticationDialogTitle) val alert = alertBuilder.create() showKeyboard(usernameInput, alert) - return alert } override fun onDismiss(dialog: DialogInterface) { super.onDismiss(dialog) + //if listener is not null, user didn't press any button. We proceed to cancel. listener?.cancelAuthentication(request) } From b7078a1cf086c9cc33af16a553b60dd401e1cef6 Mon Sep 17 00:00:00 2001 From: Cristian Monforte Date: Thu, 14 May 2020 17:39:38 +0200 Subject: [PATCH 10/11] ensure command not issued --- .../com/duckduckgo/app/browser/BrowserTabViewModelTest.kt | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt index f1529d6c4155..98a48e156f4f 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt @@ -1252,6 +1252,7 @@ class BrowserTabViewModelTest { loadUrl(url = "http://example.com", isBrowserShowing = true) testee.requiresAuthentication(authenticationRequest) + assertCommandNotIssued() assertEquals("http://example.com", omnibarViewState().omnibarText) } @@ -1732,6 +1733,11 @@ class BrowserTabViewModelTest { (issuedCommand as T).apply { instanceAssertions() } } + private inline fun assertCommandNotIssued() { + val issuedCommand = commandCaptor.allValues.find { it is T } + assertNull(issuedCommand) + } + private fun pixelParams(showedBookmarks: Boolean, bookmarkCapable: Boolean) = mapOf( Pixel.PixelParameter.SHOWED_BOOKMARKS to showedBookmarks.toString(), Pixel.PixelParameter.BOOKMARK_CAPABLE to bookmarkCapable.toString() From fe156d6e0911ee54db1c1cdd4e20ff25aac71763 Mon Sep 17 00:00:00 2001 From: Cristian Monforte Date: Thu, 14 May 2020 17:40:11 +0200 Subject: [PATCH 11/11] replace comment for self documenting class property --- .../browser/ui/HttpAuthenticationDialogFragment.kt | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/ui/HttpAuthenticationDialogFragment.kt b/app/src/main/java/com/duckduckgo/app/browser/ui/HttpAuthenticationDialogFragment.kt index 1883c0bd90a9..816e87cf5fa9 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/ui/HttpAuthenticationDialogFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/ui/HttpAuthenticationDialogFragment.kt @@ -34,8 +34,9 @@ import org.jetbrains.anko.find class HttpAuthenticationDialogFragment : DialogFragment() { - var listener: HttpAuthenticationListener? = null + private var didUserCompleteAuthentication: Boolean = false lateinit var request: BasicAuthenticationRequest + var listener: HttpAuthenticationListener? = null interface HttpAuthenticationListener { fun handleAuthentication(request: BasicAuthenticationRequest, credentials: BasicAuthenticationCredentials) @@ -61,11 +62,11 @@ class HttpAuthenticationDialogFragment : DialogFragment() { request, BasicAuthenticationCredentials(username = usernameInput.text.toString(), password = passwordInput.text.toString()) ) - listener = null + didUserCompleteAuthentication = true }.setNegativeButton(R.string.authenticationDialogNegativeButton) { _, _ -> rootView.hideKeyboard() listener?.cancelAuthentication(request) - listener = null + didUserCompleteAuthentication = true } .setTitle(R.string.authenticationDialogTitle) @@ -76,8 +77,9 @@ class HttpAuthenticationDialogFragment : DialogFragment() { override fun onDismiss(dialog: DialogInterface) { super.onDismiss(dialog) - //if listener is not null, user didn't press any button. We proceed to cancel. - listener?.cancelAuthentication(request) + if (!didUserCompleteAuthentication) { + listener?.cancelAuthentication(request) + } } private fun validateBundleArguments() {