ma1 pushed to branch firefox-android-115.2.1-13.5-1 at The Tor Project / Applications / firefox-android
Commits: cd556e14 by hackademix at 2024-08-02T22:31:10+02:00 Revert "Bug 1836786 - Improve prompts a=dmeehan - BP, tor-browser#42693"
This reverts commit 6271d70897cd1cd4fba39a75d63e3bd1869bcccc. We'll re-apply this patch (and others depending on it) after merging Bug 1903828 - Refactor PromptAbuserDetector into support-utils
- - - - - c60897e8 by hackademix at 2024-08-02T22:57:38+02:00 Bug 1903828 - Refactor PromptAbuserDetector into support-utils - BP, tor-browser#43005
- - - - - 1e885468 by hackademix at 2024-08-02T23:25:03+02:00 Bug 1836786 - Improve prompts r=gl - BP, tor-browser#43005
- - - - - 233d8217 by hackademix at 2024-08-04T17:32:36+02:00 Bug 1908344 - Improve prompts showing a=dmeehan - BP, tor-browser#43005
- - - - -
7 changed files:
- android-components/.buildconfig.yml - android-components/components/feature/prompts/src/main/java/mozilla/components/feature/prompts/PromptFeature.kt - android-components/components/feature/sitepermissions/build.gradle - android-components/components/feature/sitepermissions/src/main/java/mozilla/components/feature/sitepermissions/SitePermissionsDialogFragment.kt - android-components/components/feature/sitepermissions/src/test/java/mozilla/components/feature/sitepermissions/SitePermissionsDialogFragmentTest.kt - android-components/components/feature/prompts/src/main/java/mozilla/components/feature/prompts/dialog/PromptAbuserDetector.kt → android-components/components/support/utils/src/main/java/mozilla/components/support/ktx/util/PromptAbuserDetector.kt - focus-android/app/src/androidTest/java/org/mozilla/focus/activity/SitePermissionsTest.kt
Changes:
===================================== android-components/.buildconfig.yml ===================================== @@ -1150,7 +1150,6 @@ projects: - concept-tabstray - concept-toolbar - feature-session - - feature-prompts - feature-tabs - lib-publicsuffixlist - lib-state
===================================== android-components/components/feature/prompts/src/main/java/mozilla/components/feature/prompts/PromptFeature.kt ===================================== @@ -61,7 +61,6 @@ import mozilla.components.feature.prompts.dialog.ChoiceDialogFragment.Companion. import mozilla.components.feature.prompts.dialog.ColorPickerDialogFragment import mozilla.components.feature.prompts.dialog.ConfirmDialogFragment import mozilla.components.feature.prompts.dialog.MultiButtonDialogFragment -import mozilla.components.feature.prompts.dialog.PromptAbuserDetector import mozilla.components.feature.prompts.dialog.PromptDialogFragment import mozilla.components.feature.prompts.dialog.Prompter import mozilla.components.feature.prompts.dialog.SaveLoginDialogFragment @@ -91,6 +90,7 @@ import mozilla.components.support.base.feature.UserInteractionHandler import mozilla.components.support.base.log.logger.Logger import mozilla.components.support.ktx.kotlin.ifNullOrEmpty import mozilla.components.support.ktx.kotlinx.coroutines.flow.ifAnyChanged +import mozilla.components.support.ktx.util.PromptAbuserDetector import java.lang.ref.WeakReference import java.security.InvalidParameterException import java.util.Collections @@ -467,36 +467,51 @@ class PromptFeature private constructor( internal fun onPromptRequested(session: SessionState) { // Some requests are handle with intents session.content.promptRequests.lastOrNull()?.let { promptRequest -> - store.state.findTabOrCustomTabOrSelectedTab(customTabId)?.let { - promptRequest.executeIfWindowedPrompt { exitFullscreenUsecase(it.id) } + if (session.content.permissionRequestsList.isNotEmpty()) { + onCancel(session.id, promptRequest.uid) + } else { + processPromptRequest(promptRequest, session) } + } + }
- when (promptRequest) { - is File -> { - emitPromptDisplayedFact(promptName = "FilePrompt") - filePicker.handleFileRequest(promptRequest) - } - is Share -> handleShareRequest(promptRequest, session) - is SelectCreditCard -> { - emitSuccessfulCreditCardAutofillFormDetectedFact() - if (isCreditCardAutofillEnabled() && promptRequest.creditCards.isNotEmpty()) { - creditCardPicker?.handleSelectCreditCardRequest(promptRequest) - } + @Suppress("NestedBlockDepth") + private fun processPromptRequest( + promptRequest: PromptRequest, + session: SessionState, + ) { + store.state.findTabOrCustomTabOrSelectedTab(customTabId)?.let { + promptRequest.executeIfWindowedPrompt { exitFullscreenUsecase(it.id) } + } + + when (promptRequest) { + is File -> { + emitPromptDisplayedFact(promptName = "FilePrompt") + filePicker.handleFileRequest(promptRequest) + } + + is Share -> handleShareRequest(promptRequest, session) + is SelectCreditCard -> { + emitSuccessfulCreditCardAutofillFormDetectedFact() + if (isCreditCardAutofillEnabled() && promptRequest.creditCards.isNotEmpty()) { + creditCardPicker?.handleSelectCreditCardRequest(promptRequest) } - is SelectLoginPrompt -> { - emitPromptDisplayedFact(promptName = "SelectLoginPrompt") - if (promptRequest.logins.isNotEmpty()) { - loginPicker?.handleSelectLoginRequest(promptRequest) - } + } + + is SelectLoginPrompt -> { + emitPromptDisplayedFact(promptName = "SelectLoginPrompt") + if (promptRequest.logins.isNotEmpty()) { + loginPicker?.handleSelectLoginRequest(promptRequest) } - is SelectAddress -> { - emitSuccessfulAddressAutofillFormDetectedFact() - if (isAddressAutofillEnabled() && promptRequest.addresses.isNotEmpty()) { - addressPicker?.handleSelectAddressRequest(promptRequest) - } + } + is SelectAddress -> { + emitSuccessfulAddressAutofillFormDetectedFact() + if (isAddressAutofillEnabled() && promptRequest.addresses.isNotEmpty()) { + addressPicker?.handleSelectAddressRequest(promptRequest) } - else -> handleDialogsRequest(promptRequest, session) } + + else -> handleDialogsRequest(promptRequest, session) } }
===================================== android-components/components/feature/sitepermissions/build.gradle ===================================== @@ -58,8 +58,8 @@ dependencies { implementation project(':concept-engine') implementation project(':ui-icons') implementation project(':support-ktx') - implementation project(':feature-prompts') implementation project(':feature-tabs') + implementation project(':support-utils')
implementation ComponentsDependencies.kotlin_coroutines
===================================== android-components/components/feature/sitepermissions/src/main/java/mozilla/components/feature/sitepermissions/SitePermissionsDialogFragment.kt ===================================== @@ -23,7 +23,7 @@ import android.widget.TextView import androidx.annotation.VisibleForTesting import androidx.appcompat.app.AppCompatDialogFragment import androidx.core.content.ContextCompat -import mozilla.components.feature.prompts.dialog.PromptAbuserDetector +import mozilla.components.support.ktx.util.PromptAbuserDetector
internal const val KEY_SESSION_ID = "KEY_SESSION_ID" internal const val KEY_TITLE = "KEY_TITLE"
===================================== android-components/components/feature/sitepermissions/src/test/java/mozilla/components/feature/sitepermissions/SitePermissionsDialogFragmentTest.kt ===================================== @@ -19,6 +19,7 @@ import mozilla.components.support.test.robolectric.testContext import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue +import org.junit.Ignore import org.junit.Test import org.junit.runner.RunWith import org.mockito.Mockito.doNothing @@ -233,6 +234,7 @@ class SitePermissionsDialogFragmentTest { }
@Test + @Ignore("https://bugzilla.mozilla.org/show_bug.cgi?id=1903828") fun `clicking on positive button notifies the feature (temporary)`() { val mockFeature: SitePermissionsFeature = mock() val fragment = spy( @@ -261,6 +263,7 @@ class SitePermissionsDialogFragmentTest { }
@Test + @Ignore("https://bugzilla.mozilla.org/show_bug.cgi?id=1903828") fun `dismissing the dialog notifies the feature`() { val mockFeature: SitePermissionsFeature = mock() val fragment = spy( @@ -360,6 +363,7 @@ class SitePermissionsDialogFragmentTest { }
@Test + @Ignore("https://bugzilla.mozilla.org/show_bug.cgi?id=1903828") fun `clicking on positive button notifies the feature (permanent)`() { val mockFeature: SitePermissionsFeature = mock() val fragment = spy( @@ -389,6 +393,7 @@ class SitePermissionsDialogFragmentTest { }
@Test + @Ignore("https://bugzilla.mozilla.org/show_bug.cgi?id=1903828") fun `clicking on negative button notifies the feature (permanent)`() { val mockFeature: SitePermissionsFeature = mock() val fragment = spy(
===================================== android-components/components/feature/prompts/src/main/java/mozilla/components/feature/prompts/dialog/PromptAbuserDetector.kt → android-components/components/support/utils/src/main/java/mozilla/components/support/ktx/util/PromptAbuserDetector.kt ===================================== @@ -2,25 +2,38 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-package mozilla.components.feature.prompts.dialog +package mozilla.components.support.ktx.util
+import androidx.annotation.VisibleForTesting import java.util.Date
/** * Helper class to identify if a website has shown many dialogs. + * @param maxSuccessiveDialogSecondsLimit Maximum time required + * between dialogs in seconds before not showing more dialog. */ class PromptAbuserDetector(private val maxSuccessiveDialogSecondsLimit: Int = MAX_SUCCESSIVE_DIALOG_SECONDS_LIMIT) {
- internal var jsAlertCount = 0 - internal var lastDialogShownAt = Date() + @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) + var jsAlertCount = 0 + + @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) + var lastDialogShownAt = Date() + var shouldShowMoreDialogs = true private set
+ /** + * Updates internal state for alerts counts. + */ fun resetJSAlertAbuseState() { jsAlertCount = 0 shouldShowMoreDialogs = true }
+ /** + * Updates internal state for last shown and count of dialogs. + */ fun updateJSDialogAbusedState() { if (!areDialogsAbusedByTime()) { jsAlertCount = 0 @@ -29,25 +42,35 @@ class PromptAbuserDetector(private val maxSuccessiveDialogSecondsLimit: Int = MA lastDialogShownAt = Date() }
+ /** + * Indicates whether or not user wants to see more dialogs. + */ fun userWantsMoreDialogs(checkBox: Boolean) { shouldShowMoreDialogs = checkBox }
+ /** + * Indicates whether dialogs are being abused or not. + */ fun areDialogsBeingAbused(): Boolean { return areDialogsAbusedByTime() || areDialogsAbusedByCount() }
- internal fun areDialogsAbusedByTime(): Boolean { + @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) + @Suppress("UndocumentedPublicFunction") // this is visible only for tests + fun now() = Date() + + private fun areDialogsAbusedByTime(): Boolean { return if (jsAlertCount == 0) { false } else { - val now = Date() + val now = now() val diffInSeconds = (now.time - lastDialogShownAt.time) / SECOND_MS diffInSeconds < maxSuccessiveDialogSecondsLimit } }
- internal fun areDialogsAbusedByCount(): Boolean { + private fun areDialogsAbusedByCount(): Boolean { return jsAlertCount > MAX_SUCCESSIVE_DIALOG_COUNT }
===================================== focus-android/app/src/androidTest/java/org/mozilla/focus/activity/SitePermissionsTest.kt ===================================== @@ -229,6 +229,7 @@ class SitePermissionsTest {
@SmokeTest @Test + @Ignore fun testLocationSharingAllowed() { mockLocationUpdatesRule.setMockLocation()
@@ -244,6 +245,7 @@ class SitePermissionsTest {
@SmokeTest @Test + @Ignore fun allowCameraPermissionsTest() { Assume.assumeTrue(cameraManager.cameraIdList.isNotEmpty()) searchScreen {
View it on GitLab: https://gitlab.torproject.org/tpo/applications/firefox-android/-/compare/627...