[tor-commits] [Git][tpo/applications/firefox-android][firefox-android-115.2.1-13.5-1] 4 commits: Revert "Bug 1836786 - Improve prompts a=dmeehan - BP, tor-browser#42693"

ma1 (@ma1) git at gitlab.torproject.org
Mon Aug 5 08:49:33 UTC 2024



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/6271d70897cd1cd4fba39a75d63e3bd1869bcccc...233d82178b00182ec4ad1a52dae2fd75bd6311cb

-- 
View it on GitLab: https://gitlab.torproject.org/tpo/applications/firefox-android/-/compare/6271d70897cd1cd4fba39a75d63e3bd1869bcccc...233d82178b00182ec4ad1a52dae2fd75bd6311cb
You're receiving this email because of your account on gitlab.torproject.org.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.torproject.org/pipermail/tor-commits/attachments/20240805/2336865d/attachment-0001.htm>


More information about the tor-commits mailing list