Commit 58de8371 authored by Marcin Koziński's avatar Marcin Koziński Committed by Pier Angelo Vendrame
Browse files

Bug 2040834 - Add an initial delay before enabling positive button in addon...

Bug 2040834 - Add an initial delay before enabling positive button in addon permissions dialog r=android-reviewers,android-addons-reviewers,twhite,robwu

Differential Revision: https://phabricator.services.mozilla.com/D302742
parent fe403f1e
Loading
Loading
Loading
Loading
+40 −9
Original line number Diff line number Diff line
@@ -25,12 +25,20 @@ import androidx.appcompat.widget.AppCompatCheckBox
import androidx.core.content.ContextCompat
import androidx.core.graphics.drawable.toDrawable
import androidx.core.view.isVisible
import androidx.lifecycle.Lifecycle
import androidx.lifecycle.lifecycleScope
import androidx.lifecycle.repeatOnLifecycle
import androidx.recyclerview.widget.LinearLayoutManager
import androidx.recyclerview.widget.RecyclerView
import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.delay
import kotlinx.coroutines.launch
import mozilla.components.feature.addons.Addon
import mozilla.components.feature.addons.R
import mozilla.components.support.base.log.logger.Logger
import mozilla.components.support.utils.ext.getParcelableCompat
import kotlin.time.Duration.Companion.seconds

internal const val KEY_ADDON = "KEY_ADDON"
private const val KEY_DIALOG_GRAVITY = "KEY_DIALOG_GRAVITY"
@@ -45,11 +53,17 @@ private const val KEY_FOR_OPTIONAL_PERMISSIONS = "KEY_FOR_OPTIONAL_PERMISSIONS"
internal const val KEY_PERMISSIONS = "KEY_PERMISSIONS"
internal const val KEY_ORIGINS = "KEY_ORIGINS"
private const val DEFAULT_VALUE = Int.MAX_VALUE
private val POSITIVE_BUTTON_ENABLE_DELAY = 1.seconds

/**
 * A dialog that shows a set of permission required by an [Addon].
 */
class PermissionsDialogFragment : AddonDialogFragment() {
class PermissionsDialogFragment
@JvmOverloads constructor(
    mainDispatcher: CoroutineDispatcher? = null,
) : AddonDialogFragment() {

    private val mainDispatcher = mainDispatcher ?: Dispatchers.Main.immediate

    /**
     * A lambda called when the allow button is clicked which contains the [Addon] and
@@ -134,6 +148,10 @@ class PermissionsDialogFragment : AddonDialogFragment() {
    internal val permissions get() = requireNotNull(safeArguments.getStringArray(KEY_PERMISSIONS))
    internal val origins get() = requireNotNull(safeArguments.getStringArray(KEY_ORIGINS))

    private var initialDelayElapsed = false
    private var userScriptsPermissionOptInMissing = true
    private val shouldEnablePositiveButton: Boolean get() = initialDelayElapsed && !userScriptsPermissionOptInMissing

    override fun onCreateDialog(savedInstanceState: Bundle?): Dialog {
        val sheetDialog = Dialog(requireContext())
        sheetDialog.requestWindowFeature(Window.FEATURE_NO_TITLE)
@@ -247,7 +265,8 @@ class PermissionsDialogFragment : AddonDialogFragment() {
            permissions = listPermissions,
            permissionRequiresOptIn = isUserScriptsPermission,
            onPermissionOptInChanged = { enabled ->
                setButtonEnabled(positiveButton, enabled)
                userScriptsPermissionOptInMissing = !enabled
                setButtonEnabled(positiveButton, shouldEnablePositiveButton)
            },
            domains = displayDomainList,
            domainsHeaderText = requireContext()
@@ -277,12 +296,23 @@ class PermissionsDialogFragment : AddonDialogFragment() {
            dismiss()
        }

        if (isUserScriptsPermission) {
        // "userScripts" permission requires double-confirmation.
            // Disable "Allow" button until the user confirmed via opt-in.
            setButtonEnabled(positiveButton, false)
        } else {
            setButtonEnabled(positiveButton, true)
        // Disable "Add" button until the user confirmed via opt-in.
        userScriptsPermissionOptInMissing = isUserScriptsPermission
        setButtonEnabled(positiveButton, shouldEnablePositiveButton)

        // Disable "Add" button until an initial delay elapses
        // when dialog is first created and any time it goes out of foreground.
        lifecycleScope.launch(mainDispatcher) {
            repeatOnLifecycle(Lifecycle.State.RESUMED) {
                initialDelayElapsed = false
                setButtonEnabled(positiveButton, shouldEnablePositiveButton)

                delay(POSITIVE_BUTTON_ENABLE_DELAY)

                initialDelayElapsed = true
                setButtonEnabled(positiveButton, shouldEnablePositiveButton)
            }
        }

        negativeButton.setOnClickListener {
@@ -419,8 +449,9 @@ class PermissionsDialogFragment : AddonDialogFragment() {
            onPositiveButtonClicked: ((Addon, Boolean) -> Unit)? = null,
            onNegativeButtonClicked: (() -> Unit)? = null,
            onLearnMoreClicked: (() -> Unit)? = null,
            mainDispatcher: CoroutineDispatcher? = null,
        ): PermissionsDialogFragment {
            val fragment = PermissionsDialogFragment()
            val fragment = PermissionsDialogFragment(mainDispatcher)
            val arguments = fragment.arguments ?: Bundle()

            arguments.apply {
+103 −1
Original line number Diff line number Diff line
@@ -12,11 +12,17 @@ import androidx.appcompat.widget.AppCompatCheckBox
import androidx.core.view.isVisible
import androidx.fragment.app.FragmentManager
import androidx.fragment.app.FragmentTransaction
import androidx.lifecycle.Lifecycle
import androidx.lifecycle.LifecycleRegistry
import androidx.recyclerview.widget.RecyclerView
import androidx.test.ext.junit.runners.AndroidJUnit4
import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.test.StandardTestDispatcher
import kotlinx.coroutines.test.runTest
import mozilla.components.feature.addons.Addon
import mozilla.components.feature.addons.R
import mozilla.components.feature.addons.ui.AddonDialogFragment.PromptsStyling
import mozilla.components.support.test.any
import mozilla.components.support.test.mock
import mozilla.components.support.test.robolectric.testContext
import mozilla.components.support.utils.ext.getParcelableCompat
@@ -27,6 +33,7 @@ import org.junit.Assert.assertSame
import org.junit.Assert.assertTrue
import org.junit.Test
import org.junit.runner.RunWith
import org.mockito.Mockito.doAnswer
import org.mockito.Mockito.doNothing
import org.mockito.Mockito.doReturn
import org.mockito.Mockito.spy
@@ -153,6 +160,44 @@ class PermissionsDialogFragmentTest {
        assertTrue(learnMoreWasExecuted)
    }

    @Test
    fun `allow button is disabled immediately after the dialog is shown`() = runTest {
        val addon = Addon("id", translatableName = mapOf(Addon.DEFAULT_LOCALE to "my_addon"))
        val fragment = createPermissionsDialogFragment(
            addon,
            dispatcher = StandardTestDispatcher(testScheduler),
        )

        doReturn(testContext).`when`(fragment).requireContext()

        val dialog = fragment.onCreateDialog(null)
        dialog.show()

        val positiveButton = dialog.findViewById<Button>(R.id.allow_button)
        assertFalse(positiveButton.isEnabled)
    }

    @Test
    fun `allow button becomes enabled after the initial delay`() = runTest {
        val addon = Addon("id", translatableName = mapOf(Addon.DEFAULT_LOCALE to "my_addon"))
        val fragment = createPermissionsDialogFragment(
            addon,
            dispatcher = StandardTestDispatcher(testScheduler),
        )

        doReturn(testContext).`when`(fragment).requireContext()

        val dialog = fragment.onCreateDialog(null)
        dialog.show()

        val positiveButton = dialog.findViewById<Button>(R.id.allow_button)
        assertFalse(positiveButton.isEnabled)

        testScheduler.advanceUntilIdle()

        assertTrue(positiveButton.isEnabled)
    }

    @Test
    fun `dismissing the dialog notifies deny lambda`() {
        val addon = Addon("id", translatableName = mapOf(Addon.DEFAULT_LOCALE to "my_addon"))
@@ -709,7 +754,7 @@ class PermissionsDialogFragmentTest {
    }

    @Test
    fun `require double confirmation for userScripts optional permission`() {
    fun `require double confirmation for userScripts optional permission`() = runTest {
        // Most of the "userScripts" optional permission request is rendered as an optional permission,
        // which is already covered by the "build dialog for optional permissions" test above.
        // Here, we check aspects specific to the userScripts permission.
@@ -730,6 +775,7 @@ class PermissionsDialogFragmentTest {
            // https://searchfox.org/mozilla-central/rev/fcfb558f8946f3648d962576125af46bf6e2910a/toolkit/components/extensions/test/xpcshell/test_ext_permissions_optional_only.js#251-268
            permissions = listOf("userScripts"),
            origins = emptyList(),
            dispatcher = StandardTestDispatcher(testScheduler),
        )

        doReturn(testContext).`when`(fragment).requireContext()
@@ -768,6 +814,11 @@ class PermissionsDialogFragmentTest {
        assertFalse(allowButton.isEnabled)
        assertTrue(denyButton.isEnabled)

        // Even after the initial delay elapses, the "allow" button must remain
        // disabled until the opt-in checkbox is checked.
        testScheduler.advanceUntilIdle()
        assertFalse(allowButton.isEnabled)

        // Toggling checkbox should enable "allow" button.
        permissionOptInCheckbox.performClick()
        assertTrue(permissionOptInCheckbox.isChecked)
@@ -795,6 +846,46 @@ class PermissionsDialogFragmentTest {
        )
    }

    @Test
    fun `userScripts allow button stays disabled if opt-in is checked before the initial delay elapses`() = runTest {
        val addon = Addon(
            "id",
            translatableName = mapOf(Addon.DEFAULT_LOCALE to "my_addon"),
            optionalPermissions = listOf(Addon.Permission("userScripts", false)),
        )
        val fragment = createPermissionsDialogFragment(
            addon,
            forOptionalPermissions = true,
            permissions = listOf("userScripts"),
            dispatcher = StandardTestDispatcher(testScheduler),
        )

        doReturn(testContext).`when`(fragment).requireContext()
        val dialog = fragment.onCreateDialog(null)
        dialog.show()

        val permissionsRecyclerView = dialog.findViewById<RecyclerView>(R.id.permissions)
        val recyclerAdapter = permissionsRecyclerView.adapter!! as RequiredPermissionsAdapter
        val allowButton = dialog.findViewById<Button>(R.id.allow_button)

        val holder = recyclerAdapter.onCreateViewHolder(permissionsRecyclerView, 3)
        assertIs<RequiredPermissionsAdapter.OptInPermissionViewHolder>(holder)
        recyclerAdapter.onBindViewHolder(holder, 0)
        val permissionOptInCheckbox = holder.itemView.findViewById<AppCompatCheckBox>(R.id.permission_opt_in_item)

        // Initial state: both gates closed; button disabled.
        assertFalse(allowButton.isEnabled)

        // Satisfy the opt-in gate before the delay elapses. Button must stay disabled.
        permissionOptInCheckbox.performClick()
        assertTrue(permissionOptInCheckbox.isChecked)
        assertFalse(allowButton.isEnabled)

        // Once the delay also elapses, both gates are satisfied and the button enables.
        testScheduler.advanceUntilIdle()
        assertTrue(allowButton.isEnabled)
    }

    @Test
    fun `hide private browsing checkbox when the add-on does not allow running in private windows`() {
        val permissions = listOf("privacy", "<all_urls>", "tabs")
@@ -860,6 +951,7 @@ class PermissionsDialogFragmentTest {
        origins: List<String>,
        promptsStyling: PromptsStyling? = null,
        forOptionalPermissions: Boolean = false,
        dispatcher: CoroutineDispatcher? = null,
    ): PermissionsDialogFragment {
        return spy(
            PermissionsDialogFragment.newInstance(
@@ -868,9 +960,19 @@ class PermissionsDialogFragmentTest {
                origins = origins,
                promptsStyling = promptsStyling,
                forOptionalPermissions = forOptionalPermissions,
                mainDispatcher = dispatcher,
            ),
        ).apply {
            doNothing().`when`(this).dismiss()

            val lifecycle = LifecycleRegistry(this)
            doReturn(lifecycle).`when`(this).lifecycle
            doAnswer { invocation ->
                val dialog = invocation.callRealMethod()
                lifecycle.handleLifecycleEvent(Lifecycle.Event.ON_RESUME)
                dialog
            }
                .`when`(this).onCreateDialog(any())
        }
    }