Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -1284,6 +1284,32 @@ class BrowserTabViewModelTest {
assertSame(authenticationRequest, requiresAuthCommand.request)
}

@Test
fun whenAuthenticationIsRequiredForSameHostThenNoChangesOnBrowser() {
val mockHandler = mock<HttpAuthHandler>()
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)

assertCommandNotIssued<Command.HideWebContent>()
assertEquals("http://example.com", omnibarViewState().omnibarText)
Copy link
Contributor

Choose a reason for hiding this comment

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

Since the opposite test (the next one) checks that that the HideWebViewContent command is sent does it make sense to check that the HideWebViewContent command is not sent here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good 👍

}

@Test
fun whenAuthenticationIsRequiredForDifferentHostThenUpdateUrlAndHideWebContent() {
val mockHandler = mock<HttpAuthHandler>()
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<Command.HideWebContent>()
assertEquals(siteURL, omnibarViewState().omnibarText)
}

@Test
fun whenHandleAuthenticationThenHandlerCalledWithParameters() {
val mockHandler = mock<HttpAuthHandler>()
Expand All @@ -1296,6 +1322,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<Command.ShowWebContent>()
}

@Test
fun whenAuthenticationDialogCanceledThenShowWebContent() {
val authenticationRequest = BasicAuthenticationRequest(mock(), "example.com", "test realm", "")

testee.cancelAuthentication(request = authenticationRequest)

assertCommandIssued<Command.ShowWebContent>()
}

@Test
fun whenBookmarkSuggestionSubmittedThenAutoCompleteBookmarkSelectionPixelSent() = runBlocking {
whenever(mockBookmarksDao.hasBookmarks()).thenReturn(true)
Expand Down Expand Up @@ -1718,6 +1763,11 @@ class BrowserTabViewModelTest {
(issuedCommand as T).apply { instanceAssertions() }
}

private inline fun <reified T : Command> 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()
Expand Down
10 changes: 10 additions & 0 deletions app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt
Original file line number Diff line number Diff line change
Expand Up @@ -374,9 +374,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()
Expand Down Expand Up @@ -567,6 +575,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.HideWebContent -> webView?.hide()
is Command.ShowWebContent -> webView?.show()
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,9 @@ class BrowserTabViewModel(
class SaveCredentials(val request: BasicAuthenticationRequest, val credentials: BasicAuthenticationCredentials) : Command()
object GenerateWebViewPreviewImage : Command()
object LaunchTabSwitcher : Command()
object HideWebContent : Command()
object ShowWebContent : Command()

class ShowErrorWithAction(val action: () -> Unit) : Command()
sealed class DaxCommand : Command() {
object FinishTrackerAnimation : DaxCommand()
Expand Down Expand Up @@ -1114,16 +1117,22 @@ class BrowserTabViewModel(
}

override fun requiresAuthentication(request: BasicAuthenticationRequest) {
if (request.host != site?.uri?.host) {
omnibarViewState.value = currentOmnibarViewState().copy(omnibarText = request.site)
command.value = HideWebContent
}
command.value = RequiresAuthentication(request)
}

override fun handleAuthentication(request: BasicAuthenticationRequest, credentials: BasicAuthenticationCredentials) {
request.handler.proceed(credentials.username, credentials.password)
command.value = ShowWebContent
command.value = SaveCredentials(request, credentials)
}

override fun cancelAuthentication(request: BasicAuthenticationRequest) {
request.handler.cancel()
command.value = ShowWebContent
}

fun userLaunchingTabSwitcher() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,14 @@

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
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
Expand All @@ -33,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)
Expand All @@ -60,18 +62,24 @@ class HttpAuthenticationDialogFragment : DialogFragment() {
request,
BasicAuthenticationCredentials(username = usernameInput.text.toString(), password = passwordInput.text.toString())
)

didUserCompleteAuthentication = true
}.setNegativeButton(R.string.authenticationDialogNegativeButton) { _, _ ->
rootView.hideKeyboard()
listener?.cancelAuthentication(request)
didUserCompleteAuthentication = true
}
.setTitle(R.string.authenticationDialogTitle)

val alert = alertBuilder.create()
showKeyboard(usernameInput, alert)

return alert
}

override fun onDismiss(dialog: DialogInterface) {
super.onDismiss(dialog)
if (!didUserCompleteAuthentication) {
listener?.cancelAuthentication(request)
}
}

private fun validateBundleArguments() {
Expand Down