diff --git a/app/src/main/java/org/mozilla/fenix/extension/WebExtensionPromptFeature.kt b/app/src/main/java/org/mozilla/fenix/extension/WebExtensionPromptFeature.kt index a336041ed9..486c2e9436 100644 --- a/app/src/main/java/org/mozilla/fenix/extension/WebExtensionPromptFeature.kt +++ b/app/src/main/java/org/mozilla/fenix/extension/WebExtensionPromptFeature.kt @@ -77,11 +77,11 @@ class WebExtensionPromptFeature( // opens the add-ons manager. val addon = Addon.newFromWebExtension(promptRequest.extension) when (promptRequest) { - is WebExtensionPromptRequest.AfterInstallation.Permissions -> handlePermissionRequest( + is WebExtensionPromptRequest.AfterInstallation.Permissions.Required -> handleRequiredPermissionRequest( addon, promptRequest, ) - is WebExtensionPromptRequest.AfterInstallation.OptionalPermissions -> handleOptionalPermissionsRequest( + is WebExtensionPromptRequest.AfterInstallation.Permissions.Optional -> handleOptionalPermissionsRequest( addon, promptRequest, ) @@ -108,36 +108,31 @@ class WebExtensionPromptFeature( showPostInstallationDialog(addon) } - private fun handlePermissionRequest( + private fun handleRequiredPermissionRequest( addon: Addon, - promptRequest: WebExtensionPromptRequest.AfterInstallation.Permissions, + promptRequest: WebExtensionPromptRequest.AfterInstallation.Permissions.Required, ) { - showPermissionDialog( - addon = addon, - onConfirm = promptRequest.onConfirm, - ) + showPermissionDialog(addon = addon, promptRequest = promptRequest) } @VisibleForTesting internal fun handleOptionalPermissionsRequest( addon: Addon, - promptRequest: WebExtensionPromptRequest.AfterInstallation.OptionalPermissions, + promptRequest: WebExtensionPromptRequest.AfterInstallation.Permissions.Optional, ) { val shouldGrantWithoutPrompt = Addon.localizePermissions(promptRequest.permissions, context).isEmpty() // If we don't have any promptable permissions, just proceed. if (shouldGrantWithoutPrompt) { - promptRequest.onConfirm(true) - consumePromptRequest() + handlePermissions(promptRequest, granted = true) return } showPermissionDialog( - // This is a bit of a hack so that the permission prompt only lists - // the optional permissions that are requested. - addon = addon.copy(permissions = promptRequest.permissions), - onConfirm = promptRequest.onConfirm, + addon = addon, + promptRequest = promptRequest, forOptionalPermissions = true, + optionalPermissions = promptRequest.permissions, ) } @@ -211,8 +206,9 @@ class WebExtensionPromptFeature( @VisibleForTesting internal fun showPermissionDialog( addon: Addon, - onConfirm: (Boolean) -> Unit, + promptRequest: WebExtensionPromptRequest.AfterInstallation.Permissions, forOptionalPermissions: Boolean = false, + optionalPermissions: List = emptyList(), ) { if (isInstallationInProgress || hasExistingPermissionDialogFragment()) { return @@ -221,6 +217,7 @@ class WebExtensionPromptFeature( val dialog = PermissionsDialogFragment.newInstance( addon = addon, forOptionalPermissions = forOptionalPermissions, + optionalPermissions = optionalPermissions, promptsStyling = PermissionsDialogFragment.PromptsStyling( gravity = Gravity.BOTTOM, shouldWidthMatchParent = true, @@ -235,14 +232,8 @@ class WebExtensionPromptFeature( positiveButtonRadius = (context.resources.getDimensionPixelSize(R.dimen.tab_corner_radius)).toFloat(), ), - onPositiveButtonClicked = { - onConfirm(true) - consumePromptRequest() - }, - onNegativeButtonClicked = { - onConfirm(false) - consumePromptRequest() - }, + onPositiveButtonClicked = { handlePermissions(promptRequest, granted = true) }, + onNegativeButtonClicked = { handlePermissions(promptRequest, granted = false) }, ) dialog.show( fragmentManager, @@ -257,14 +248,14 @@ class WebExtensionPromptFeature( if (promptRequest is WebExtensionPromptRequest.AfterInstallation.Permissions && addon.id == promptRequest.extension.id ) { - handleApprovedPermissions(promptRequest) + handlePermissions(promptRequest, granted = true) } } } dialog.onNegativeButtonClicked = { store.state.webExtensionPromptRequest?.let { promptRequest -> if (promptRequest is WebExtensionPromptRequest.AfterInstallation.Permissions) { - handleDeniedPermissions(promptRequest) + handlePermissions(promptRequest, granted = false) } } } @@ -292,13 +283,11 @@ class WebExtensionPromptFeature( } } - private fun handleDeniedPermissions(promptRequest: WebExtensionPromptRequest.AfterInstallation.Permissions) { - promptRequest.onConfirm(false) - consumePromptRequest() - } - - private fun handleApprovedPermissions(promptRequest: WebExtensionPromptRequest.AfterInstallation.Permissions) { - promptRequest.onConfirm(true) + private fun handlePermissions( + promptRequest: WebExtensionPromptRequest.AfterInstallation.Permissions, + granted: Boolean, + ) { + promptRequest.onConfirm(granted) consumePromptRequest() } diff --git a/app/src/test/java/org/mozilla/fenix/extension/WebExtensionPromptFeatureTest.kt b/app/src/test/java/org/mozilla/fenix/extension/WebExtensionPromptFeatureTest.kt index c2d0dfb689..006aada989 100644 --- a/app/src/test/java/org/mozilla/fenix/extension/WebExtensionPromptFeatureTest.kt +++ b/app/src/test/java/org/mozilla/fenix/extension/WebExtensionPromptFeatureTest.kt @@ -220,14 +220,14 @@ class WebExtensionPromptFeatureTest { } @Test - fun `WHEN OptionalPermissions is dispatched THEN handleOptionalPermissionsRequest is called`() { + fun `WHEN PermissionsOptional is dispatched THEN handleOptionalPermissionsRequest is called`() { webExtensionPromptFeature.start() every { webExtensionPromptFeature.handleOptionalPermissionsRequest(any(), any()) } just runs store.dispatch( UpdatePromptRequestWebExtensionAction( - WebExtensionPromptRequest.AfterInstallation.OptionalPermissions( + WebExtensionPromptRequest.AfterInstallation.Permissions.Optional( mockk(relaxed = true), mockk(), mockk(), @@ -241,54 +241,60 @@ class WebExtensionPromptFeatureTest { @Test fun `WHEN calling handleOptionalPermissionsRequest with permissions THEN call showPermissionDialog`() { val addon: Addon = mockk(relaxed = true) - val onConfirm: ((Boolean) -> Unit) = mockk() - val promptRequest = WebExtensionPromptRequest.AfterInstallation.OptionalPermissions( + val promptRequest = WebExtensionPromptRequest.AfterInstallation.Permissions.Optional( extension = mockk(), permissions = listOf("tabs"), - onConfirm = onConfirm, + onConfirm = mockk(), ) - webExtensionPromptFeature.handleOptionalPermissionsRequest( - addon = addon, - promptRequest = promptRequest, - ) + webExtensionPromptFeature.handleOptionalPermissionsRequest(addon = addon, promptRequest = promptRequest) - verify { webExtensionPromptFeature.showPermissionDialog(any(), eq(onConfirm), eq(true)) } - // We should verify that only the requested optional permissions are assigned to the add-on because - // those are the permissions that are going to be listed in the dialog. - verify { addon.copy(permissions = promptRequest.permissions) } + verify { + webExtensionPromptFeature.showPermissionDialog( + eq(addon), + eq(promptRequest), + eq(true), + eq(promptRequest.permissions), + ) + } } @Test fun `WHEN calling handleOptionalPermissionsRequest with a permission that doesn't have a description THEN do not call showPermissionDialog`() { + val addon: Addon = mockk(relaxed = true) val onConfirm: ((Boolean) -> Unit) = mockk() every { onConfirm(any()) } just runs - val promptRequest = WebExtensionPromptRequest.AfterInstallation.OptionalPermissions( + val promptRequest = WebExtensionPromptRequest.AfterInstallation.Permissions.Optional( extension = mockk(), // The "scripting" API permission doesn't have a description so we should not show a dialog for it. permissions = listOf("scripting"), onConfirm = onConfirm, ) - webExtensionPromptFeature.handleOptionalPermissionsRequest(addon = mockk(relaxed = true), promptRequest = promptRequest) + webExtensionPromptFeature.handleOptionalPermissionsRequest(addon = addon, promptRequest = promptRequest) - verify(exactly = 0) { webExtensionPromptFeature.showPermissionDialog(any(), any(), any()) } + verify(exactly = 0) { + webExtensionPromptFeature.showPermissionDialog(any(), any(), any(), any()) + } verify(exactly = 1) { onConfirm(true) } } @Test fun `WHEN calling handleOptionalPermissionsRequest with no permissions THEN do not call showPermissionDialog`() { + val addon: Addon = mockk(relaxed = true) val onConfirm: ((Boolean) -> Unit) = mockk() every { onConfirm(any()) } just runs - val promptRequest = WebExtensionPromptRequest.AfterInstallation.OptionalPermissions( + val promptRequest = WebExtensionPromptRequest.AfterInstallation.Permissions.Optional( extension = mockk(), permissions = emptyList(), onConfirm = onConfirm, ) - webExtensionPromptFeature.handleOptionalPermissionsRequest(addon = mockk(relaxed = true), promptRequest = promptRequest) + webExtensionPromptFeature.handleOptionalPermissionsRequest(addon = addon, promptRequest = promptRequest) - verify(exactly = 0) { webExtensionPromptFeature.showPermissionDialog(any(), any(), any()) } + verify(exactly = 0) { + webExtensionPromptFeature.showPermissionDialog(any(), any(), any(), any()) + } verify(exactly = 1) { onConfirm(true) } } }