commit b82ab024e668b6248e1e5628c89b29960cce53f1 parent 46eb82d1165b5e5f2f81e950f02883f11808ff9b Author: Akhil Pindiprolu <apindiprolu@mozilla.com> Date: Wed, 1 Oct 2025 16:55:50 +0000 Bug 1970744 - [WebCompat Reporter v2] Add a checkbox for sending URLs blocked by ETP r=android-reviewers,android-l10n-reviewers,tthibaud,flod,webcompat-reviewers,twisniewski Differential Revision: https://phabricator.services.mozilla.com/D260373 Diffstat:
15 files changed, 146 insertions(+), 18 deletions(-)
diff --git a/mobile/android/fenix/app/metrics.yaml b/mobile/android/fenix/app/metrics.yaml @@ -13823,15 +13823,20 @@ webcompatreporting: description: | Recorded when a user selects the Send button to submit webcompat report data. bugs: - - https://bugzilla.mozilla.org/show_bug.cgi?id=1932462 + - https://bugzilla.mozilla.org/show_bug.cgi?id=1970744 data_reviews: - - https://bugzilla.mozilla.org/show_bug.cgi?id=1932462 + - https://phabricator.services.mozilla.com/D260373 data_sensitivity: - interaction notification_emails: - twisniewski@mozilla.com - webcompat-reporting-tool-telemetry@mozilla.com expires: never + extra_keys: + sent_with_blocked_trackers: + description: > + Whether the user opted into including blocked tracker origins. + type: boolean send_more_info: type: event description: | diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/appstate/webcompat/WebCompatState.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/appstate/webcompat/WebCompatState.kt @@ -12,10 +12,12 @@ package org.mozilla.fenix.components.appstate.webcompat * in the Web Compat Reporter feature. * @property reason Optional param specifying the reason that [tabUrl] is broken. * @property problemDescription Description of the encountered problem. + * @property includeEtpBlockedUrls Checks if the user wants to include URLs blocked by ETP when they submit the report. */ data class WebCompatState( val tabUrl: String = "", val enteredUrl: String = "", val reason: String? = null, val problemDescription: String = "", + val includeEtpBlockedUrls: Boolean = false, ) diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/webcompat/middleware/WebCompatInfoDto.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/webcompat/middleware/WebCompatInfoDto.kt @@ -35,6 +35,7 @@ data class WebCompatInfoDto( * WebCompat anti-tracking data associated with the current tab. * * @property blockList The blocklist string. + * @property blockedOrigins The list of URLs currently blocked by ETP. * @property btpHasPurgedSite Whether the current tab has recently been purged by Bounce Tracking Protection. * @property etpCategory The current ETP category. * @property hasMixedActiveContentBlocked Whether the current tab has mixed active content blocked. @@ -45,6 +46,7 @@ data class WebCompatInfoDto( @Serializable data class WebCompatAntiTrackingDto( val blockList: String, + val blockedOrigins: List<String>, val btpHasPurgedSite: Boolean, val etpCategory: String, val hasMixedActiveContentBlocked: Boolean, diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/webcompat/middleware/WebCompatReporterStorageMiddleware.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/webcompat/middleware/WebCompatReporterStorageMiddleware.kt @@ -71,6 +71,7 @@ internal fun WebCompatState.toReporterState() = WebCompatReporterState( enteredUrl = enteredUrl, reason = reason?.let { WebCompatReporterState.BrokenSiteReason.valueOf(it) }, problemDescription = problemDescription, + includeEtpBlockedUrls = includeEtpBlockedUrls, ) @VisibleForTesting @@ -79,4 +80,5 @@ internal fun WebCompatReporterState.toPersistedState() = WebCompatState( enteredUrl = enteredUrl.ifEmpty { tabUrl }, // do not save the URL is the text field is empty reason = reason?.name, problemDescription = problemDescription, + includeEtpBlockedUrls = includeEtpBlockedUrls, ) diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/webcompat/middleware/WebCompatReporterSubmissionMiddleware.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/webcompat/middleware/WebCompatReporterSubmissionMiddleware.kt @@ -62,7 +62,10 @@ class WebCompatReporterSubmissionMiddleware( webCompatInfo?.let { val enteredUrlMatchesTabUrl = context.state.enteredUrl == webCompatInfo.url if (enteredUrlMatchesTabUrl) { - setTabAntiTrackingMetrics(antiTracking = webCompatInfo.antitracking) + setTabAntiTrackingMetrics( + antiTracking = webCompatInfo.antitracking, + sendBlockedUrls = context.state.includeEtpBlockedUrls, + ) setTabFrameworksMetrics(frameworks = webCompatInfo.frameworks) setTabLanguageMetrics(languages = webCompatInfo.languages) setTabUserAgentMetrics(userAgent = webCompatInfo.userAgent) @@ -98,8 +101,14 @@ class WebCompatReporterSubmissionMiddleware( } } - private fun setTabAntiTrackingMetrics(antiTracking: WebCompatInfoDto.WebCompatAntiTrackingDto) { + private fun setTabAntiTrackingMetrics( + antiTracking: WebCompatInfoDto.WebCompatAntiTrackingDto, + sendBlockedUrls: Boolean, + ) { BrokenSiteReportTabInfoAntitracking.blockList.set(antiTracking.blockList) + if (sendBlockedUrls) { + BrokenSiteReportTabInfoAntitracking.blockedOrigins.set(antiTracking.blockedOrigins) + } BrokenSiteReportTabInfoAntitracking.btpHasPurgedSite.set(antiTracking.btpHasPurgedSite) BrokenSiteReportTabInfoAntitracking.etpCategory.set(antiTracking.etpCategory) BrokenSiteReportTabInfoAntitracking.hasMixedActiveContentBlocked.set( diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/webcompat/middleware/WebCompatReporterTelemetryMiddleware.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/webcompat/middleware/WebCompatReporterTelemetryMiddleware.kt @@ -35,12 +35,15 @@ class WebCompatReporterTelemetryMiddleware : } WebCompatReporterAction.SendReportClicked -> { - Webcompatreporting.send.record(NoExtras()) + Webcompatreporting.send.record( + Webcompatreporting.SendExtra(sentWithBlockedTrackers = context.state.includeEtpBlockedUrls), + ) } WebCompatReporterAction.LearnMoreClicked -> { Webcompatreporting.learnMore.record(NoExtras()) } + else -> {} } } diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/webcompat/store/WebCompatReporterStore.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/webcompat/store/WebCompatReporterStore.kt @@ -22,12 +22,14 @@ import org.mozilla.fenix.R * @property enteredUrl The URL that is being reported as broken. * @property reason Specifies the reason that [enteredUrl] is broken. * @property problemDescription Description of the encountered problem. + * @property includeEtpBlockedUrls Checks if the user wants to include ETP-blocked URLs in the report. */ data class WebCompatReporterState( val tabUrl: String = "", val enteredUrl: String = "", val reason: BrokenSiteReason? = null, val problemDescription: String = "", + val includeEtpBlockedUrls: Boolean = false, ) : State { /** @@ -114,6 +116,13 @@ sealed class WebCompatReporterAction : Action { data class ReasonChanged(val newReason: WebCompatReporterState.BrokenSiteReason) : WebCompatReporterAction() /** + * Dispatched when the ETP checkbox is toggled. + * + * @property include The value of the checkbox being toggled or not. + */ + data class IncludeEtpBlockedUrlsChanged(val include: Boolean) : WebCompatReporterAction() + + /** * Dispatched when the problem description is updated. * * @property newProblemDescription The updated problem description. @@ -178,9 +187,10 @@ private fun reduce( WebCompatReporterAction.Initialized -> state is WebCompatReporterAction.StateRestored -> action.restoredState is WebCompatReporterAction.NavigationAction -> state - is WebCompatReporterAction.SendReportClicked -> state + WebCompatReporterAction.SendReportClicked -> state WebCompatReporterAction.SendMoreInfoClicked -> state WebCompatReporterAction.LearnMoreClicked -> state + is WebCompatReporterAction.IncludeEtpBlockedUrlsChanged -> state.copy(includeEtpBlockedUrls = action.include) } /** diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/webcompat/ui/WebCompatReporter.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/webcompat/ui/WebCompatReporter.kt @@ -18,7 +18,10 @@ import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.width import androidx.compose.foundation.layout.wrapContentSize import androidx.compose.foundation.rememberScrollState +import androidx.compose.foundation.selection.toggleable import androidx.compose.foundation.verticalScroll +import androidx.compose.material3.Checkbox +import androidx.compose.material3.CheckboxDefaults import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.Icon import androidx.compose.material3.IconButton @@ -32,6 +35,7 @@ import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.res.painterResource import androidx.compose.ui.res.stringResource +import androidx.compose.ui.semantics.Role import androidx.compose.ui.semantics.clearAndSetSemantics import androidx.compose.ui.semantics.error import androidx.compose.ui.semantics.semantics @@ -51,6 +55,7 @@ import mozilla.components.compose.base.modifier.thenConditional import mozilla.components.compose.base.text.Text.Resource import mozilla.components.compose.base.textfield.TextField import mozilla.components.compose.base.textfield.TextFieldColors +import mozilla.components.compose.base.theme.AcornTheme import mozilla.components.lib.state.ext.observeAsState import org.mozilla.fenix.Config import org.mozilla.fenix.R @@ -94,7 +99,8 @@ fun WebCompatReporter( containerColor = FirefoxTheme.colors.layer2, ) { paddingValues -> Column( - modifier = Modifier.verticalScroll(rememberScrollState()) + modifier = Modifier + .verticalScroll(rememberScrollState()) .padding(paddingValues) .imePadding() .padding(horizontal = 16.dp, vertical = 12.dp), @@ -172,7 +178,11 @@ fun WebCompatReporter( TextField( value = state.problemDescription, onValueChange = { - store.dispatch(WebCompatReporterAction.ProblemDescriptionChanged(newProblemDescription = it)) + store.dispatch( + WebCompatReporterAction.ProblemDescriptionChanged( + newProblemDescription = it, + ), + ) }, placeholder = stringResource(id = R.string.webcompat_reporter_problem_description_placeholder_text), errorText = "", @@ -184,6 +194,51 @@ fun WebCompatReporter( ), ) + Spacer(modifier = Modifier.height(16.dp)) + + Row( + modifier = Modifier + .toggleable( + value = state.includeEtpBlockedUrls, + role = Role.Checkbox, + onValueChange = { isChecked -> + store.dispatch( + WebCompatReporterAction.IncludeEtpBlockedUrlsChanged( + include = isChecked, + ), + ) + }, + ) + .padding(vertical = 6.dp), + verticalAlignment = Alignment.CenterVertically, + ) { + Checkbox( + checked = state.includeEtpBlockedUrls, + onCheckedChange = null, + modifier = Modifier, + colors = CheckboxDefaults.colors( + checkedColor = FirefoxTheme.colors.formSelected, + uncheckedColor = FirefoxTheme.colors.formDefault, + ), + ) + + Spacer(modifier = Modifier.width(16.dp)) + + Column { + Text( + text = stringResource(id = R.string.webcompat_reporter_etp_checkbox_text), + color = FirefoxTheme.colors.textPrimary, + style = AcornTheme.typography.subtitle1, + ) + + Text( + text = stringResource(id = R.string.webcompat_reporter_etp_checkbox_description), + color = FirefoxTheme.colors.textSecondary, + style = AcornTheme.typography.body2, + ) + } + } + Row( modifier = Modifier .fillMaxWidth() @@ -302,11 +357,11 @@ private class WebCompatPreviewParameterProvider : PreviewParameterProvider<WebCo enteredUrl = "www.example.com/url_parameters_that_break_the_page", reason = BrokenSiteReason.Slow, problemDescription = "The site wouldn’t load and after I tried xyz it still wouldn’t " + - "load and then again site wouldn’t load and after I tried xyz it still wouldn’t " + - "load and then again site wouldn’t load and after I tried xyz it still wouldn’t " + - "load and then again site wouldn’t load and after I tried xyz it still wouldn’t " + - "load and then again site wouldn’t load and after I tried xyz it still wouldn’t " + - "load and then again ", + "load and then again site wouldn’t load and after I tried xyz it still wouldn’t " + + "load and then again site wouldn’t load and after I tried xyz it still wouldn’t " + + "load and then again site wouldn’t load and after I tried xyz it still wouldn’t " + + "load and then again site wouldn’t load and after I tried xyz it still wouldn’t " + + "load and then again ", ), ) } diff --git a/mobile/android/fenix/app/src/main/res/values/strings.xml b/mobile/android/fenix/app/src/main/res/values/strings.xml @@ -1554,6 +1554,10 @@ <string name="webcompat_reporter_dismiss_success_snackbar_text" moz:removedIn="144" tools:ignore="UnusedResources">Close</string> <!-- Placeholder text for the description field to prompt the user on what they should type. --> <string name="webcompat_reporter_problem_description_placeholder_text">What happened? What should have happened?</string> + <!-- Label of the checkbox on the “Report broken site” screen for sending URLS blocked by Enhanced Tracking Protection. --> + <string name="webcompat_reporter_etp_checkbox_text">Send URLs blocked by tracking protection</string> + <!-- Description text for the checkbox on the “Report broken site” screen for sending URLS blocked by Enhanced Tracking Protection. It informs the user when checked, the report will include the list of URLs that were blocked by Enhanced Tracking Protection on the current page.--> + <string name="webcompat_reporter_etp_checkbox_description">Enhanced Tracking Protection may block trackers and scripts that some websites need to work properly.</string> <!-- These reason strings are dropdown options on a WebCompat reporter form, indicating what is broken on the site. --> <!-- Broken site reason text for site doesn’t load --> diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/components/appstate/WebCompatReducerTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/components/appstate/WebCompatReducerTest.kt @@ -34,7 +34,7 @@ class WebCompatReducerTest { @Test fun `WHEN the web compat state is reset THEN the web compat data in AppState should be reset`() { - val expected = AppState() + val expected = AppState(webCompatState = null) val actual = WebCompatReducer.reduce( state = AppState( webCompatState = WebCompatState( @@ -42,6 +42,7 @@ class WebCompatReducerTest { enteredUrl = "www.mozilla.org/3", reason = "slow", problemDescription = "problem description", + includeEtpBlockedUrls = true, ), ), action = WebCompatAction.WebCompatStateReset, @@ -58,6 +59,7 @@ class WebCompatReducerTest { enteredUrl = "www.mozilla.org/3", reason = "slow", problemDescription = "problem description", + includeEtpBlockedUrls = true, ), snackbarState = SnackbarState.None(), ) diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/webcompat/middleware/DefaultWebCompatReporterRetrievalServiceTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/webcompat/middleware/DefaultWebCompatReporterRetrievalServiceTest.kt @@ -46,6 +46,7 @@ class DefaultWebCompatReporterRetrievalServiceTest { hasMixedDisplayContentBlocked = false, hasTrackingContentBlocked = false, isPrivateBrowsing = false, + blockedOrigins = listOf("https://blockedUrlExample.com"), ), browser = WebCompatInfoDto.WebCompatBrowserDto( addons = listOf( @@ -142,6 +143,7 @@ class DefaultWebCompatReporterRetrievalServiceTest { hasMixedDisplayContentBlocked = false, hasTrackingContentBlocked = false, isPrivateBrowsing = false, + blockedOrigins = listOf("https://blockedUrlExample.com", "https://blockedUrlExample2.com"), ), browser = WebCompatInfoDto.WebCompatBrowserDto( addons = listOf( diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/webcompat/middleware/WebCompatReporterSubmissionMiddlewareTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/webcompat/middleware/WebCompatReporterSubmissionMiddlewareTest.kt @@ -687,6 +687,7 @@ class WebCompatReporterSubmissionMiddlewareTest { hasMixedDisplayContentBlocked = false, hasTrackingContentBlocked = false, isPrivateBrowsing = false, + blockedOrigins = listOf("https://exampleBlockedURLByETP.com"), ), browser = WebCompatInfoDto.WebCompatBrowserDto( addons = listOf( diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/webcompat/middleware/WebCompatReporterTelemetryMiddlewareTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/webcompat/middleware/WebCompatReporterTelemetryMiddlewareTest.kt @@ -61,6 +61,30 @@ class WebCompatReporterTelemetryMiddlewareTest { } @Test + fun `WHEN send report button is clicked and ETP checkbox is unchecked THEN record telemetry`() { + val store = createStore() + assertNull(Webcompatreporting.send.testGetValue()) + + store.dispatch(WebCompatReporterAction.SendReportClicked).joinBlocking() + + val snapshot = Webcompatreporting.send.testGetValue()!! + assertEquals(1, snapshot.size) + assertEquals("send", snapshot.single().name) + assertEquals("false", snapshot.single().extra?.get("sent_with_blocked_trackers")) + } + + @Test + fun `WHEN send report button is clicked and checkbox is checked THEN record telemetry`() { + val store = createStore() + store.dispatch(WebCompatReporterAction.IncludeEtpBlockedUrlsChanged(true)) + + store.dispatch(WebCompatReporterAction.SendReportClicked).joinBlocking() + + val snapshot = Webcompatreporting.send.testGetValue()!!.single() + assertEquals("true", snapshot.extra?.get("sent_with_blocked_trackers")) + } + + @Test fun `WHEN learn more button is clicked THEN record learn more button telemetry`() { val store = createStore() assertNull(Webcompatreporting.learnMore.testGetValue()) diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/webcompat/testdata/WebCompatTestData.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/webcompat/testdata/WebCompatTestData.kt @@ -15,7 +15,8 @@ object WebCompatTestData { "hasMixedActiveContentBlocked": false, "hasMixedDisplayContentBlocked": false, "hasTrackingContentBlocked": false, - "isPrivateBrowsing": false + "isPrivateBrowsing": false, + "blockedOrigins": ["https://blockedUrlExample.com"] }, "browser": { "addons": [ @@ -90,7 +91,8 @@ object WebCompatTestData { "hasMixedActiveContentBlocked": false, "hasMixedDisplayContentBlocked": false, "hasTrackingContentBlocked": false, - "isPrivateBrowsing": false + "isPrivateBrowsing": false, + "blockedOrigins": ["https://blockedUrlExample.com", "https://blockedUrlExample2.com"] }, "browser": { "addons": [ diff --git a/toolkit/components/reportbrokensite/metrics.yaml b/toolkit/components/reportbrokensite/metrics.yaml @@ -823,15 +823,20 @@ webcompatreporting: description: > Recorded when a user selects the Send button to submit webcompat report data. bugs: - - https://bugzilla.mozilla.org/show_bug.cgi?id=1852340 + - https://bugzilla.mozilla.org/show_bug.cgi?id=1970744 data_reviews: - - https://bugzilla.mozilla.org/show_bug.cgi?id=1852340#c16 + - https://phabricator.services.mozilla.com/D260373 data_sensitivity: - interaction notification_emails: - twisniewski@mozilla.com - webcompat-reporting-tool-telemetry@mozilla.com expires: never + extra_keys: + sent_with_blocked_trackers: + description: > + Whether the user opted into including blocked tracker origins. + type: boolean send_more_info: type: event description: >