For #15291 - Limit current CFRs to show max one every 3 days
This commit is contained in:
parent
94d9a2a1ed
commit
24983af94e
|
@ -160,7 +160,7 @@ class BrowserFragment : BaseBrowserFragment(), UserInteractionHandler {
|
|||
}
|
||||
session?.register(toolbarSessionObserver, viewLifecycleOwner, autoPause = true)
|
||||
|
||||
if (settings.shouldShowOpenInAppBanner && session != null) {
|
||||
if (settings.shouldShowOpenInAppCfr && session != null) {
|
||||
openInAppOnboardingObserver = OpenInAppOnboardingObserver(
|
||||
context = context,
|
||||
navController = findNavController(),
|
||||
|
|
|
@ -42,7 +42,7 @@ class OpenInAppOnboardingObserver(
|
|||
|
||||
if (!loading &&
|
||||
!settings.openLinksInExternalApp &&
|
||||
settings.shouldShowOpenInAppBanner &&
|
||||
settings.shouldShowOpenInAppCfr &&
|
||||
appLink(session.url).hasExternalApp()
|
||||
) {
|
||||
infoBanner = InfoBanner(
|
||||
|
@ -60,6 +60,7 @@ class OpenInAppOnboardingObserver(
|
|||
|
||||
infoBanner?.showBanner()
|
||||
sessionDomainForDisplayedBanner = session.url.tryGetHostFromUrl()
|
||||
settings.lastCfrShownTimeInMillis = System.currentTimeMillis()
|
||||
settings.shouldShowOpenInAppBanner = false
|
||||
}
|
||||
}
|
||||
|
|
|
@ -35,7 +35,7 @@ class SearchWidgetCFR(
|
|||
|
||||
fun displayIfNecessary() {
|
||||
if (settings.isInSearchWidgetExperiment &&
|
||||
settings.shouldDisplaySearchWidgetCFR() &&
|
||||
settings.shouldDisplaySearchWidgetCfr() &&
|
||||
!isShown
|
||||
) {
|
||||
isShown = true
|
||||
|
@ -45,6 +45,7 @@ class SearchWidgetCFR(
|
|||
|
||||
@Suppress("InflateParams")
|
||||
private fun showSearchWidgetCFR() {
|
||||
settings.lastCfrShownTimeInMillis = System.currentTimeMillis()
|
||||
settings.incrementSearchWidgetCFRDisplayed()
|
||||
|
||||
val searchWidgetCFRDialog = Dialog(context)
|
||||
|
|
|
@ -320,7 +320,8 @@ class HomeFragment : Fragment() {
|
|||
)
|
||||
|
||||
view.homeAppBar.updateLayoutParams<ViewGroup.MarginLayoutParams> {
|
||||
topMargin = resources.getDimensionPixelSize(R.dimen.home_fragment_top_toolbar_header_margin)
|
||||
topMargin =
|
||||
resources.getDimensionPixelSize(R.dimen.home_fragment_top_toolbar_header_margin)
|
||||
}
|
||||
}
|
||||
ToolbarPosition.BOTTOM -> {
|
||||
|
@ -471,7 +472,8 @@ class HomeFragment : Fragment() {
|
|||
private fun removeAllTabsAndShowSnackbar(sessionCode: String) {
|
||||
val tabs = sessionManager.sessionsOfType(private = sessionCode == ALL_PRIVATE_TABS).toList()
|
||||
val selectedIndex = sessionManager
|
||||
.selectedSession?.let { sessionManager.sessions.indexOf(it) } ?: SessionManager.NO_SELECTION
|
||||
.selectedSession?.let { sessionManager.sessions.indexOf(it) }
|
||||
?: SessionManager.NO_SELECTION
|
||||
|
||||
val snapshot = tabs
|
||||
.map(sessionManager::createSessionSnapshot)
|
||||
|
@ -597,8 +599,8 @@ class HomeFragment : Fragment() {
|
|||
}, owner = this@HomeFragment.viewLifecycleOwner)
|
||||
}
|
||||
|
||||
if (context.settings().showPrivateModeContextualFeatureRecommender &&
|
||||
browsingModeManager.mode.isPrivate
|
||||
if (browsingModeManager.mode.isPrivate &&
|
||||
context.settings().showPrivateModeCfr
|
||||
) {
|
||||
recommendPrivateBrowsingShortcut()
|
||||
}
|
||||
|
@ -683,8 +685,8 @@ class HomeFragment : Fragment() {
|
|||
}
|
||||
|
||||
private fun recommendPrivateBrowsingShortcut() {
|
||||
context?.let {
|
||||
val layout = LayoutInflater.from(it)
|
||||
context?.let { context ->
|
||||
val layout = LayoutInflater.from(context)
|
||||
.inflate(R.layout.pbm_shortcut_popup, null)
|
||||
val privateBrowsingRecommend =
|
||||
PopupWindow(
|
||||
|
@ -712,6 +714,7 @@ class HomeFragment : Fragment() {
|
|||
// We want to show the popup only after privateBrowsingButton is available.
|
||||
// Otherwise, we will encounter an activity token error.
|
||||
privateBrowsingButton.post {
|
||||
context.settings().lastCfrShownTimeInMillis = System.currentTimeMillis()
|
||||
privateBrowsingRecommend.showAsDropDown(
|
||||
privateBrowsingButton, 0, CFR_Y_OFFSET, Gravity.TOP or Gravity.END
|
||||
)
|
||||
|
|
|
@ -24,10 +24,11 @@ class PwaOnboardingObserver(
|
|||
override fun onLoadingStateChanged(session: Session, loading: Boolean) {
|
||||
if (!loading && webAppUseCases.isInstallable() && !settings.userKnowsAboutPwas) {
|
||||
settings.incrementVisitedInstallableCount()
|
||||
if (settings.shouldShowPwaOnboarding) {
|
||||
if (settings.shouldShowPwaCfr) {
|
||||
val directions =
|
||||
BrowserFragmentDirections.actionBrowserFragmentToPwaOnboardingDialogFragment()
|
||||
navController.nav(R.id.browserFragment, directions)
|
||||
settings.lastCfrShownTimeInMillis = System.currentTimeMillis()
|
||||
settings.userKnowsAboutPwas = true
|
||||
}
|
||||
}
|
||||
|
|
|
@ -43,9 +43,9 @@ class TrackingProtectionOverlay(
|
|||
}
|
||||
|
||||
private fun shouldShowTrackingProtectionOnboarding(session: Session) =
|
||||
settings.shouldShowTrackingProtectionOnboarding &&
|
||||
session.trackerBlockingEnabled &&
|
||||
session.trackersBlocked.isNotEmpty()
|
||||
session.trackerBlockingEnabled &&
|
||||
session.trackersBlocked.isNotEmpty() &&
|
||||
settings.shouldShowTrackingProtectionCfr
|
||||
|
||||
@Suppress("MagicNumber", "InflateParams")
|
||||
private fun showTrackingProtectionOnboarding() {
|
||||
|
@ -57,9 +57,9 @@ class TrackingProtectionOverlay(
|
|||
if (event.action == MotionEvent.ACTION_DOWN) {
|
||||
metrics.track(Event.ContextualHintETPOutsideTap)
|
||||
}
|
||||
return super.onTouchEvent(event)
|
||||
}
|
||||
return super.onTouchEvent(event)
|
||||
}
|
||||
}
|
||||
|
||||
val layout = LayoutInflater.from(context)
|
||||
.inflate(R.layout.tracking_protection_onboarding_popup, null)
|
||||
|
@ -121,6 +121,7 @@ class TrackingProtectionOverlay(
|
|||
|
||||
metrics.track(Event.ContextualHintETPDisplayed)
|
||||
trackingOnboardingDialog.show()
|
||||
settings.lastCfrShownTimeInMillis = System.currentTimeMillis()
|
||||
settings.incrementTrackingProtectionOnboardingCount()
|
||||
}
|
||||
|
||||
|
|
|
@ -62,6 +62,7 @@ class Settings(private val appContext: Context) : PreferencesHolder {
|
|||
private const val CFR_COUNT_CONDITION_FOCUS_NOT_INSTALLED = 3
|
||||
|
||||
const val ONE_DAY_MS = 60 * 60 * 24 * 1000L
|
||||
const val THREE_DAYS_MS = 3 * ONE_DAY_MS
|
||||
const val ONE_WEEK_MS = 60 * 60 * 24 * 7 * 1000L
|
||||
const val ONE_MONTH_MS = (60 * 60 * 24 * 365 * 1000L) / 12
|
||||
|
||||
|
@ -115,6 +116,14 @@ class Settings(private val appContext: Context) : PreferencesHolder {
|
|||
default = 0L
|
||||
)
|
||||
|
||||
var lastCfrShownTimeInMillis by longPreference(
|
||||
appContext.getPreferenceKey(R.string.pref_key_last_cfr_shown_time),
|
||||
default = 0L
|
||||
)
|
||||
|
||||
val canShowCfr: Boolean
|
||||
get() = (System.currentTimeMillis() - lastCfrShownTimeInMillis) > THREE_DAYS_MS
|
||||
|
||||
var waitToShowPageUntilFirstPaint by featureFlagPreference(
|
||||
appContext.getPreferenceKey(R.string.pref_key_wait_first_paint),
|
||||
default = false,
|
||||
|
@ -191,11 +200,10 @@ class Settings(private val appContext: Context) : PreferencesHolder {
|
|||
private val isActiveSearcher: Boolean
|
||||
get() = activeSearchCount.value > 2
|
||||
|
||||
fun shouldDisplaySearchWidgetCFR(): Boolean =
|
||||
isActiveSearcher &&
|
||||
searchWidgetCFRDismissCount.underMaxCount() &&
|
||||
!searchWidgetInstalled &&
|
||||
!searchWidgetCFRManuallyDismissed
|
||||
fun shouldDisplaySearchWidgetCfr(): Boolean = canShowCfr && isActiveSearcher &&
|
||||
searchWidgetCFRDismissCount.underMaxCount() &&
|
||||
!searchWidgetInstalled &&
|
||||
!searchWidgetCFRManuallyDismissed
|
||||
|
||||
private val searchWidgetCFRDisplayCount = counterPreference(
|
||||
appContext.getPreferenceKey(R.string.pref_key_search_widget_cfr_display_count)
|
||||
|
@ -284,8 +292,8 @@ class Settings(private val appContext: Context) : PreferencesHolder {
|
|||
private var trackingProtectionOnboardingShownThisSession = false
|
||||
var isOverrideTPPopupsForPerformanceTest = false
|
||||
|
||||
val shouldShowTrackingProtectionOnboarding: Boolean
|
||||
get() = !isOverrideTPPopupsForPerformanceTest &&
|
||||
val shouldShowTrackingProtectionCfr: Boolean
|
||||
get() = !isOverrideTPPopupsForPerformanceTest && canShowCfr &&
|
||||
(trackingProtectionOnboardingCount.underMaxCount() &&
|
||||
!trackingProtectionOnboardingShownThisSession)
|
||||
|
||||
|
@ -650,8 +658,9 @@ class Settings(private val appContext: Context) : PreferencesHolder {
|
|||
private val userNeedsToVisitInstallableSites: Boolean
|
||||
get() = pwaInstallableVisitCount.underMaxCount()
|
||||
|
||||
val shouldShowPwaOnboarding: Boolean
|
||||
val shouldShowPwaCfr: Boolean
|
||||
get() {
|
||||
if (!canShowCfr) return false
|
||||
// We only want to show this on the 3rd time a user visits a site
|
||||
if (userNeedsToVisitInstallableSites) return false
|
||||
|
||||
|
@ -677,6 +686,9 @@ class Settings(private val appContext: Context) : PreferencesHolder {
|
|||
default = true
|
||||
)
|
||||
|
||||
val shouldShowOpenInAppCfr: Boolean
|
||||
get() = canShowCfr && shouldShowOpenInAppBanner
|
||||
|
||||
@VisibleForTesting(otherwise = PRIVATE)
|
||||
internal val trackingProtectionOnboardingCount = counterPreference(
|
||||
appContext.getPreferenceKey(R.string.pref_key_tracking_protection_onboarding),
|
||||
|
@ -833,8 +845,9 @@ class Settings(private val appContext: Context) : PreferencesHolder {
|
|||
appContext.getPreferenceKey(R.string.pref_key_private_mode_opened)
|
||||
)
|
||||
|
||||
val showPrivateModeContextualFeatureRecommender: Boolean
|
||||
val showPrivateModeCfr: Boolean
|
||||
get() {
|
||||
if (!canShowCfr) return false
|
||||
val focusInstalled = MozillaProductDetector
|
||||
.getInstalledMozillaProducts(appContext as Application)
|
||||
.contains(MozillaProductDetector.MozillaProducts.FOCUS.productName)
|
||||
|
|
|
@ -60,6 +60,7 @@
|
|||
<string name="pref_key_install_pwa_visits" translatable="false">pref_key_install_pwa_visits</string>
|
||||
<string name="pref_key_times_app_opened" translatable="false">pref_key_times_app_opened</string>
|
||||
<string name="pref_key_last_review_prompt_shown_time" translatable="false">pref_key_last_review_prompt_shown_time</string>
|
||||
<string name="pref_key_last_cfr_shown_time" translatable="false">pref_key_last_cfr_shown_time</string>
|
||||
|
||||
<!-- Data Choices -->
|
||||
<string name="pref_key_telemetry" translatable="false">pref_key_telemetry</string>
|
||||
|
|
|
@ -44,7 +44,7 @@ class TrackingProtectionOverlayTest {
|
|||
|
||||
@Test
|
||||
fun `no-op when loading`() {
|
||||
every { settings.shouldShowTrackingProtectionOnboarding } returns true
|
||||
every { settings.shouldShowTrackingProtectionCfr } returns true
|
||||
every { session.trackerBlockingEnabled } returns true
|
||||
every { session.trackersBlocked } returns listOf(mockk())
|
||||
|
||||
|
@ -54,7 +54,7 @@ class TrackingProtectionOverlayTest {
|
|||
|
||||
@Test
|
||||
fun `no-op when should not show onboarding`() {
|
||||
every { settings.shouldShowTrackingProtectionOnboarding } returns false
|
||||
every { settings.shouldShowTrackingProtectionCfr } returns false
|
||||
|
||||
overlay.onLoadingStateChanged(session, loading = false)
|
||||
verify(exactly = 0) { settings.incrementTrackingProtectionOnboardingCount() }
|
||||
|
@ -62,7 +62,7 @@ class TrackingProtectionOverlayTest {
|
|||
|
||||
@Test
|
||||
fun `no-op when tracking protection disabled`() {
|
||||
every { settings.shouldShowTrackingProtectionOnboarding } returns true
|
||||
every { settings.shouldShowTrackingProtectionCfr } returns true
|
||||
every { session.trackerBlockingEnabled } returns false
|
||||
|
||||
overlay.onLoadingStateChanged(session, loading = false)
|
||||
|
@ -71,7 +71,7 @@ class TrackingProtectionOverlayTest {
|
|||
|
||||
@Test
|
||||
fun `no-op when no trackers blocked`() {
|
||||
every { settings.shouldShowTrackingProtectionOnboarding } returns true
|
||||
every { settings.shouldShowTrackingProtectionCfr } returns true
|
||||
every { session.trackerBlockingEnabled } returns true
|
||||
every { session.trackersBlocked } returns emptyList()
|
||||
|
||||
|
@ -82,7 +82,7 @@ class TrackingProtectionOverlayTest {
|
|||
@Test
|
||||
fun `show onboarding when trackers are blocked`() {
|
||||
every { toolbar.hasWindowFocus() } returns true
|
||||
every { settings.shouldShowTrackingProtectionOnboarding } returns true
|
||||
every { settings.shouldShowTrackingProtectionCfr } returns true
|
||||
every { session.trackerBlockingEnabled } returns true
|
||||
every { session.trackersBlocked } returns listOf(mockk())
|
||||
|
||||
|
@ -93,7 +93,7 @@ class TrackingProtectionOverlayTest {
|
|||
@Test
|
||||
fun `no-op when toolbar doesn't have focus`() {
|
||||
every { toolbar.hasWindowFocus() } returns false
|
||||
every { settings.shouldShowTrackingProtectionOnboarding } returns true
|
||||
every { settings.shouldShowTrackingProtectionCfr } returns true
|
||||
every { session.trackerBlockingEnabled } returns true
|
||||
every { session.trackersBlocked } returns listOf(mockk())
|
||||
|
||||
|
|
|
@ -117,6 +117,20 @@ class SettingsTest {
|
|||
assertFalse(settings.isRemoteDebuggingEnabled)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun canShowCfrTest() {
|
||||
// When just created
|
||||
// Then
|
||||
assertEquals(0L, settings.lastCfrShownTimeInMillis)
|
||||
assertTrue(settings.canShowCfr)
|
||||
|
||||
// When
|
||||
settings.lastCfrShownTimeInMillis = System.currentTimeMillis()
|
||||
|
||||
// Then
|
||||
assertFalse(settings.canShowCfr)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun isTelemetryEnabled() {
|
||||
// When just created
|
||||
|
@ -394,25 +408,25 @@ class SettingsTest {
|
|||
fun showPwaFragment() {
|
||||
// When just created
|
||||
// Then
|
||||
assertFalse(settings.shouldShowPwaOnboarding)
|
||||
assertFalse(settings.shouldShowPwaCfr)
|
||||
|
||||
// When visited once
|
||||
settings.incrementVisitedInstallableCount()
|
||||
|
||||
// Then
|
||||
assertFalse(settings.shouldShowPwaOnboarding)
|
||||
assertFalse(settings.shouldShowPwaCfr)
|
||||
|
||||
// When visited twice
|
||||
settings.incrementVisitedInstallableCount()
|
||||
|
||||
// Then
|
||||
assertFalse(settings.shouldShowPwaOnboarding)
|
||||
assertFalse(settings.shouldShowPwaCfr)
|
||||
|
||||
// When visited thrice
|
||||
settings.incrementVisitedInstallableCount()
|
||||
|
||||
// Then
|
||||
assertTrue(settings.shouldShowPwaOnboarding)
|
||||
assertTrue(settings.shouldShowPwaCfr)
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
Loading…
Reference in New Issue