Bug 1810047 - Fixup WebExtensionPromptFeature

fenix/120.0
William Durand 1 year ago committed by mergify[bot]
parent d94b5d2f00
commit ee64e28554

@ -77,11 +77,11 @@ class WebExtensionPromptFeature(
// opens the add-ons manager. // opens the add-ons manager.
val addon = Addon.newFromWebExtension(promptRequest.extension) val addon = Addon.newFromWebExtension(promptRequest.extension)
when (promptRequest) { when (promptRequest) {
is WebExtensionPromptRequest.AfterInstallation.Permissions -> handlePermissionRequest( is WebExtensionPromptRequest.AfterInstallation.Permissions.Required -> handleRequiredPermissionRequest(
addon, addon,
promptRequest, promptRequest,
) )
is WebExtensionPromptRequest.AfterInstallation.OptionalPermissions -> handleOptionalPermissionsRequest( is WebExtensionPromptRequest.AfterInstallation.Permissions.Optional -> handleOptionalPermissionsRequest(
addon, addon,
promptRequest, promptRequest,
) )
@ -108,36 +108,31 @@ class WebExtensionPromptFeature(
showPostInstallationDialog(addon) showPostInstallationDialog(addon)
} }
private fun handlePermissionRequest( private fun handleRequiredPermissionRequest(
addon: Addon, addon: Addon,
promptRequest: WebExtensionPromptRequest.AfterInstallation.Permissions, promptRequest: WebExtensionPromptRequest.AfterInstallation.Permissions.Required,
) { ) {
showPermissionDialog( showPermissionDialog(addon = addon, promptRequest = promptRequest)
addon = addon,
onConfirm = promptRequest.onConfirm,
)
} }
@VisibleForTesting @VisibleForTesting
internal fun handleOptionalPermissionsRequest( internal fun handleOptionalPermissionsRequest(
addon: Addon, addon: Addon,
promptRequest: WebExtensionPromptRequest.AfterInstallation.OptionalPermissions, promptRequest: WebExtensionPromptRequest.AfterInstallation.Permissions.Optional,
) { ) {
val shouldGrantWithoutPrompt = Addon.localizePermissions(promptRequest.permissions, context).isEmpty() val shouldGrantWithoutPrompt = Addon.localizePermissions(promptRequest.permissions, context).isEmpty()
// If we don't have any promptable permissions, just proceed. // If we don't have any promptable permissions, just proceed.
if (shouldGrantWithoutPrompt) { if (shouldGrantWithoutPrompt) {
promptRequest.onConfirm(true) handlePermissions(promptRequest, granted = true)
consumePromptRequest()
return return
} }
showPermissionDialog( showPermissionDialog(
// This is a bit of a hack so that the permission prompt only lists addon = addon,
// the optional permissions that are requested. promptRequest = promptRequest,
addon = addon.copy(permissions = promptRequest.permissions),
onConfirm = promptRequest.onConfirm,
forOptionalPermissions = true, forOptionalPermissions = true,
optionalPermissions = promptRequest.permissions,
) )
} }
@ -211,8 +206,9 @@ class WebExtensionPromptFeature(
@VisibleForTesting @VisibleForTesting
internal fun showPermissionDialog( internal fun showPermissionDialog(
addon: Addon, addon: Addon,
onConfirm: (Boolean) -> Unit, promptRequest: WebExtensionPromptRequest.AfterInstallation.Permissions,
forOptionalPermissions: Boolean = false, forOptionalPermissions: Boolean = false,
optionalPermissions: List<String> = emptyList(),
) { ) {
if (isInstallationInProgress || hasExistingPermissionDialogFragment()) { if (isInstallationInProgress || hasExistingPermissionDialogFragment()) {
return return
@ -221,6 +217,7 @@ class WebExtensionPromptFeature(
val dialog = PermissionsDialogFragment.newInstance( val dialog = PermissionsDialogFragment.newInstance(
addon = addon, addon = addon,
forOptionalPermissions = forOptionalPermissions, forOptionalPermissions = forOptionalPermissions,
optionalPermissions = optionalPermissions,
promptsStyling = PermissionsDialogFragment.PromptsStyling( promptsStyling = PermissionsDialogFragment.PromptsStyling(
gravity = Gravity.BOTTOM, gravity = Gravity.BOTTOM,
shouldWidthMatchParent = true, shouldWidthMatchParent = true,
@ -235,14 +232,8 @@ class WebExtensionPromptFeature(
positiveButtonRadius = positiveButtonRadius =
(context.resources.getDimensionPixelSize(R.dimen.tab_corner_radius)).toFloat(), (context.resources.getDimensionPixelSize(R.dimen.tab_corner_radius)).toFloat(),
), ),
onPositiveButtonClicked = { onPositiveButtonClicked = { handlePermissions(promptRequest, granted = true) },
onConfirm(true) onNegativeButtonClicked = { handlePermissions(promptRequest, granted = false) },
consumePromptRequest()
},
onNegativeButtonClicked = {
onConfirm(false)
consumePromptRequest()
},
) )
dialog.show( dialog.show(
fragmentManager, fragmentManager,
@ -257,14 +248,14 @@ class WebExtensionPromptFeature(
if (promptRequest is WebExtensionPromptRequest.AfterInstallation.Permissions && if (promptRequest is WebExtensionPromptRequest.AfterInstallation.Permissions &&
addon.id == promptRequest.extension.id addon.id == promptRequest.extension.id
) { ) {
handleApprovedPermissions(promptRequest) handlePermissions(promptRequest, granted = true)
} }
} }
} }
dialog.onNegativeButtonClicked = { dialog.onNegativeButtonClicked = {
store.state.webExtensionPromptRequest?.let { promptRequest -> store.state.webExtensionPromptRequest?.let { promptRequest ->
if (promptRequest is WebExtensionPromptRequest.AfterInstallation.Permissions) { 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) { private fun handlePermissions(
promptRequest.onConfirm(false) promptRequest: WebExtensionPromptRequest.AfterInstallation.Permissions,
consumePromptRequest() granted: Boolean,
} ) {
promptRequest.onConfirm(granted)
private fun handleApprovedPermissions(promptRequest: WebExtensionPromptRequest.AfterInstallation.Permissions) {
promptRequest.onConfirm(true)
consumePromptRequest() consumePromptRequest()
} }

@ -220,14 +220,14 @@ class WebExtensionPromptFeatureTest {
} }
@Test @Test
fun `WHEN OptionalPermissions is dispatched THEN handleOptionalPermissionsRequest is called`() { fun `WHEN PermissionsOptional is dispatched THEN handleOptionalPermissionsRequest is called`() {
webExtensionPromptFeature.start() webExtensionPromptFeature.start()
every { webExtensionPromptFeature.handleOptionalPermissionsRequest(any(), any()) } just runs every { webExtensionPromptFeature.handleOptionalPermissionsRequest(any(), any()) } just runs
store.dispatch( store.dispatch(
UpdatePromptRequestWebExtensionAction( UpdatePromptRequestWebExtensionAction(
WebExtensionPromptRequest.AfterInstallation.OptionalPermissions( WebExtensionPromptRequest.AfterInstallation.Permissions.Optional(
mockk(relaxed = true), mockk(relaxed = true),
mockk(), mockk(),
mockk(), mockk(),
@ -241,54 +241,60 @@ class WebExtensionPromptFeatureTest {
@Test @Test
fun `WHEN calling handleOptionalPermissionsRequest with permissions THEN call showPermissionDialog`() { fun `WHEN calling handleOptionalPermissionsRequest with permissions THEN call showPermissionDialog`() {
val addon: Addon = mockk(relaxed = true) val addon: Addon = mockk(relaxed = true)
val onConfirm: ((Boolean) -> Unit) = mockk() val promptRequest = WebExtensionPromptRequest.AfterInstallation.Permissions.Optional(
val promptRequest = WebExtensionPromptRequest.AfterInstallation.OptionalPermissions(
extension = mockk(), extension = mockk(),
permissions = listOf("tabs"), permissions = listOf("tabs"),
onConfirm = onConfirm, onConfirm = mockk(),
) )
webExtensionPromptFeature.handleOptionalPermissionsRequest( webExtensionPromptFeature.handleOptionalPermissionsRequest(addon = addon, promptRequest = promptRequest)
addon = addon,
promptRequest = promptRequest,
)
verify { webExtensionPromptFeature.showPermissionDialog(any(), eq(onConfirm), eq(true)) } verify {
// We should verify that only the requested optional permissions are assigned to the add-on because webExtensionPromptFeature.showPermissionDialog(
// those are the permissions that are going to be listed in the dialog. eq(addon),
verify { addon.copy(permissions = promptRequest.permissions) } eq(promptRequest),
eq(true),
eq(promptRequest.permissions),
)
}
} }
@Test @Test
fun `WHEN calling handleOptionalPermissionsRequest with a permission that doesn't have a description THEN do not call showPermissionDialog`() { 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() val onConfirm: ((Boolean) -> Unit) = mockk()
every { onConfirm(any()) } just runs every { onConfirm(any()) } just runs
val promptRequest = WebExtensionPromptRequest.AfterInstallation.OptionalPermissions( val promptRequest = WebExtensionPromptRequest.AfterInstallation.Permissions.Optional(
extension = mockk(), extension = mockk(),
// The "scripting" API permission doesn't have a description so we should not show a dialog for it. // The "scripting" API permission doesn't have a description so we should not show a dialog for it.
permissions = listOf("scripting"), permissions = listOf("scripting"),
onConfirm = onConfirm, 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) } verify(exactly = 1) { onConfirm(true) }
} }
@Test @Test
fun `WHEN calling handleOptionalPermissionsRequest with no permissions THEN do not call showPermissionDialog`() { fun `WHEN calling handleOptionalPermissionsRequest with no permissions THEN do not call showPermissionDialog`() {
val addon: Addon = mockk(relaxed = true)
val onConfirm: ((Boolean) -> Unit) = mockk() val onConfirm: ((Boolean) -> Unit) = mockk()
every { onConfirm(any()) } just runs every { onConfirm(any()) } just runs
val promptRequest = WebExtensionPromptRequest.AfterInstallation.OptionalPermissions( val promptRequest = WebExtensionPromptRequest.AfterInstallation.Permissions.Optional(
extension = mockk(), extension = mockk(),
permissions = emptyList(), permissions = emptyList(),
onConfirm = onConfirm, 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) } verify(exactly = 1) { onConfirm(true) }
} }
} }

Loading…
Cancel
Save