From 4dd0c0f2240eed9ee24c54cfa0f55871a0e85e2f Mon Sep 17 00:00:00 2001 From: Tiger Oakes Date: Thu, 16 Jul 2020 15:05:01 -0700 Subject: [PATCH 01/17] For #12457: Add MockK matcher for intents (#12612) --- .../fenix/addons/AddonDetailsViewTest.kt | 15 +++++++++------ .../DefaultBrowsingModeManagerTest.kt | 4 ++-- .../metrics/BreadcrumbRecorderTest.kt | 16 +++++++++------- .../tips/providers/MigrationTipProviderTest.kt | 5 +---- .../org/mozilla/fenix/ext/MockKMatcherScope.kt | 18 ++++++++++++++++++ .../DefaultSessionControlControllerTest.kt | 16 ++++++++++++++-- .../search/DefaultSearchControllerTest.kt | 8 +++++++- .../incontent/InContentTelemetryTest.kt | 2 +- .../DefaultDeleteBrowsingDataControllerTest.kt | 2 +- 9 files changed, 62 insertions(+), 24 deletions(-) diff --git a/app/src/test/java/org/mozilla/fenix/addons/AddonDetailsViewTest.kt b/app/src/test/java/org/mozilla/fenix/addons/AddonDetailsViewTest.kt index 288c5d2f5..252b26f45 100644 --- a/app/src/test/java/org/mozilla/fenix/addons/AddonDetailsViewTest.kt +++ b/app/src/test/java/org/mozilla/fenix/addons/AddonDetailsViewTest.kt @@ -81,25 +81,28 @@ class AddonDetailsViewTest { @Test fun `bind addons version`() { - detailsView.bind(baseAddon.copy( + val addon1 = baseAddon.copy( version = "1.0.0", installedState = null - )) + ) + + detailsView.bind(addon1) assertEquals("1.0.0", view.version_text.text) view.version_text.performLongClick() - verify(exactly = 0) { interactor.showUpdaterDialog(any()) } + verify(exactly = 0) { interactor.showUpdaterDialog(addon1) } - detailsView.bind(baseAddon.copy( + val addon2 = baseAddon.copy( version = "1.0.0", installedState = Addon.InstalledState( id = "", version = "2.0.0", optionsPageUrl = null ) - )) + ) + detailsView.bind(addon2) assertEquals("2.0.0", view.version_text.text) view.version_text.performLongClick() - verify { interactor.showUpdaterDialog(any()) } + verify { interactor.showUpdaterDialog(addon2) } } @Test diff --git a/app/src/test/java/org/mozilla/fenix/browser/browsingmode/DefaultBrowsingModeManagerTest.kt b/app/src/test/java/org/mozilla/fenix/browser/browsingmode/DefaultBrowsingModeManagerTest.kt index 1130996d6..8313debc7 100644 --- a/app/src/test/java/org/mozilla/fenix/browser/browsingmode/DefaultBrowsingModeManagerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/browser/browsingmode/DefaultBrowsingModeManagerTest.kt @@ -32,12 +32,12 @@ class DefaultBrowsingModeManagerTest { manager.mode = BrowsingMode.Private manager.mode = BrowsingMode.Private - verify(exactly = 3) { callback.invoke(any()) } + verify(exactly = 3) { callback.invoke(BrowsingMode.Private) } manager.mode = BrowsingMode.Normal manager.mode = BrowsingMode.Normal - verify(exactly = 5) { callback.invoke(any()) } + verify(exactly = 2) { callback.invoke(BrowsingMode.Normal) } } @Test diff --git a/app/src/test/java/org/mozilla/fenix/components/metrics/BreadcrumbRecorderTest.kt b/app/src/test/java/org/mozilla/fenix/components/metrics/BreadcrumbRecorderTest.kt index ea9add329..61a62981b 100644 --- a/app/src/test/java/org/mozilla/fenix/components/metrics/BreadcrumbRecorderTest.kt +++ b/app/src/test/java/org/mozilla/fenix/components/metrics/BreadcrumbRecorderTest.kt @@ -13,6 +13,7 @@ import mozilla.components.lib.crash.Crash import mozilla.components.lib.crash.CrashReporter import mozilla.components.lib.crash.service.CrashReporterService import mozilla.components.support.base.crash.Breadcrumb +import org.junit.Assert.assertEquals import org.junit.Test internal class BreadcrumbRecorderTest { @@ -36,17 +37,18 @@ internal class BreadcrumbRecorderTest { ) ) - fun getBreadcrumbMessage(@Suppress("UNUSED_PARAMETER") destination: NavDestination): String { - return "test" - } - val navController: NavController = mockk() val navDestination: NavDestination = mockk() - val breadCrumbRecorder = - BreadcrumbsRecorder(reporter, navController, ::getBreadcrumbMessage) + val breadCrumbRecorder = BreadcrumbsRecorder(reporter, navController) { "test" } breadCrumbRecorder.onDestinationChanged(navController, navDestination, null) - verify { reporter.recordCrashBreadcrumb(any()) } + verify { + reporter.recordCrashBreadcrumb(withArg { + assertEquals("test", it.message) + assertEquals("DestinationChanged", it.category) + assertEquals(Breadcrumb.Level.INFO, it.level) + }) + } } } diff --git a/app/src/test/java/org/mozilla/fenix/components/tips/providers/MigrationTipProviderTest.kt b/app/src/test/java/org/mozilla/fenix/components/tips/providers/MigrationTipProviderTest.kt index a01272b38..cc45ed743 100644 --- a/app/src/test/java/org/mozilla/fenix/components/tips/providers/MigrationTipProviderTest.kt +++ b/app/src/test/java/org/mozilla/fenix/components/tips/providers/MigrationTipProviderTest.kt @@ -8,7 +8,6 @@ import android.content.Context import android.content.Intent import android.content.Intent.ACTION_VIEW import androidx.core.net.toUri -import io.mockk.MockKMatcherScope import io.mockk.Runs import io.mockk.every import io.mockk.just @@ -32,6 +31,7 @@ import org.mozilla.fenix.R import org.mozilla.fenix.components.metrics.MozillaProductDetector import org.mozilla.fenix.components.metrics.MozillaProductDetector.MozillaProducts import org.mozilla.fenix.components.tips.TipType +import org.mozilla.fenix.ext.intentFilterEq import org.mozilla.fenix.ext.settings import org.mozilla.fenix.helpers.FenixRobolectricTestRunner import org.mozilla.fenix.settings.SupportUtils @@ -205,7 +205,4 @@ class MigrationTipProviderTest { every { settings.shouldDisplayFenixMovingTip() } returns true assertTrue(MigrationTipProvider(context).shouldDisplay) } - - private fun MockKMatcherScope.intentFilterEq(value: Intent): Intent = - match { it.filterEquals(value) } } diff --git a/app/src/test/java/org/mozilla/fenix/ext/MockKMatcherScope.kt b/app/src/test/java/org/mozilla/fenix/ext/MockKMatcherScope.kt index 176a78b3c..6a49f0efe 100644 --- a/app/src/test/java/org/mozilla/fenix/ext/MockKMatcherScope.kt +++ b/app/src/test/java/org/mozilla/fenix/ext/MockKMatcherScope.kt @@ -1,5 +1,6 @@ package org.mozilla.fenix.ext +import android.content.Intent import androidx.navigation.NavDirections import io.mockk.Matcher import io.mockk.MockKMatcherScope @@ -11,6 +12,13 @@ import mozilla.components.support.ktx.android.os.contentEquals */ fun MockKMatcherScope.directionsEq(value: NavDirections) = match(EqNavDirectionsMatcher(value)) +/** + * Verify that two intents are the same for the purposes of intent resolution (filtering). + * Checks if their action, data, type, identity, class, and categories are the same. + * Does not compare extras. + */ +fun MockKMatcherScope.intentFilterEq(value: Intent) = match(EqIntentFilterMatcher(value)) + private data class EqNavDirectionsMatcher(private val value: NavDirections) : Matcher { override fun match(arg: NavDirections?): Boolean = @@ -19,3 +27,13 @@ private data class EqNavDirectionsMatcher(private val value: NavDirections) : Ma override fun substitute(map: Map) = copy(value = value.internalSubstitute(map)) } + +private data class EqIntentFilterMatcher(private val value: Intent) : Matcher { + + override fun match(arg: Intent?): Boolean = value.filterEquals(arg) + + override fun substitute(map: Map) = + copy(value = value.internalSubstitute(map)) + + override fun toString() = "intentFilterEq($value)" +} diff --git a/app/src/test/java/org/mozilla/fenix/home/DefaultSessionControlControllerTest.kt b/app/src/test/java/org/mozilla/fenix/home/DefaultSessionControlControllerTest.kt index 2db0a9b94..aa67649dc 100644 --- a/app/src/test/java/org/mozilla/fenix/home/DefaultSessionControlControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/home/DefaultSessionControlControllerTest.kt @@ -128,9 +128,21 @@ class DefaultSessionControlControllerTest { @Test fun handleDeleteCollectionTapped() { - val collection: TabCollection = mockk(relaxed = true) + val collection = mockk { + every { title } returns "Collection" + } + every { + activity.resources.getString(R.string.tab_collection_dialog_message, "Collection") + } returns "Are you sure you want to delete Collection?" + controller.handleDeleteCollectionTapped(collection) - verify { showDeleteCollectionPrompt(collection, null, any()) } + verify { + showDeleteCollectionPrompt( + collection, + null, + "Are you sure you want to delete Collection?" + ) + } } @Test diff --git a/app/src/test/java/org/mozilla/fenix/search/DefaultSearchControllerTest.kt b/app/src/test/java/org/mozilla/fenix/search/DefaultSearchControllerTest.kt index c2fea785d..cd9b08ffe 100644 --- a/app/src/test/java/org/mozilla/fenix/search/DefaultSearchControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/search/DefaultSearchControllerTest.kt @@ -4,6 +4,7 @@ package org.mozilla.fenix.search +import android.content.Intent import androidx.navigation.NavController import androidx.navigation.NavDirections import io.mockk.every @@ -24,7 +25,9 @@ import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.R import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.components.metrics.MetricController +import org.mozilla.fenix.crashes.CrashListActivity import org.mozilla.fenix.ext.components +import org.mozilla.fenix.ext.intentFilterEq import org.mozilla.fenix.ext.metrics import org.mozilla.fenix.ext.navigateSafe import org.mozilla.fenix.ext.searchEngineManager @@ -88,10 +91,13 @@ class DefaultSearchControllerTest { @Test fun handleCrashesUrlCommitted() { val url = "about:crashes" + every { activity.packageName } returns testContext.packageName controller.handleUrlCommitted(url) - verify { activity.startActivity(any()) } + verify { + activity.startActivity(intentFilterEq(Intent(testContext, CrashListActivity::class.java))) + } } @Test diff --git a/app/src/test/java/org/mozilla/fenix/search/telemetry/incontent/InContentTelemetryTest.kt b/app/src/test/java/org/mozilla/fenix/search/telemetry/incontent/InContentTelemetryTest.kt index 4714fe086..b491df390 100644 --- a/app/src/test/java/org/mozilla/fenix/search/telemetry/incontent/InContentTelemetryTest.kt +++ b/app/src/test/java/org/mozilla/fenix/search/telemetry/incontent/InContentTelemetryTest.kt @@ -61,7 +61,7 @@ class InContentTelemetryTest { telemetry.processMessage(message) - verify { telemetry.trackPartnerUrlTypeMetric(url, any()) } + verify { telemetry.trackPartnerUrlTypeMetric(url, listOf(first, second)) } } @Test diff --git a/app/src/test/java/org/mozilla/fenix/settings/deletebrowsingdata/DefaultDeleteBrowsingDataControllerTest.kt b/app/src/test/java/org/mozilla/fenix/settings/deletebrowsingdata/DefaultDeleteBrowsingDataControllerTest.kt index bf0cc524c..118edc67c 100644 --- a/app/src/test/java/org/mozilla/fenix/settings/deletebrowsingdata/DefaultDeleteBrowsingDataControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/settings/deletebrowsingdata/DefaultDeleteBrowsingDataControllerTest.kt @@ -54,7 +54,7 @@ class DefaultDeleteBrowsingDataControllerTest { controller.deleteBrowsingData() verify { - context.components.core.engine.clearData(any()) + context.components.core.engine.clearData(Engine.BrowsingData.select(Engine.BrowsingData.DOM_STORAGES)) context.components.core.historyStorage } } From 023a4983fa769a6fb9eea6600ec7ce8e620618c4 Mon Sep 17 00:00:00 2001 From: Elise Richards Date: Thu, 16 Jul 2020 17:08:04 -0500 Subject: [PATCH 02/17] For #10173: login duplicates and save (#11208) * Extract controller into it's own class. Implement find dupes and filter based on username. Create edit login controller. Add text watchers and check for duplicates. Edit controller test * Find duplicates and save to store * Retrieve duplicates from AC and check list on username text changed Move duplicates logic into the controller * Add glean pings for delete and edit. Move logic for login manipulation into the datastore. * Use correct threads in controller. Enable save button when applicable. Save enabled in datastore. Move login data to datastore Rebase with password error states Update metrics to be more specific for edit * Create logins controller for AC calls * Interactor and controller methods for edit login. Add edit view to separate out some layout manipulation. Inflate view in edit fragment. Double layout showing up. Edit view Controller tests Controller tests passing Interactor tests Lint and detekt cleanup * Remove datastore and use storage controller for all logins calls to password storage. Addressed comments Lint : Rebase - 1 --- app/metrics.yaml | 33 +++ .../java/org/mozilla/fenix/HomeActivity.kt | 5 +- .../components/metrics/GleanMetricsService.kt | 9 + .../fenix/components/metrics/Metrics.kt | 3 + .../fenix/library/history/HistoryFragment.kt | 7 +- .../fenix/settings/logins/LoginDetailView.kt | 20 -- .../settings/logins/LoginsFragmentStore.kt | 18 +- .../fenix/settings/logins/SavedLoginsView.kt | 135 --------- .../logins/controller/LoginsListController.kt | 64 ++++ .../SavedLoginsStorageController.kt | 198 +++++++++++++ .../{ => fragment}/EditLoginFragment.kt | 275 +++++++++--------- .../{ => fragment}/LoginDetailFragment.kt | 129 +++----- .../{ => fragment}/SavedLoginsAuthFragment.kt | 5 +- .../{ => fragment}/SavedLoginsFragment.kt | 141 +++++---- .../SavedLoginsSettingFragment.kt | 2 +- .../logins/interactor/EditLoginInteractor.kt | 24 ++ .../interactor/LoginDetailInteractor.kt | 24 ++ .../interactor/SavedLoginsInteractor.kt | 39 +++ .../settings/logins/view/EditLoginView.kt | 54 ++++ .../settings/logins/view/LoginDetailView.kt | 65 +++++ .../logins/{ => view}/LoginsAdapter.kt | 4 +- .../logins/{ => view}/LoginsListViewHolder.kt | 4 +- .../logins/view/SavedLoginsListView.kt | 67 +++++ .../java/org/mozilla/fenix/utils/Settings.kt | 2 +- .../main/res/color/save_enabled_ic_color.xml | 8 + .../saved_login_clear_edit_text_tint.xml | 2 +- .../main/res/layout/fragment_edit_login.xml | 3 +- .../main/res/layout/fragment_saved_logins.xml | 2 +- app/src/main/res/menu/login_save.xml | 2 +- app/src/main/res/navigation/nav_graph.xml | 10 +- .../logins/EditLoginInteractorTest.kt | 34 +++ .../logins/LoginDetailInteractorTest.kt | 30 ++ .../settings/logins/LoginDetailViewTest.kt | 4 +- .../logins/LoginsFragmentStoreTest.kt | 3 +- ...lerTest.kt => LoginsListControllerTest.kt} | 25 +- .../logins/LoginsListViewHolderTest.kt | 12 +- .../logins/SavedLoginsInteractorTest.kt | 26 +- .../SavedLoginsStorageControllerTest.kt | 139 +++++++++ docs/metrics.md | 3 + 39 files changed, 1140 insertions(+), 490 deletions(-) delete mode 100644 app/src/main/java/org/mozilla/fenix/settings/logins/LoginDetailView.kt delete mode 100644 app/src/main/java/org/mozilla/fenix/settings/logins/SavedLoginsView.kt create mode 100644 app/src/main/java/org/mozilla/fenix/settings/logins/controller/LoginsListController.kt create mode 100644 app/src/main/java/org/mozilla/fenix/settings/logins/controller/SavedLoginsStorageController.kt rename app/src/main/java/org/mozilla/fenix/settings/logins/{ => fragment}/EditLoginFragment.kt (53%) rename app/src/main/java/org/mozilla/fenix/settings/logins/{ => fragment}/LoginDetailFragment.kt (66%) rename app/src/main/java/org/mozilla/fenix/settings/logins/{ => fragment}/SavedLoginsAuthFragment.kt (98%) rename app/src/main/java/org/mozilla/fenix/settings/logins/{ => fragment}/SavedLoginsFragment.kt (66%) rename app/src/main/java/org/mozilla/fenix/settings/logins/{ => fragment}/SavedLoginsSettingFragment.kt (98%) create mode 100644 app/src/main/java/org/mozilla/fenix/settings/logins/interactor/EditLoginInteractor.kt create mode 100644 app/src/main/java/org/mozilla/fenix/settings/logins/interactor/LoginDetailInteractor.kt create mode 100644 app/src/main/java/org/mozilla/fenix/settings/logins/interactor/SavedLoginsInteractor.kt create mode 100644 app/src/main/java/org/mozilla/fenix/settings/logins/view/EditLoginView.kt create mode 100644 app/src/main/java/org/mozilla/fenix/settings/logins/view/LoginDetailView.kt rename app/src/main/java/org/mozilla/fenix/settings/logins/{ => view}/LoginsAdapter.kt (88%) rename app/src/main/java/org/mozilla/fenix/settings/logins/{ => view}/LoginsListViewHolder.kt (88%) create mode 100644 app/src/main/java/org/mozilla/fenix/settings/logins/view/SavedLoginsListView.kt create mode 100644 app/src/main/res/color/save_enabled_ic_color.xml create mode 100644 app/src/test/java/org/mozilla/fenix/settings/logins/EditLoginInteractorTest.kt create mode 100644 app/src/test/java/org/mozilla/fenix/settings/logins/LoginDetailInteractorTest.kt rename app/src/test/java/org/mozilla/fenix/settings/logins/{SavedLoginsControllerTest.kt => LoginsListControllerTest.kt} (81%) create mode 100644 app/src/test/java/org/mozilla/fenix/settings/logins/SavedLoginsStorageControllerTest.kt diff --git a/app/metrics.yaml b/app/metrics.yaml index 8f7e414e7..2c9163ba4 100644 --- a/app/metrics.yaml +++ b/app/metrics.yaml @@ -2444,6 +2444,39 @@ logins: notification_emails: - fenix-core@mozilla.com expires: "2020-10-01" + open_login_editor: + type: event + description: | + A user entered the edit screen for an individual saved login + bugs: + - https://github.com/mozilla-mobile/fenix/issues/10173 + data_reviews: + - https://github.com/mozilla-mobile/fenix/issues/11208 + notification_emails: + - fenix-core@mozilla.com + expires: "2020-10-01" + delete_saved_login: + type: event + description: | + A user confirms delete of a saved login + bugs: + - https://github.com/mozilla-mobile/fenix/issues/10173 + data_reviews: + - https://github.com/mozilla-mobile/fenix/issues/11208 + notification_emails: + - fenix-core@mozilla.com + expires: "2020-10-01" + save_edited_login: + type: event + description: | + A user saves changes made to an individual login + bugs: + - https://github.com/mozilla-mobile/fenix/issues/10173 + data_reviews: + - https://github.com/mozilla-mobile/fenix/issues/11208 + notification_emails: + - fenix-core@mozilla.com + expires: "2020-10-01" download_notification: resume: diff --git a/app/src/main/java/org/mozilla/fenix/HomeActivity.kt b/app/src/main/java/org/mozilla/fenix/HomeActivity.kt index ca06e6964..8acd3d709 100644 --- a/app/src/main/java/org/mozilla/fenix/HomeActivity.kt +++ b/app/src/main/java/org/mozilla/fenix/HomeActivity.kt @@ -26,6 +26,7 @@ import androidx.navigation.fragment.NavHostFragment import androidx.navigation.ui.AppBarConfiguration import androidx.navigation.ui.NavigationUI import androidx.recyclerview.widget.LinearLayoutManager +import com.google.android.gms.tasks.Tasks.call import kotlinx.android.synthetic.main.activity_home.* import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers @@ -86,8 +87,8 @@ import org.mozilla.fenix.session.NotificationSessionObserver import org.mozilla.fenix.settings.SettingsFragmentDirections import org.mozilla.fenix.settings.TrackingProtectionFragmentDirections import org.mozilla.fenix.settings.about.AboutFragmentDirections -import org.mozilla.fenix.settings.logins.LoginDetailFragmentDirections -import org.mozilla.fenix.settings.logins.SavedLoginsAuthFragmentDirections +import org.mozilla.fenix.settings.logins.fragment.SavedLoginsAuthFragmentDirections +import org.mozilla.fenix.settings.logins.fragment.LoginDetailFragmentDirections import org.mozilla.fenix.settings.search.AddSearchEngineFragmentDirections import org.mozilla.fenix.settings.search.EditCustomSearchEngineFragmentDirections import org.mozilla.fenix.share.AddNewDeviceFragmentDirections diff --git a/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt b/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt index bd1c43e7c..677e0aba7 100644 --- a/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt +++ b/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt @@ -466,6 +466,15 @@ private val Event.wrapper: EventWrapper<*>? is Event.ViewLoginPassword -> EventWrapper( { Logins.viewPasswordLogin.record(it) } ) + is Event.DeleteLogin -> EventWrapper( + { Logins.deleteSavedLogin.record(it) } + ) + is Event.EditLogin -> EventWrapper( + { Logins.openLoginEditor.record(it) } + ) + is Event.EditLoginSave -> EventWrapper( + { Logins.saveEditedLogin.record(it) } + ) is Event.PrivateBrowsingShowSearchSuggestions -> EventWrapper( { SearchSuggestions.enableInPrivate.record(it) } ) diff --git a/app/src/main/java/org/mozilla/fenix/components/metrics/Metrics.kt b/app/src/main/java/org/mozilla/fenix/components/metrics/Metrics.kt index 9e8fe2d88..77a08ce20 100644 --- a/app/src/main/java/org/mozilla/fenix/components/metrics/Metrics.kt +++ b/app/src/main/java/org/mozilla/fenix/components/metrics/Metrics.kt @@ -154,6 +154,9 @@ sealed class Event { object OpenLogins : Event() object OpenOneLogin : Event() object CopyLogin : Event() + object DeleteLogin : Event() + object EditLogin : Event() + object EditLoginSave : Event() object ViewLoginPassword : Event() object CustomEngineAdded : Event() object CustomEngineDeleted : Event() diff --git a/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragment.kt b/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragment.kt index a6796f1d6..868ac3668 100644 --- a/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragment.kt @@ -210,9 +210,10 @@ class HistoryFragment : LibraryPageFragment(), UserInteractionHandl return if (historyItems.size > 1) { getString(R.string.history_delete_multiple_items_snackbar) } else { - getString( - R.string.history_delete_single_item_snackbar, - historyItems.first().url.toShortUrl(requireComponents.publicSuffixList) + String.format( + requireContext().getString( + R.string.history_delete_single_item_snackbar + ), historyItems.first().url.toShortUrl(requireComponents.publicSuffixList) ) } } diff --git a/app/src/main/java/org/mozilla/fenix/settings/logins/LoginDetailView.kt b/app/src/main/java/org/mozilla/fenix/settings/logins/LoginDetailView.kt deleted file mode 100644 index 0eb6300d8..000000000 --- a/app/src/main/java/org/mozilla/fenix/settings/logins/LoginDetailView.kt +++ /dev/null @@ -1,20 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * 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 org.mozilla.fenix.settings.logins - -import android.view.ViewGroup -import kotlinx.android.extensions.LayoutContainer -import kotlinx.android.synthetic.main.fragment_login_detail.* - -/** - * View that contains and configures the Login Details - */ -class LoginDetailView(override val containerView: ViewGroup?) : LayoutContainer { - fun update(login: LoginsListState) { - webAddressText.text = login.currentItem?.origin - usernameText.text = login.currentItem?.username - passwordText.text = login.currentItem?.password - } -} diff --git a/app/src/main/java/org/mozilla/fenix/settings/logins/LoginsFragmentStore.kt b/app/src/main/java/org/mozilla/fenix/settings/logins/LoginsFragmentStore.kt index 9e10508fd..ea130f07e 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/logins/LoginsFragmentStore.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/logins/LoginsFragmentStore.kt @@ -54,18 +54,20 @@ sealed class LoginsAction : Action { data class UpdateLoginsList(val list: List) : LoginsAction() data class UpdateCurrentLogin(val item: SavedLogin) : LoginsAction() data class SortLogins(val sortingStrategy: SortingStrategy) : LoginsAction() + data class ListOfDupes(val dupeList: List) : LoginsAction() data class LoginSelected(val item: SavedLogin) : LoginsAction() } /** * The state for the Saved Logins Screen - * @property loginList Source of truth for local list of logins * @property loginList Filterable list of logins to display * @property currentItem The last item that was opened into the detail view * @property searchedForText String used by the user to filter logins * @property sortingStrategy sorting strategy selected by the user (Currently we support * sorting alphabetically and by last used) * @property highlightedItem The current selected sorting strategy from the sort menu + * @property duplicateLogins The current list of possible duplicates for a selected login origin, + * httpRealm, and formActionOrigin */ data class LoginsListState( val isLoading: Boolean = false, @@ -74,7 +76,8 @@ data class LoginsListState( val currentItem: SavedLogin? = null, val searchedForText: String?, val sortingStrategy: SortingStrategy, - val highlightedItem: SavedLoginsSortingStrategyMenu.Item + val highlightedItem: SavedLoginsSortingStrategyMenu.Item, + val duplicateLogins: List ) : State /** @@ -113,9 +116,14 @@ private fun savedLoginsStateReducer( } is LoginsAction.LoginSelected -> { state.copy( - isLoading = true, - loginList = emptyList(), - filteredItems = emptyList() + isLoading = true, + loginList = emptyList(), + filteredItems = emptyList() + ) + } + is LoginsAction.ListOfDupes -> { + state.copy( + duplicateLogins = action.dupeList ) } } diff --git a/app/src/main/java/org/mozilla/fenix/settings/logins/SavedLoginsView.kt b/app/src/main/java/org/mozilla/fenix/settings/logins/SavedLoginsView.kt deleted file mode 100644 index dd1564df3..000000000 --- a/app/src/main/java/org/mozilla/fenix/settings/logins/SavedLoginsView.kt +++ /dev/null @@ -1,135 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * 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 org.mozilla.fenix.settings.logins - -import android.text.method.LinkMovementMethod -import android.view.LayoutInflater -import android.view.ViewGroup -import android.widget.FrameLayout -import androidx.core.view.isVisible -import androidx.navigation.NavController -import androidx.recyclerview.widget.LinearLayoutManager -import kotlinx.android.extensions.LayoutContainer -import kotlinx.android.synthetic.main.component_saved_logins.view.* -import org.mozilla.fenix.BrowserDirection -import org.mozilla.fenix.R -import org.mozilla.fenix.components.metrics.Event -import org.mozilla.fenix.components.metrics.MetricController -import org.mozilla.fenix.ext.addUnderline -import org.mozilla.fenix.settings.SupportUtils -import org.mozilla.fenix.utils.Settings - -/** - * View that contains and configures the Saved Logins List - */ -class SavedLoginsView( - override val containerView: ViewGroup, - val interactor: SavedLoginsInteractor -) : LayoutContainer { - - val view: FrameLayout = LayoutInflater.from(containerView.context) - .inflate(R.layout.component_saved_logins, containerView, true) - .findViewById(R.id.saved_logins_wrapper) - - private val loginsAdapter = LoginsAdapter(interactor) - - init { - view.saved_logins_list.apply { - adapter = loginsAdapter - layoutManager = LinearLayoutManager(containerView.context) - itemAnimator = null - } - - with(view.saved_passwords_empty_learn_more) { - movementMethod = LinkMovementMethod.getInstance() - addUnderline() - setOnClickListener { interactor.onLearnMoreClicked() } - } - - with(view.saved_passwords_empty_message) { - val appName = context.getString(R.string.app_name) - text = String.format( - context.getString( - R.string.preferences_passwords_saved_logins_description_empty_text - ), appName - ) - } - } - - fun update(state: LoginsListState) { - // todo MVI views should not have logic. Needs refactoring. - if (state.isLoading) { - view.progress_bar.isVisible = true - } else { - view.progress_bar.isVisible = false - view.saved_logins_list.isVisible = state.loginList.isNotEmpty() - view.saved_passwords_empty_view.isVisible = state.loginList.isEmpty() - } - loginsAdapter.submitList(state.filteredItems) - } -} - -/** - * Interactor for the saved logins screen - * - * @param savedLoginsController [SavedLoginsController] which will be delegated for all users interactions. - */ -class SavedLoginsInteractor( - private val savedLoginsController: SavedLoginsController -) { - fun onItemClicked(item: SavedLogin) { - savedLoginsController.handleItemClicked(item) - } - - fun onLearnMoreClicked() { - savedLoginsController.handleLearnMoreClicked() - } - - fun onSortingStrategyChanged(sortingStrategy: SortingStrategy) { - savedLoginsController.handleSort(sortingStrategy) - } -} - -/** - * Controller for the saved logins screen - * - * @param store Store used to hold in-memory collection state. - * @param navController NavController manages app navigation within a NavHost. - * @param browserNavigator Controller allowing browser navigation to any Uri. - * @param settings SharedPreferences wrapper for easier usage. - * @param metrics Controller that handles telemetry events. - */ -class SavedLoginsController( - private val store: LoginsFragmentStore, - private val navController: NavController, - private val browserNavigator: ( - searchTermOrURL: String, - newTab: Boolean, - from: BrowserDirection - ) -> Unit, - private val settings: Settings, - private val metrics: MetricController -) { - fun handleSort(sortingStrategy: SortingStrategy) { - store.dispatch(LoginsAction.SortLogins(sortingStrategy)) - settings.savedLoginsSortingStrategy = sortingStrategy - } - - fun handleItemClicked(item: SavedLogin) { - store.dispatch(LoginsAction.LoginSelected(item)) - metrics.track(Event.OpenOneLogin) - navController.navigate( - SavedLoginsFragmentDirections.actionSavedLoginsFragmentToLoginDetailFragment(item.guid) - ) - } - - fun handleLearnMoreClicked() { - browserNavigator.invoke( - SupportUtils.getGenericSumoURLForTopic(SupportUtils.SumoTopic.SYNC_SETUP), - true, - BrowserDirection.FromSavedLoginsFragment - ) - } -} diff --git a/app/src/main/java/org/mozilla/fenix/settings/logins/controller/LoginsListController.kt b/app/src/main/java/org/mozilla/fenix/settings/logins/controller/LoginsListController.kt new file mode 100644 index 000000000..8d4c5629f --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/settings/logins/controller/LoginsListController.kt @@ -0,0 +1,64 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * 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 org.mozilla.fenix.settings.logins.controller + +import androidx.navigation.NavController +import org.mozilla.fenix.BrowserDirection +import org.mozilla.fenix.components.metrics.Event +import org.mozilla.fenix.components.metrics.MetricController +import org.mozilla.fenix.settings.SupportUtils +import org.mozilla.fenix.settings.logins.LoginsAction +import org.mozilla.fenix.settings.logins.LoginsFragmentStore +import org.mozilla.fenix.settings.logins.SavedLogin +import org.mozilla.fenix.settings.logins.SortingStrategy +import org.mozilla.fenix.settings.logins.fragment.SavedLoginsFragmentDirections +import org.mozilla.fenix.utils.Settings + +/** + * Controller for the saved logins list + * + * @param loginsFragmentStore Store used to hold in-memory collection state. + * @param navController NavController manages app navigation within a NavHost. + * @param browserNavigator Controller allowing browser navigation to any Uri. + * @param settings SharedPreferences wrapper for easier usage. + * @param metrics Controller that handles telemetry events. + */ +class LoginsListController( + private val loginsFragmentStore: LoginsFragmentStore, + private val navController: NavController, + private val browserNavigator: ( + searchTermOrURL: String, + newTab: Boolean, + from: BrowserDirection + ) -> Unit, + private val settings: Settings, + private val metrics: MetricController +) { + + fun handleItemClicked(item: SavedLogin) { + loginsFragmentStore.dispatch(LoginsAction.LoginSelected(item)) + metrics.track(Event.OpenOneLogin) + navController.navigate( + SavedLoginsFragmentDirections.actionSavedLoginsFragmentToLoginDetailFragment(item.guid) + ) + } + + fun handleLearnMoreClicked() { + browserNavigator.invoke( + SupportUtils.getGenericSumoURLForTopic(SupportUtils.SumoTopic.SYNC_SETUP), + true, + BrowserDirection.FromSavedLoginsFragment + ) + } + + fun handleSort(sortingStrategy: SortingStrategy) { + loginsFragmentStore.dispatch( + LoginsAction.SortLogins( + sortingStrategy + ) + ) + settings.savedLoginsSortingStrategy = sortingStrategy + } +} diff --git a/app/src/main/java/org/mozilla/fenix/settings/logins/controller/SavedLoginsStorageController.kt b/app/src/main/java/org/mozilla/fenix/settings/logins/controller/SavedLoginsStorageController.kt new file mode 100644 index 000000000..7df5dd741 --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/settings/logins/controller/SavedLoginsStorageController.kt @@ -0,0 +1,198 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * 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 org.mozilla.fenix.settings.logins.controller + +import android.content.Context +import android.util.Log +import androidx.navigation.NavController +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Deferred +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.async +import kotlinx.coroutines.launch +import kotlinx.coroutines.withContext +import mozilla.components.concept.storage.Login +import mozilla.components.service.sync.logins.InvalidRecordException +import mozilla.components.service.sync.logins.LoginsStorageException +import mozilla.components.service.sync.logins.NoSuchRecordException +import org.mozilla.fenix.R +import org.mozilla.fenix.ext.components +import org.mozilla.fenix.settings.logins.LoginsAction +import org.mozilla.fenix.settings.logins.LoginsFragmentStore +import org.mozilla.fenix.settings.logins.fragment.EditLoginFragmentDirections +import org.mozilla.fenix.settings.logins.mapToSavedLogin + +/** + * Controller for all saved logins interactions with the password storage component + */ +open class SavedLoginsStorageController( + private val context: Context, + private val viewLifecycleScope: CoroutineScope, + private val navController: NavController, + private val loginsFragmentStore: LoginsFragmentStore +) { + + private suspend fun getLogin(loginId: String): Login? = + context.components.core.passwordsStorage.get(loginId) + + fun delete(loginId: String) { + var deleteLoginJob: Deferred? = null + val deleteJob = viewLifecycleScope.launch(Dispatchers.IO) { + deleteLoginJob = async { + context.components.core.passwordsStorage.delete(loginId) + } + deleteLoginJob?.await() + withContext(Dispatchers.Main) { + navController.popBackStack(R.id.savedLoginsFragment, false) + } + } + deleteJob.invokeOnCompletion { + if (it is CancellationException) { + deleteLoginJob?.cancel() + } + } + } + + fun save(loginId: String, usernameText: String, passwordText: String) { + var saveLoginJob: Deferred? = null + viewLifecycleScope.launch(Dispatchers.IO) { + saveLoginJob = async { + // must retrieve from storage to get the httpsRealm and formActionOrigin + val oldLogin = context.components.core.passwordsStorage.get(loginId) + + // Update requires a Login type, which needs at least one of + // httpRealm or formActionOrigin + val loginToSave = Login( + guid = loginId, + origin = oldLogin?.origin!!, + username = usernameText, // new value + password = passwordText, // new value + httpRealm = oldLogin.httpRealm, + formActionOrigin = oldLogin.formActionOrigin + ) + + save(loginToSave) + syncAndUpdateList(loginToSave) + } + saveLoginJob?.await() + withContext(Dispatchers.Main) { + val directions = + EditLoginFragmentDirections.actionEditLoginFragmentToLoginDetailFragment( + loginId + ) + navController.navigate(directions) + } + } + saveLoginJob?.invokeOnCompletion { + if (it is CancellationException) { + saveLoginJob?.cancel() + } + } + } + + private suspend fun save(loginToSave: Login) { + try { + context.components.core.passwordsStorage.update(loginToSave) + } catch (loginException: LoginsStorageException) { + when (loginException) { + is NoSuchRecordException, + is InvalidRecordException -> { + Log.e("Edit login", + "Failed to save edited login.", loginException) + } + else -> Log.e("Edit login", + "Failed to save edited login.", loginException) + } + } + } + + private fun syncAndUpdateList(updatedLogin: Login) { + val login = updatedLogin.mapToSavedLogin() + loginsFragmentStore.dispatch( + LoginsAction.UpdateLoginsList( + listOf(login) + ) + ) + } + + fun findPotentialDuplicates(loginId: String) { + var deferredLogin: Deferred>? = null + // What scope should be used here? + val fetchLoginJob = viewLifecycleScope.launch(Dispatchers.IO) { + deferredLogin = async { + val login = getLogin(loginId) + context.components.core.passwordsStorage.getPotentialDupesIgnoringUsername(login!!) + } + val fetchedDuplicatesList = deferredLogin?.await() + fetchedDuplicatesList?.let { list -> + withContext(Dispatchers.Main) { + val savedLoginList = list.map { it.mapToSavedLogin() } + loginsFragmentStore.dispatch( + LoginsAction.ListOfDupes( + savedLoginList + ) + ) + } + } + } + fetchLoginJob.invokeOnCompletion { + if (it is CancellationException) { + deferredLogin?.cancel() + } + } + } + + fun fetchLoginDetails(loginId: String) { + var deferredLogin: Deferred>? = null + val fetchLoginJob = viewLifecycleScope.launch(Dispatchers.IO) { + deferredLogin = async { + context.components.core.passwordsStorage.list() + } + val fetchedLoginList = deferredLogin?.await() + + fetchedLoginList?.let { + withContext(Dispatchers.Main) { + val login = fetchedLoginList.filter { + it.guid == loginId + }.first() + loginsFragmentStore.dispatch( + LoginsAction.UpdateCurrentLogin( + login.mapToSavedLogin() + ) + ) + } + } + } + fetchLoginJob.invokeOnCompletion { + if (it is CancellationException) { + deferredLogin?.cancel() + } + } + } + + fun handleLoadAndMapLogins() { + var deferredLogins: Deferred>? = null + val fetchLoginsJob = viewLifecycleScope.launch(Dispatchers.IO) { + deferredLogins = async { + context.components.core.passwordsStorage.list() + } + val logins = deferredLogins?.await() + logins?.let { + withContext(Dispatchers.Main) { + loginsFragmentStore.dispatch( + LoginsAction.UpdateLoginsList( + logins.map { it.mapToSavedLogin() }) + ) + } + } + } + fetchLoginsJob.invokeOnCompletion { + if (it is CancellationException) { + deferredLogins?.cancel() + } + } + } +} diff --git a/app/src/main/java/org/mozilla/fenix/settings/logins/EditLoginFragment.kt b/app/src/main/java/org/mozilla/fenix/settings/logins/fragment/EditLoginFragment.kt similarity index 53% rename from app/src/main/java/org/mozilla/fenix/settings/logins/EditLoginFragment.kt rename to app/src/main/java/org/mozilla/fenix/settings/logins/fragment/EditLoginFragment.kt index 80f54d5b6..15827fdf5 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/logins/EditLoginFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/logins/fragment/EditLoginFragment.kt @@ -2,59 +2,72 @@ * 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 org.mozilla.fenix.settings.logins +package org.mozilla.fenix.settings.logins.fragment import android.os.Bundle import android.text.Editable import android.text.InputType import android.text.TextWatcher -import android.util.Log import android.view.Menu import android.view.MenuInflater import android.view.MenuItem import android.view.View -import androidx.core.content.ContextCompat +import androidx.appcompat.view.menu.ActionMenuItemView import androidx.fragment.app.Fragment import androidx.lifecycle.lifecycleScope import androidx.navigation.fragment.findNavController import androidx.navigation.fragment.navArgs import kotlinx.android.synthetic.main.fragment_edit_login.* -import kotlinx.coroutines.CancellationException -import kotlinx.coroutines.Deferred -import kotlinx.coroutines.Dispatchers.IO -import kotlinx.coroutines.Dispatchers.Main -import kotlinx.coroutines.async -import kotlinx.coroutines.launch -import kotlinx.coroutines.withContext -import mozilla.components.concept.storage.Login -import mozilla.components.service.sync.logins.InvalidRecordException -import mozilla.components.service.sync.logins.LoginsStorageException -import mozilla.components.service.sync.logins.NoSuchRecordException +import kotlinx.android.synthetic.main.fragment_edit_login.view.* +import kotlinx.coroutines.ExperimentalCoroutinesApi +import mozilla.components.lib.state.ext.consumeFrom import mozilla.components.support.ktx.android.view.hideKeyboard import org.mozilla.fenix.R import org.mozilla.fenix.components.StoreProvider import org.mozilla.fenix.components.metrics.Event -import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.redirectToReAuth +import org.mozilla.fenix.ext.requireComponents import org.mozilla.fenix.ext.settings +import org.mozilla.fenix.settings.logins.LoginsAction +import org.mozilla.fenix.settings.logins.LoginsFragmentStore +import org.mozilla.fenix.settings.logins.LoginsListState +import org.mozilla.fenix.settings.logins.SavedLogin +import org.mozilla.fenix.settings.logins.controller.SavedLoginsStorageController +import org.mozilla.fenix.settings.logins.interactor.EditLoginInteractor +import org.mozilla.fenix.settings.logins.view.EditLoginView /** - * Displays the editable saved login information for a single website. + * Displays the editable saved login information for a single website */ +@ExperimentalCoroutinesApi @Suppress("TooManyFunctions", "NestedBlockDepth", "ForbiddenComment") class EditLoginFragment : Fragment(R.layout.fragment_edit_login) { - private val args by navArgs() - private lateinit var savedLoginsStore: LoginsFragmentStore fun String.toEditable(): Editable = Editable.Factory.getInstance().newEditable(this) + private val args by navArgs() + private lateinit var loginsFragmentStore: LoginsFragmentStore + private lateinit var interactor: EditLoginInteractor + private lateinit var editLoginView: EditLoginView private lateinit var oldLogin: SavedLogin - override fun onCreate(savedInstanceState: Bundle?) { - super.onCreate(savedInstanceState) + private var listOfPossibleDupes: List? = null + + private var usernameChanged = false + private var passwordChanged = false + private var saveEnabled = false + private var showPassword = true + + private var validPassword = true + private var validUsername = true + + override fun onViewCreated(view: View, savedInstanceState: Bundle?) { + super.onViewCreated(view, savedInstanceState) setHasOptionsMenu(true) oldLogin = args.savedLoginItem - savedLoginsStore = StoreProvider.get(this) { + editLoginView = EditLoginView(view.editLoginLayout) + + loginsFragmentStore = StoreProvider.get(this) { LoginsFragmentStore( LoginsListState( isLoading = true, @@ -62,31 +75,59 @@ class EditLoginFragment : Fragment(R.layout.fragment_edit_login) { filteredItems = listOf(), searchedForText = null, sortingStrategy = requireContext().settings().savedLoginsSortingStrategy, - highlightedItem = requireContext().settings().savedLoginsMenuHighlightedItem + highlightedItem = requireContext().settings().savedLoginsMenuHighlightedItem, + duplicateLogins = listOf() ) ) } - } - override fun onViewCreated(view: View, savedInstanceState: Bundle?) { - super.onViewCreated(view, savedInstanceState) + interactor = EditLoginInteractor( + SavedLoginsStorageController( + context = requireContext(), + viewLifecycleScope = viewLifecycleOwner.lifecycleScope, + navController = findNavController(), + loginsFragmentStore = loginsFragmentStore + ) + ) - // ensure hostname isn't editable + loginsFragmentStore.dispatch(LoginsAction.UpdateCurrentLogin(args.savedLoginItem)) + interactor.findPotentialDuplicates(args.savedLoginItem.guid) + + // initialize editable values hostnameText.text = args.savedLoginItem.origin.toEditable() - hostnameText.isClickable = false - hostnameText.isFocusable = false - usernameText.text = args.savedLoginItem.username.toEditable() passwordText.text = args.savedLoginItem.password.toEditable() + formatEditableValues() + initSaveState() + setUpClickListeners() + setUpTextListeners() + editLoginView.showPassword() + + consumeFrom(loginsFragmentStore) { + listOfPossibleDupes = loginsFragmentStore.state.duplicateLogins + } + } + + private fun initSaveState() { + saveEnabled = false // don't enable saving until something has been changed + val saveButton = + activity?.findViewById(R.id.save_login_button) + saveButton?.isEnabled = saveEnabled + + usernameChanged = false + passwordChanged = false + } + + private fun formatEditableValues() { + hostnameText.isClickable = false + hostnameText.isFocusable = false + usernameText.inputType = InputType.TYPE_TEXT_FLAG_NO_SUGGESTIONS // TODO: extend PasswordTransformationMethod() to change bullets to asterisks passwordText.inputType = InputType.TYPE_CLASS_TEXT or InputType.TYPE_TEXT_VARIATION_PASSWORD passwordText.compoundDrawablePadding = requireContext().resources .getDimensionPixelOffset(R.dimen.saved_logins_end_icon_drawable_padding) - - setUpClickListeners() - setUpTextListeners() } private fun setUpClickListeners() { @@ -105,14 +146,11 @@ class EditLoginFragment : Fragment(R.layout.fragment_edit_login) { it.isEnabled = false } revealPasswordButton.setOnClickListener { - togglePasswordReveal() - } - - var firstClick = true - passwordText.setOnClickListener { - if (firstClick) { - togglePasswordReveal() - firstClick = false + showPassword = !showPassword + if (showPassword) { + editLoginView.showPassword() + } else { + editLoginView.hidePassword() } } } @@ -124,7 +162,6 @@ class EditLoginFragment : Fragment(R.layout.fragment_edit_login) { view?.hideKeyboard() } } - editLoginLayout.onFocusChangeListener = View.OnFocusChangeListener { _, hasFocus -> if (!hasFocus) { view?.hideKeyboard() @@ -133,13 +170,20 @@ class EditLoginFragment : Fragment(R.layout.fragment_edit_login) { usernameText.addTextChangedListener(object : TextWatcher { override fun afterTextChanged(u: Editable?) { - if (u.toString() == oldLogin.username) { - inputLayoutUsername.error = null - inputLayoutUsername.errorIconDrawable = null - } else { - clearUsernameTextButton.isEnabled = true - // setDupeError() TODO in #10173 + when { + u.toString() == oldLogin.username -> { + usernameChanged = false + validUsername = true + inputLayoutUsername.error = null + inputLayoutUsername.errorIconDrawable = null + } + else -> { + usernameChanged = true + clearUsernameTextButton.isEnabled = true + setDupeError() + } } + setSaveButtonState() } override fun beforeTextChanged(u: CharSequence?, start: Int, count: Int, after: Int) { @@ -155,20 +199,26 @@ class EditLoginFragment : Fragment(R.layout.fragment_edit_login) { override fun afterTextChanged(p: Editable?) { when { p.toString().isEmpty() -> { + passwordChanged = true clearPasswordTextButton.isEnabled = false setPasswordError() } p.toString() == oldLogin.password -> { + passwordChanged = false + validPassword = true inputLayoutPassword.error = null inputLayoutPassword.errorIconDrawable = null clearPasswordTextButton.isEnabled = true } else -> { + passwordChanged = true + validPassword = true inputLayoutPassword.error = null inputLayoutPassword.errorIconDrawable = null clearPasswordTextButton.isEnabled = true } } + setSaveButtonState() } override fun beforeTextChanged(p: CharSequence?, start: Int, count: Int, after: Int) { @@ -181,14 +231,40 @@ class EditLoginFragment : Fragment(R.layout.fragment_edit_login) { }) } + private fun isDupe(username: String): Boolean = + loginsFragmentStore.state.duplicateLogins.filter { it.username == username }.any() + + private fun setDupeError() { + if (isDupe(usernameText.text.toString())) { + inputLayoutUsername?.let { + usernameChanged = true + validUsername = false + it.setErrorIconDrawable(R.drawable.mozac_ic_warning) + it.error = context?.getString(R.string.saved_login_duplicate) + } + } else { + usernameChanged = true + validUsername = true + inputLayoutUsername.error = null + } + } + private fun setPasswordError() { inputLayoutPassword?.let { layout -> + validPassword = false layout.error = context?.getString(R.string.saved_login_password_required) layout.setErrorIconDrawable(R.drawable.mozac_ic_warning) + } + } - layout.errorIconDrawable?.setTint( - ContextCompat.getColor(requireContext(), R.color.design_default_color_error) - ) + private fun setSaveButtonState() { + val saveButton = activity?.findViewById(R.id.save_login_button) + val changesMadeWithNoErrors = + validUsername && validPassword && (usernameChanged || passwordChanged) + + changesMadeWithNoErrors.let { + saveButton?.isEnabled = it + saveEnabled = it } } @@ -207,101 +283,16 @@ class EditLoginFragment : Fragment(R.layout.fragment_edit_login) { override fun onOptionsItemSelected(item: MenuItem): Boolean = when (item.itemId) { R.id.save_login_button -> { view?.hideKeyboard() - if (!passwordText.text.isNullOrBlank()) { - try { - attemptSaveAndExit() - } catch (loginException: LoginsStorageException) { - when (loginException) { - is NoSuchRecordException, - is InvalidRecordException -> { - Log.e( - "Edit login", - "Failed to save edited login.", - loginException - ) - } - else -> Log.e( - "Edit login", - "Failed to save edited login.", - loginException - ) - } - } + if (saveEnabled) { + interactor.onSaveLogin( + args.savedLoginItem.guid, + usernameText.text.toString(), + passwordText.text.toString() + ) + requireComponents.analytics.metrics.track(Event.EditLoginSave) } true } else -> false } - - // TODO: Move interactions with the component's password storage into a separate datastore - // This includes Delete, Update/Edit, Create - private fun attemptSaveAndExit() { - var saveLoginJob: Deferred? = null - viewLifecycleOwner.lifecycleScope.launch(IO) { - saveLoginJob = async { - val oldLogin = - requireContext().components.core.passwordsStorage.get(args.savedLoginItem.guid) - - // Update requires a Login type, which needs at least one of - // httpRealm or formActionOrigin - val loginToSave = Login( - guid = oldLogin?.guid, - origin = oldLogin?.origin!!, - username = usernameText.text.toString(), // new value - password = passwordText.text.toString(), // new value - httpRealm = oldLogin.httpRealm, - formActionOrigin = oldLogin.formActionOrigin - ) - - save(loginToSave) - syncAndUpdateList(loginToSave) - } - saveLoginJob?.await() - withContext(Main) { - val directions = - EditLoginFragmentDirections - .actionEditLoginFragmentToLoginDetailFragment(args.savedLoginItem.guid) - findNavController().navigate(directions) - } - } - saveLoginJob?.invokeOnCompletion { - if (it is CancellationException) { - saveLoginJob?.cancel() - } - } - } - - private suspend fun save(loginToSave: Login) = - requireContext().components.core.passwordsStorage.update(loginToSave) - - private fun syncAndUpdateList(updatedLogin: Login) { - val login = updatedLogin.mapToSavedLogin() - savedLoginsStore.dispatch(LoginsAction.UpdateLoginsList(listOf(login))) - } - - // TODO: create helper class for toggling passwords. Used in login info and edit fragments. - private fun togglePasswordReveal() { - val currText = passwordText.text - if (passwordText.inputType == InputType.TYPE_TEXT_VARIATION_PASSWORD - or InputType.TYPE_CLASS_TEXT - ) { - context?.components?.analytics?.metrics?.track(Event.ViewLoginPassword) - passwordText.inputType = InputType.TYPE_TEXT_VARIATION_VISIBLE_PASSWORD - revealPasswordButton.setImageDrawable( - resources.getDrawable(R.drawable.mozac_ic_password_hide, null) - ) - revealPasswordButton.contentDescription = - resources.getString(R.string.saved_login_hide_password) - } else { - passwordText.inputType = - InputType.TYPE_CLASS_TEXT or InputType.TYPE_TEXT_VARIATION_PASSWORD - revealPasswordButton.setImageDrawable( - resources.getDrawable(R.drawable.mozac_ic_password_reveal, null) - ) - revealPasswordButton.contentDescription = - context?.getString(R.string.saved_login_reveal_password) - } - // For the new type to take effect you need to reset the text to it's current edited version - passwordText?.text = currText - } } diff --git a/app/src/main/java/org/mozilla/fenix/settings/logins/LoginDetailFragment.kt b/app/src/main/java/org/mozilla/fenix/settings/logins/fragment/LoginDetailFragment.kt similarity index 66% rename from app/src/main/java/org/mozilla/fenix/settings/logins/LoginDetailFragment.kt rename to app/src/main/java/org/mozilla/fenix/settings/logins/fragment/LoginDetailFragment.kt index f7c842978..501e26763 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/logins/LoginDetailFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/logins/fragment/LoginDetailFragment.kt @@ -2,7 +2,7 @@ * 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 org.mozilla.fenix.settings.logins +package org.mozilla.fenix.settings.logins.fragment import android.content.DialogInterface import android.os.Bundle @@ -21,16 +21,8 @@ import androidx.navigation.fragment.findNavController import androidx.navigation.fragment.navArgs import com.google.android.material.snackbar.Snackbar import kotlinx.android.synthetic.main.fragment_login_detail.* -import kotlinx.coroutines.CancellationException -import kotlinx.coroutines.Deferred -import kotlinx.coroutines.Dispatchers.IO -import kotlinx.coroutines.Dispatchers.Main import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.ObsoleteCoroutinesApi -import kotlinx.coroutines.async -import kotlinx.coroutines.launch -import kotlinx.coroutines.withContext -import mozilla.components.concept.storage.Login import mozilla.components.lib.state.ext.consumeFrom import org.mozilla.fenix.BrowserDirection import org.mozilla.fenix.FeatureFlags @@ -42,9 +34,16 @@ import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.increaseTapArea import org.mozilla.fenix.ext.redirectToReAuth +import org.mozilla.fenix.ext.requireComponents import org.mozilla.fenix.ext.settings import org.mozilla.fenix.ext.showToolbar -import org.mozilla.fenix.ext.urlToTrimmedHost +import org.mozilla.fenix.ext.simplifiedUrl +import org.mozilla.fenix.settings.logins.LoginsFragmentStore +import org.mozilla.fenix.settings.logins.LoginsListState +import org.mozilla.fenix.settings.logins.SavedLogin +import org.mozilla.fenix.settings.logins.controller.SavedLoginsStorageController +import org.mozilla.fenix.settings.logins.interactor.LoginDetailInteractor +import org.mozilla.fenix.settings.logins.view.LoginDetailView /** * Displays saved login information for a single website. @@ -57,8 +56,10 @@ class LoginDetailFragment : Fragment(R.layout.fragment_login_detail) { private var login: SavedLogin? = null private lateinit var savedLoginsStore: LoginsFragmentStore private lateinit var loginDetailView: LoginDetailView + private lateinit var interactor: LoginDetailInteractor private lateinit var menu: Menu private var deleteDialog: AlertDialog? = null + private var showPassword = true override fun onCreateView( inflater: LayoutInflater, @@ -74,12 +75,14 @@ class LoginDetailFragment : Fragment(R.layout.fragment_login_detail) { filteredItems = listOf(), searchedForText = null, sortingStrategy = requireContext().settings().savedLoginsSortingStrategy, - highlightedItem = requireContext().settings().savedLoginsMenuHighlightedItem + highlightedItem = requireContext().settings().savedLoginsMenuHighlightedItem, + duplicateLogins = listOf() // assume on load there are no dupes ) ) } - loginDetailView = LoginDetailView(view?.findViewById(R.id.loginDetailLayout)) - fetchLoginDetails() + loginDetailView = LoginDetailView( + view.findViewById(R.id.loginDetailLayout) + ) return view } @@ -87,16 +90,29 @@ class LoginDetailFragment : Fragment(R.layout.fragment_login_detail) { @ObsoleteCoroutinesApi @ExperimentalCoroutinesApi override fun onViewCreated(view: View, savedInstanceState: Bundle?) { + super.onViewCreated(view, savedInstanceState) + + interactor = LoginDetailInteractor( + SavedLoginsStorageController( + context = requireContext(), + viewLifecycleScope = viewLifecycleOwner.lifecycleScope, + navController = findNavController(), + loginsFragmentStore = savedLoginsStore + ) + ) + interactor.onFetchLoginList(args.savedLoginId) + consumeFrom(savedLoginsStore) { loginDetailView.update(it) login = savedLoginsStore.state.currentItem setUpCopyButtons() showToolbar( - savedLoginsStore.state.currentItem?.origin?.urlToTrimmedHost(requireContext()) + savedLoginsStore.state.currentItem?.origin?.simplifiedUrl() ?: "" ) setUpPasswordReveal() } + loginDetailView.togglePasswordReveal(showPassword) } override fun onCreate(savedInstanceState: Bundle?) { @@ -124,7 +140,11 @@ class LoginDetailFragment : Fragment(R.layout.fragment_login_detail) { InputType.TYPE_CLASS_TEXT or InputType.TYPE_TEXT_VARIATION_PASSWORD revealPasswordButton.increaseTapArea(BUTTON_INCREASE_DPS) revealPasswordButton.setOnClickListener { - togglePasswordReveal() + showPassword = !showPassword + loginDetailView.togglePasswordReveal(!showPassword) + } + passwordText.setOnClickListener { + loginDetailView.togglePasswordReveal(!showPassword) } } @@ -149,33 +169,6 @@ class LoginDetailFragment : Fragment(R.layout.fragment_login_detail) { ) } - // TODO: Move interactions with the component's password storage into a separate datastore - private fun fetchLoginDetails() { - var deferredLogin: Deferred>? = null - val fetchLoginJob = viewLifecycleOwner.lifecycleScope.launch(IO) { - deferredLogin = async { - requireContext().components.core.passwordsStorage.list() - } - val fetchedLoginList = deferredLogin?.await() - - fetchedLoginList?.let { - withContext(Main) { - val login = fetchedLoginList.filter { - it.guid == args.savedLoginId - }.first() - savedLoginsStore.dispatch( - LoginsAction.UpdateCurrentLogin(login.mapToSavedLogin()) - ) - } - } - } - fetchLoginJob.invokeOnCompletion { - if (it is CancellationException) { - deferredLogin?.cancel() - } - } - } - override fun onCreateOptionsMenu(menu: Menu, inflater: MenuInflater) { if (FeatureFlags.loginsEdit) { inflater.inflate(R.menu.login_options_menu, menu) @@ -206,9 +199,11 @@ class LoginDetailFragment : Fragment(R.layout.fragment_login_detail) { } private fun editLogin() { + requireComponents.analytics.metrics.track(Event.EditLogin) val directions = - LoginDetailFragmentDirections - .actionLoginDetailFragmentToEditLoginFragment(login!!) + LoginDetailFragmentDirections.actionLoginDetailFragmentToEditLoginFragment( + login!! + ) findNavController().navigate(directions) } @@ -220,7 +215,8 @@ class LoginDetailFragment : Fragment(R.layout.fragment_login_detail) { dialog.cancel() } setPositiveButton(R.string.dialog_delete_positive) { dialog: DialogInterface, _ -> - deleteLogin() + requireComponents.analytics.metrics.track(Event.DeleteLogin) + interactor.onDeleteLogin(args.savedLoginId) dialog.dismiss() } create() @@ -228,49 +224,6 @@ class LoginDetailFragment : Fragment(R.layout.fragment_login_detail) { } } - // TODO: Move interactions with the component's password storage into a separate datastore - // This includes Delete, Update/Edit, Create - private fun deleteLogin() { - var deleteLoginJob: Deferred? = null - val deleteJob = viewLifecycleOwner.lifecycleScope.launch(IO) { - deleteLoginJob = async { - requireContext().components.core.passwordsStorage.delete(args.savedLoginId) - } - deleteLoginJob?.await() - withContext(Main) { - findNavController().popBackStack(R.id.savedLoginsFragment, false) - } - } - deleteJob.invokeOnCompletion { - if (it is CancellationException) { - deleteLoginJob?.cancel() - } - } - } - - // TODO: create helper class for toggling passwords. Used in login info and edit fragments. - private fun togglePasswordReveal() { - if (passwordText.inputType == InputType.TYPE_TEXT_VARIATION_PASSWORD or InputType.TYPE_CLASS_TEXT) { - context?.components?.analytics?.metrics?.track(Event.ViewLoginPassword) - passwordText.inputType = InputType.TYPE_TEXT_VARIATION_VISIBLE_PASSWORD - revealPasswordButton.setImageDrawable( - resources.getDrawable(R.drawable.mozac_ic_password_hide, null) - ) - revealPasswordButton.contentDescription = - resources.getString(R.string.saved_login_hide_password) - } else { - passwordText.inputType = - InputType.TYPE_CLASS_TEXT or InputType.TYPE_TEXT_VARIATION_PASSWORD - revealPasswordButton.setImageDrawable( - resources.getDrawable(R.drawable.mozac_ic_password_reveal, null) - ) - revealPasswordButton.contentDescription = - context?.getString(R.string.saved_login_reveal_password) - } - // For the new type to take effect you need to reset the text - passwordText.text = login?.password - } - /** * Click listener for a textview's copy button. * @param value Value to be copied diff --git a/app/src/main/java/org/mozilla/fenix/settings/logins/SavedLoginsAuthFragment.kt b/app/src/main/java/org/mozilla/fenix/settings/logins/fragment/SavedLoginsAuthFragment.kt similarity index 98% rename from app/src/main/java/org/mozilla/fenix/settings/logins/SavedLoginsAuthFragment.kt rename to app/src/main/java/org/mozilla/fenix/settings/logins/fragment/SavedLoginsAuthFragment.kt index d8051e7f3..63fc052e0 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/logins/SavedLoginsAuthFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/logins/fragment/SavedLoginsAuthFragment.kt @@ -2,7 +2,7 @@ * 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 org.mozilla.fenix.settings.logins +package org.mozilla.fenix.settings.logins.fragment import android.annotation.TargetApi import android.app.Activity.RESULT_OK @@ -313,7 +313,8 @@ class SavedLoginsAuthFragment : PreferenceFragmentCompat(), AccountObserver { } private fun navigateToAccountProblemFragment() { - val directions = SavedLoginsAuthFragmentDirections.actionGlobalAccountProblemFragment() + val directions = + SavedLoginsAuthFragmentDirections.actionGlobalAccountProblemFragment() findNavController().navigate(directions) } diff --git a/app/src/main/java/org/mozilla/fenix/settings/logins/SavedLoginsFragment.kt b/app/src/main/java/org/mozilla/fenix/settings/logins/fragment/SavedLoginsFragment.kt similarity index 66% rename from app/src/main/java/org/mozilla/fenix/settings/logins/SavedLoginsFragment.kt rename to app/src/main/java/org/mozilla/fenix/settings/logins/fragment/SavedLoginsFragment.kt index cec4c1c29..16a4a8974 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/logins/SavedLoginsFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/logins/fragment/SavedLoginsFragment.kt @@ -2,7 +2,7 @@ * 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 org.mozilla.fenix.settings.logins +package org.mozilla.fenix.settings.logins.fragment import android.os.Bundle import android.view.LayoutInflater @@ -21,17 +21,9 @@ import androidx.fragment.app.Fragment import androidx.lifecycle.lifecycleScope import androidx.navigation.fragment.findNavController import kotlinx.android.synthetic.main.fragment_saved_logins.view.* -import kotlinx.coroutines.Deferred -import kotlinx.coroutines.Dispatchers.IO -import kotlinx.coroutines.Dispatchers.Main -import kotlinx.coroutines.async -import kotlinx.coroutines.launch -import kotlinx.coroutines.withContext -import kotlinx.coroutines.CancellationException import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.ObsoleteCoroutinesApi import mozilla.components.browser.menu.BrowserMenu -import mozilla.components.concept.storage.Login import mozilla.components.lib.state.ext.consumeFrom import org.mozilla.fenix.BrowserDirection import org.mozilla.fenix.HomeActivity @@ -41,17 +33,28 @@ import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.redirectToReAuth import org.mozilla.fenix.ext.settings import org.mozilla.fenix.ext.showToolbar +import org.mozilla.fenix.settings.logins.LoginsAction +import org.mozilla.fenix.settings.logins.LoginsFragmentStore +import org.mozilla.fenix.settings.logins.controller.LoginsListController +import org.mozilla.fenix.settings.logins.LoginsListState +import org.mozilla.fenix.settings.logins.interactor.SavedLoginsInteractor +import org.mozilla.fenix.settings.logins.SavedLoginsSortingStrategyMenu +import org.mozilla.fenix.settings.logins.view.SavedLoginsListView +import org.mozilla.fenix.settings.logins.SortingStrategy +import org.mozilla.fenix.settings.logins.controller.SavedLoginsStorageController @SuppressWarnings("TooManyFunctions") class SavedLoginsFragment : Fragment() { private lateinit var savedLoginsStore: LoginsFragmentStore - private lateinit var savedLoginsView: SavedLoginsView + private lateinit var savedLoginsListView: SavedLoginsListView private lateinit var savedLoginsInteractor: SavedLoginsInteractor private lateinit var dropDownMenuAnchorView: View private lateinit var sortingStrategyMenu: SavedLoginsSortingStrategyMenu private lateinit var sortingStrategyPopupMenu: BrowserMenu private lateinit var toolbarChildContainer: FrameLayout private lateinit var sortLoginsMenuRoot: ConstraintLayout + private lateinit var loginsListController: LoginsListController + private lateinit var savedLoginsStorageController: SavedLoginsStorageController override fun onResume() { super.onResume() @@ -81,21 +84,39 @@ class SavedLoginsFragment : Fragment() { filteredItems = listOf(), searchedForText = null, sortingStrategy = requireContext().settings().savedLoginsSortingStrategy, - highlightedItem = requireContext().settings().savedLoginsMenuHighlightedItem + highlightedItem = requireContext().settings().savedLoginsMenuHighlightedItem, + duplicateLogins = listOf() // assume on load there are no dupes ) ) } - val savedLoginsController: SavedLoginsController = - SavedLoginsController( - store = savedLoginsStore, - navController = findNavController(), - browserNavigator = ::openToBrowserAndLoad, - settings = requireContext().settings(), - metrics = requireContext().components.analytics.metrics + + loginsListController = + LoginsListController( + loginsFragmentStore = savedLoginsStore, + navController = findNavController(), + browserNavigator = ::openToBrowserAndLoad, + settings = requireContext().settings(), + metrics = requireContext().components.analytics.metrics ) - savedLoginsInteractor = SavedLoginsInteractor(savedLoginsController) - savedLoginsView = SavedLoginsView(view.savedLoginsLayout, savedLoginsInteractor) - loadAndMapLogins() + savedLoginsStorageController = + SavedLoginsStorageController( + context = requireContext(), + viewLifecycleScope = viewLifecycleOwner.lifecycleScope, + navController = findNavController(), + loginsFragmentStore = savedLoginsStore + ) + + savedLoginsInteractor = + SavedLoginsInteractor( + loginsListController, + savedLoginsStorageController + ) + + savedLoginsListView = SavedLoginsListView( + view.savedLoginsLayout, + savedLoginsInteractor + ) + savedLoginsInteractor.loadAndMapLogins() return view } @@ -105,7 +126,7 @@ class SavedLoginsFragment : Fragment() { super.onViewCreated(view, savedInstanceState) consumeFrom(savedLoginsStore) { sortingStrategyMenu.updateMenu(savedLoginsStore.state.highlightedItem) - savedLoginsView.update(it) + savedLoginsListView.update(it) } } @@ -122,7 +143,11 @@ class SavedLoginsFragment : Fragment() { } override fun onQueryTextChange(newText: String?): Boolean { - savedLoginsStore.dispatch(LoginsAction.FilterLogins(newText)) + savedLoginsStore.dispatch( + LoginsAction.FilterLogins( + newText + ) + ) return false } }) @@ -141,31 +166,11 @@ class SavedLoginsFragment : Fragment() { super.onPause() } - private fun openToBrowserAndLoad(searchTermOrURL: String, newTab: Boolean, from: BrowserDirection) { - (activity as HomeActivity).openToBrowserAndLoad(searchTermOrURL, newTab, from) - } - - private fun loadAndMapLogins() { - var deferredLogins: Deferred>? = null - val fetchLoginsJob = viewLifecycleOwner.lifecycleScope.launch(IO) { - deferredLogins = async { - requireContext().components.core.passwordsStorage.list() - } - val logins = deferredLogins?.await() - logins?.let { - withContext(Main) { - savedLoginsStore.dispatch( - LoginsAction.UpdateLoginsList(logins.map { it.mapToSavedLogin() }) - ) - } - } - } - fetchLoginsJob.invokeOnCompletion { - if (it is CancellationException) { - deferredLogins?.cancel() - } - } - } + private fun openToBrowserAndLoad( + searchTermOrURL: String, + newTab: Boolean, + from: BrowserDirection + ) = (activity as HomeActivity).openToBrowserAndLoad(searchTermOrURL, newTab, from) private fun initToolbar() { showToolbar(getString(R.string.preferences_passwords_saved_logins)) @@ -175,8 +180,12 @@ class SavedLoginsFragment : Fragment() { sortLoginsMenuRoot = inflateSortLoginsMenuRoot() dropDownMenuAnchorView = sortLoginsMenuRoot.findViewById(R.id.drop_down_menu_anchor_view) when (requireContext().settings().savedLoginsSortingStrategy) { - is SortingStrategy.Alphabetically -> setupMenu(SavedLoginsSortingStrategyMenu.Item.AlphabeticallySort) - is SortingStrategy.LastUsed -> setupMenu(SavedLoginsSortingStrategyMenu.Item.LastUsedSort) + is SortingStrategy.Alphabetically -> setupMenu( + SavedLoginsSortingStrategyMenu.Item.AlphabeticallySort + ) + is SortingStrategy.LastUsed -> setupMenu( + SavedLoginsSortingStrategyMenu.Item.LastUsedSort + ) } } @@ -210,21 +219,29 @@ class SavedLoginsFragment : Fragment() { } private fun setupMenu(itemToHighlight: SavedLoginsSortingStrategyMenu.Item) { - sortingStrategyMenu = SavedLoginsSortingStrategyMenu(requireContext(), itemToHighlight) { - when (it) { - SavedLoginsSortingStrategyMenu.Item.AlphabeticallySort -> { - savedLoginsInteractor.onSortingStrategyChanged( - SortingStrategy.Alphabetically(requireContext().applicationContext) - ) - } + sortingStrategyMenu = + SavedLoginsSortingStrategyMenu( + requireContext(), + itemToHighlight + ) { + when (it) { + SavedLoginsSortingStrategyMenu.Item.AlphabeticallySort -> { + savedLoginsInteractor.onSortingStrategyChanged( + SortingStrategy.Alphabetically( + requireContext().applicationContext + ) + ) + } - SavedLoginsSortingStrategyMenu.Item.LastUsedSort -> { - savedLoginsInteractor.onSortingStrategyChanged( - SortingStrategy.LastUsed(requireContext().applicationContext) - ) + SavedLoginsSortingStrategyMenu.Item.LastUsedSort -> { + savedLoginsInteractor.onSortingStrategyChanged( + SortingStrategy.LastUsed( + requireContext().applicationContext + ) + ) + } } } - } attachMenu() } diff --git a/app/src/main/java/org/mozilla/fenix/settings/logins/SavedLoginsSettingFragment.kt b/app/src/main/java/org/mozilla/fenix/settings/logins/fragment/SavedLoginsSettingFragment.kt similarity index 98% rename from app/src/main/java/org/mozilla/fenix/settings/logins/SavedLoginsSettingFragment.kt rename to app/src/main/java/org/mozilla/fenix/settings/logins/fragment/SavedLoginsSettingFragment.kt index f044da1b4..b781e0ee3 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/logins/SavedLoginsSettingFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/logins/fragment/SavedLoginsSettingFragment.kt @@ -2,7 +2,7 @@ * 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 org.mozilla.fenix.settings.logins +package org.mozilla.fenix.settings.logins.fragment import android.os.Bundle import androidx.preference.Preference diff --git a/app/src/main/java/org/mozilla/fenix/settings/logins/interactor/EditLoginInteractor.kt b/app/src/main/java/org/mozilla/fenix/settings/logins/interactor/EditLoginInteractor.kt new file mode 100644 index 000000000..3039dd693 --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/settings/logins/interactor/EditLoginInteractor.kt @@ -0,0 +1,24 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * 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 org.mozilla.fenix.settings.logins.interactor + +import org.mozilla.fenix.settings.logins.controller.SavedLoginsStorageController + +/** + * Interactor for the edit login screen + * + * @property savedLoginsController controller for the saved logins storage + */ +class EditLoginInteractor( + private val savedLoginsController: SavedLoginsStorageController +) { + fun findPotentialDuplicates(loginId: String) { + savedLoginsController.findPotentialDuplicates(loginId) + } + + fun onSaveLogin(loginId: String, usernameText: String, passwordText: String) { + savedLoginsController.save(loginId, usernameText, passwordText) + } +} diff --git a/app/src/main/java/org/mozilla/fenix/settings/logins/interactor/LoginDetailInteractor.kt b/app/src/main/java/org/mozilla/fenix/settings/logins/interactor/LoginDetailInteractor.kt new file mode 100644 index 000000000..7d8f6cb9c --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/settings/logins/interactor/LoginDetailInteractor.kt @@ -0,0 +1,24 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * 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 org.mozilla.fenix.settings.logins.interactor + +import org.mozilla.fenix.settings.logins.controller.SavedLoginsStorageController + +/** + * Interactor for the login detail screen + * + * @property savedLoginsController controller for the saved logins storage + */ +class LoginDetailInteractor( + private val savedLoginsController: SavedLoginsStorageController +) { + fun onFetchLoginList(loginId: String) { + savedLoginsController.fetchLoginDetails(loginId) + } + + fun onDeleteLogin(loginId: String) { + savedLoginsController.delete(loginId) + } +} diff --git a/app/src/main/java/org/mozilla/fenix/settings/logins/interactor/SavedLoginsInteractor.kt b/app/src/main/java/org/mozilla/fenix/settings/logins/interactor/SavedLoginsInteractor.kt new file mode 100644 index 000000000..5bf69a99b --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/settings/logins/interactor/SavedLoginsInteractor.kt @@ -0,0 +1,39 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * 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 org.mozilla.fenix.settings.logins.interactor + +import org.mozilla.fenix.settings.logins.SavedLogin +import org.mozilla.fenix.settings.logins.SortingStrategy +import org.mozilla.fenix.settings.logins.controller.LoginsListController +import org.mozilla.fenix.settings.logins.controller.SavedLoginsStorageController + +/** + * Interactor for the saved logins screen + * + * @param loginsListController [LoginsListController] which will be delegated for all + * user interactions. + * @param savedLoginsStorageController [SavedLoginsStorageController] which will be delegated + * for all calls to the password storage component + */ +class SavedLoginsInteractor( + private val loginsListController: LoginsListController, + private val savedLoginsStorageController: SavedLoginsStorageController +) { + fun onItemClicked(item: SavedLogin) { + loginsListController.handleItemClicked(item) + } + + fun onLearnMoreClicked() { + loginsListController.handleLearnMoreClicked() + } + + fun onSortingStrategyChanged(sortingStrategy: SortingStrategy) { + loginsListController.handleSort(sortingStrategy) + } + + fun loadAndMapLogins() { + savedLoginsStorageController.handleLoadAndMapLogins() + } +} diff --git a/app/src/main/java/org/mozilla/fenix/settings/logins/view/EditLoginView.kt b/app/src/main/java/org/mozilla/fenix/settings/logins/view/EditLoginView.kt new file mode 100644 index 000000000..cdc1c8a9b --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/settings/logins/view/EditLoginView.kt @@ -0,0 +1,54 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * 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 org.mozilla.fenix.settings.logins.view + +import android.text.InputType +import android.view.ViewGroup +import androidx.appcompat.content.res.AppCompatResources +import kotlinx.android.extensions.LayoutContainer +import kotlinx.android.synthetic.main.fragment_edit_login.view.* +import org.mozilla.fenix.R +import org.mozilla.fenix.components.metrics.Event +import org.mozilla.fenix.ext.components + +/** + * View that contains and configures the Edit Login screen + */ +@Suppress("ForbiddenComment") +class EditLoginView( + override val containerView: ViewGroup +) : LayoutContainer { + private val context = containerView.context + + // TODO: create helper class for toggling passwords. https://github.com/mozilla-mobile/fenix/issues/12554 + fun showPassword() { + val currText = containerView.passwordText?.text + context.components.analytics.metrics.track(Event.ViewLoginPassword) + containerView.passwordText?.inputType = + InputType.TYPE_TEXT_VARIATION_VISIBLE_PASSWORD + containerView.revealPasswordButton?.setImageDrawable( + AppCompatResources.getDrawable(context, R.drawable.mozac_ic_password_hide) + ) + containerView.revealPasswordButton?.contentDescription = + context.resources.getString(R.string.saved_login_hide_password) + + // For the new type to take effect you need to reset the text to it's current edited version + containerView.passwordText?.text = currText + } + + fun hidePassword() { + val currText = containerView.passwordText?.text + containerView.passwordText?.inputType = + InputType.TYPE_CLASS_TEXT or InputType.TYPE_TEXT_VARIATION_PASSWORD + containerView.revealPasswordButton?.setImageDrawable( + AppCompatResources.getDrawable(context, R.drawable.mozac_ic_password_reveal) + ) + containerView.revealPasswordButton?.contentDescription = + context.getString(R.string.saved_login_reveal_password) + + // For the new type to take effect you need to reset the text to it's current edited version + containerView.passwordText?.text = currText + } +} diff --git a/app/src/main/java/org/mozilla/fenix/settings/logins/view/LoginDetailView.kt b/app/src/main/java/org/mozilla/fenix/settings/logins/view/LoginDetailView.kt new file mode 100644 index 000000000..6276772f6 --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/settings/logins/view/LoginDetailView.kt @@ -0,0 +1,65 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * 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 org.mozilla.fenix.settings.logins.view + +import android.text.InputType +import android.view.ViewGroup +import androidx.core.content.res.ResourcesCompat +import kotlinx.android.extensions.LayoutContainer +import kotlinx.android.synthetic.main.fragment_login_detail.* +import kotlinx.android.synthetic.main.fragment_login_detail.view.* +import org.mozilla.fenix.R +import org.mozilla.fenix.components.metrics.Event +import org.mozilla.fenix.ext.components +import org.mozilla.fenix.settings.logins.LoginsListState + +/** + * View that contains and configures the Login Details + */ +@Suppress("ForbiddenComment") +class LoginDetailView(override val containerView: ViewGroup) : LayoutContainer { + private val context = containerView.context + + fun update(login: LoginsListState) { + webAddressText.text = login.currentItem?.origin + usernameText.text = login.currentItem?.username + passwordText.text = login.currentItem?.password + } + + fun togglePasswordReveal(show: Boolean) { + if (show) showPassword() else { hidePassword() } + } + + // TODO: create helper class for toggling passwords. https://github.com/mozilla-mobile/fenix/issues/12554 + fun showPassword() { + if (passwordText.inputType == InputType.TYPE_TEXT_VARIATION_PASSWORD or InputType.TYPE_CLASS_TEXT) { + context?.components?.analytics?.metrics?.track(Event.ViewLoginPassword) + passwordText.inputType = InputType.TYPE_TEXT_VARIATION_VISIBLE_PASSWORD + revealPasswordButton.setImageDrawable( + ResourcesCompat.getDrawable( + context.resources, + R.drawable.mozac_ic_password_hide, null + ) + ) + revealPasswordButton.contentDescription = + context.resources.getString(R.string.saved_login_hide_password) + } + // For the new type to take effect you need to reset the text + passwordText.text = containerView.passwordText.editableText + } + + fun hidePassword() { + passwordText.inputType = + InputType.TYPE_CLASS_TEXT or InputType.TYPE_TEXT_VARIATION_PASSWORD + revealPasswordButton.setImageDrawable( + ResourcesCompat.getDrawable(context.resources, + R.drawable.mozac_ic_password_reveal, null) + ) + revealPasswordButton.contentDescription = + context.getString(R.string.saved_login_reveal_password) + // For the new type to take effect you need to reset the text + passwordText.text = containerView.passwordText.editableText + } +} diff --git a/app/src/main/java/org/mozilla/fenix/settings/logins/LoginsAdapter.kt b/app/src/main/java/org/mozilla/fenix/settings/logins/view/LoginsAdapter.kt similarity index 88% rename from app/src/main/java/org/mozilla/fenix/settings/logins/LoginsAdapter.kt rename to app/src/main/java/org/mozilla/fenix/settings/logins/view/LoginsAdapter.kt index 40e1d39ca..b5801ff9d 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/logins/LoginsAdapter.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/logins/view/LoginsAdapter.kt @@ -2,13 +2,15 @@ * 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 org.mozilla.fenix.settings.logins +package org.mozilla.fenix.settings.logins.view import android.view.LayoutInflater import android.view.ViewGroup import androidx.recyclerview.widget.DiffUtil import androidx.recyclerview.widget.ListAdapter import org.mozilla.fenix.R +import org.mozilla.fenix.settings.logins.SavedLogin +import org.mozilla.fenix.settings.logins.interactor.SavedLoginsInteractor class LoginsAdapter( private val interactor: SavedLoginsInteractor diff --git a/app/src/main/java/org/mozilla/fenix/settings/logins/LoginsListViewHolder.kt b/app/src/main/java/org/mozilla/fenix/settings/logins/view/LoginsListViewHolder.kt similarity index 88% rename from app/src/main/java/org/mozilla/fenix/settings/logins/LoginsListViewHolder.kt rename to app/src/main/java/org/mozilla/fenix/settings/logins/view/LoginsListViewHolder.kt index f7bd68faa..b5cd79943 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/logins/LoginsListViewHolder.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/logins/view/LoginsListViewHolder.kt @@ -2,13 +2,15 @@ * 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 org.mozilla.fenix.settings.logins +package org.mozilla.fenix.settings.logins.view import android.view.View import androidx.recyclerview.widget.RecyclerView import kotlinx.android.synthetic.main.logins_item.view.* import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.loadIntoView +import org.mozilla.fenix.settings.logins.SavedLogin +import org.mozilla.fenix.settings.logins.interactor.SavedLoginsInteractor class LoginsListViewHolder( private val view: View, diff --git a/app/src/main/java/org/mozilla/fenix/settings/logins/view/SavedLoginsListView.kt b/app/src/main/java/org/mozilla/fenix/settings/logins/view/SavedLoginsListView.kt new file mode 100644 index 000000000..400d3442f --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/settings/logins/view/SavedLoginsListView.kt @@ -0,0 +1,67 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * 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 org.mozilla.fenix.settings.logins.view + +import android.text.method.LinkMovementMethod +import android.view.LayoutInflater +import android.view.ViewGroup +import android.widget.FrameLayout +import androidx.core.view.isVisible +import androidx.recyclerview.widget.LinearLayoutManager +import kotlinx.android.extensions.LayoutContainer +import kotlinx.android.synthetic.main.component_saved_logins.view.* +import org.mozilla.fenix.R +import org.mozilla.fenix.settings.logins.LoginsListState +import org.mozilla.fenix.settings.logins.interactor.SavedLoginsInteractor +import org.mozilla.fenix.ext.addUnderline + +/** + * View that contains and configures the Saved Logins List + */ +class SavedLoginsListView( + override val containerView: ViewGroup, + val interactor: SavedLoginsInteractor +) : LayoutContainer { + + val view: FrameLayout = LayoutInflater.from(containerView.context) + .inflate(R.layout.component_saved_logins, containerView, true) + .findViewById(R.id.saved_logins_wrapper) + + private val loginsAdapter = LoginsAdapter(interactor) + + init { + view.saved_logins_list.apply { + adapter = loginsAdapter + layoutManager = LinearLayoutManager(containerView.context) + itemAnimator = null + } + + with(view.saved_passwords_empty_learn_more) { + movementMethod = LinkMovementMethod.getInstance() + addUnderline() + setOnClickListener { interactor.onLearnMoreClicked() } + } + + with(view.saved_passwords_empty_message) { + val appName = context.getString(R.string.app_name) + text = String.format( + context.getString( + R.string.preferences_passwords_saved_logins_description_empty_text + ), appName + ) + } + } + + fun update(state: LoginsListState) { + if (state.isLoading) { + view.progress_bar.isVisible = true + } else { + view.progress_bar.isVisible = false + view.saved_logins_list.isVisible = state.loginList.isNotEmpty() + view.saved_passwords_empty_view.isVisible = state.loginList.isEmpty() + } + loginsAdapter.submitList(state.filteredItems) + } +} diff --git a/app/src/main/java/org/mozilla/fenix/utils/Settings.kt b/app/src/main/java/org/mozilla/fenix/utils/Settings.kt index fbf5b28e3..b6d4530c6 100644 --- a/app/src/main/java/org/mozilla/fenix/utils/Settings.kt +++ b/app/src/main/java/org/mozilla/fenix/utils/Settings.kt @@ -33,7 +33,7 @@ import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.getPreferenceKey import org.mozilla.fenix.settings.PhoneFeature import org.mozilla.fenix.settings.deletebrowsingdata.DeleteBrowsingDataOnQuitType -import org.mozilla.fenix.settings.logins.SavedLoginsFragment +import org.mozilla.fenix.settings.logins.fragment.SavedLoginsFragment import org.mozilla.fenix.settings.logins.SavedLoginsSortingStrategyMenu import org.mozilla.fenix.settings.logins.SortingStrategy import org.mozilla.fenix.settings.registerOnSharedPreferenceChangeListener diff --git a/app/src/main/res/color/save_enabled_ic_color.xml b/app/src/main/res/color/save_enabled_ic_color.xml new file mode 100644 index 000000000..4c0a682f3 --- /dev/null +++ b/app/src/main/res/color/save_enabled_ic_color.xml @@ -0,0 +1,8 @@ + + + + + + diff --git a/app/src/main/res/color/saved_login_clear_edit_text_tint.xml b/app/src/main/res/color/saved_login_clear_edit_text_tint.xml index acb4e5e7e..ada9ebc97 100644 --- a/app/src/main/res/color/saved_login_clear_edit_text_tint.xml +++ b/app/src/main/res/color/saved_login_clear_edit_text_tint.xml @@ -6,5 +6,5 @@ + android:color="?disabled" /> \ No newline at end of file diff --git a/app/src/main/res/layout/fragment_edit_login.xml b/app/src/main/res/layout/fragment_edit_login.xml index 5881ed11c..9c5a6373f 100644 --- a/app/src/main/res/layout/fragment_edit_login.xml +++ b/app/src/main/res/layout/fragment_edit_login.xml @@ -134,12 +134,13 @@ android:id="@+id/clearUsernameTextButton" android:layout_width="48dp" android:layout_height="30dp" + android:layout_marginTop="3dp" android:layout_marginBottom="10dp" android:background="@null" android:contentDescription="@string/saved_login_copy_username" app:tint="@color/saved_login_clear_edit_text_tint" app:layout_constraintEnd_toEndOf="parent" - app:layout_constraintBottom_toBottomOf="@id/inputLayoutUsername" + app:layout_constraintTop_toTopOf="@id/inputLayoutUsername" app:srcCompat="@drawable/ic_clear" /> + tools:context="org.mozilla.fenix.settings.logins.fragment.SavedLoginsFragment" /> diff --git a/app/src/main/res/menu/login_save.xml b/app/src/main/res/menu/login_save.xml index f8c8d2738..fb9682757 100644 --- a/app/src/main/res/menu/login_save.xml +++ b/app/src/main/res/menu/login_save.xml @@ -8,7 +8,7 @@ \ No newline at end of file diff --git a/app/src/main/res/navigation/nav_graph.xml b/app/src/main/res/navigation/nav_graph.xml index b01087482..33ecf6269 100644 --- a/app/src/main/res/navigation/nav_graph.xml +++ b/app/src/main/res/navigation/nav_graph.xml @@ -321,7 +321,7 @@ Unit = mockk(relaxed = true) - - private val settings: Settings = mockk(relaxed = true) private val metrics: MetricController = mockk(relaxed = true) - private val sortingStrategy: SortingStrategy = SortingStrategy.Alphabetically(testContext) - private val controller = SavedLoginsController(store, navController, browserNavigator, settings, metrics) + private val controller = + LoginsListController( + loginsFragmentStore = store, + navController = navController, + browserNavigator = browserNavigator, + settings = settings, + metrics = metrics + ) @Test fun `GIVEN a sorting strategy, WHEN handleSort is called on the controller, THEN the correct action should be dispatched and the strategy saved in sharedPref`() { controller.handleSort(sortingStrategy) - verify { + verifyAll { store.dispatch( LoginsAction.SortLogins( SortingStrategy.Alphabetically( @@ -55,7 +62,7 @@ class SavedLoginsControllerTest { store.dispatch(LoginsAction.LoginSelected(login)) metrics.track(Event.OpenOneLogin) navController.navigate( - SavedLoginsFragmentDirections.actionSavedLoginsFragmentToLoginDetailFragment(login.guid) + SavedLoginsFragmentDirections.actionSavedLoginsFragmentToLoginDetailFragment(login.guid) ) } } @@ -64,7 +71,7 @@ class SavedLoginsControllerTest { fun `GIVEN the learn more option, WHEN handleLearnMoreClicked is called for it, then we should open the right support webpage`() { controller.handleLearnMoreClicked() - verify { + verifyAll { browserNavigator.invoke( SupportUtils.getGenericSumoURLForTopic(SupportUtils.SumoTopic.SYNC_SETUP), true, diff --git a/app/src/test/java/org/mozilla/fenix/settings/logins/LoginsListViewHolderTest.kt b/app/src/test/java/org/mozilla/fenix/settings/logins/LoginsListViewHolderTest.kt index 3f15ad9fe..73378590b 100644 --- a/app/src/test/java/org/mozilla/fenix/settings/logins/LoginsListViewHolderTest.kt +++ b/app/src/test/java/org/mozilla/fenix/settings/logins/LoginsListViewHolderTest.kt @@ -16,6 +16,8 @@ import org.junit.Test import org.junit.runner.RunWith import org.mozilla.fenix.R import org.mozilla.fenix.helpers.FenixRobolectricTestRunner +import org.mozilla.fenix.settings.logins.interactor.SavedLoginsInteractor +import org.mozilla.fenix.settings.logins.view.LoginsListViewHolder @RunWith(FenixRobolectricTestRunner::class) class LoginsListViewHolderTest { @@ -39,7 +41,10 @@ class LoginsListViewHolderTest { @Test fun `bind url and username`() { - val holder = LoginsListViewHolder(view, interactor) + val holder = LoginsListViewHolder( + view, + interactor + ) holder.bind(baseLogin) assertEquals("mozilla.org", view.webAddressView.text) @@ -48,7 +53,10 @@ class LoginsListViewHolderTest { @Test fun `call interactor on click`() { - val holder = LoginsListViewHolder(view, interactor) + val holder = LoginsListViewHolder( + view, + interactor + ) holder.bind(baseLogin) view.performClick() diff --git a/app/src/test/java/org/mozilla/fenix/settings/logins/SavedLoginsInteractorTest.kt b/app/src/test/java/org/mozilla/fenix/settings/logins/SavedLoginsInteractorTest.kt index 7e56dd379..d82e265e8 100644 --- a/app/src/test/java/org/mozilla/fenix/settings/logins/SavedLoginsInteractorTest.kt +++ b/app/src/test/java/org/mozilla/fenix/settings/logins/SavedLoginsInteractorTest.kt @@ -7,15 +7,25 @@ package org.mozilla.fenix.settings.logins import io.mockk.mockk import io.mockk.verifyAll import mozilla.components.support.test.robolectric.testContext +import org.junit.Before import org.junit.Test import org.junit.runner.RunWith import org.mozilla.fenix.helpers.FenixRobolectricTestRunner +import org.mozilla.fenix.settings.logins.controller.LoginsListController +import org.mozilla.fenix.settings.logins.controller.SavedLoginsStorageController +import org.mozilla.fenix.settings.logins.interactor.SavedLoginsInteractor import kotlin.random.Random @RunWith(FenixRobolectricTestRunner::class) class SavedLoginsInteractorTest { - private val controller: SavedLoginsController = mockk(relaxed = true) - private val interactor = SavedLoginsInteractor(controller) + private val listController: LoginsListController = mockk(relaxed = true) + private val savedLoginsStorageController: SavedLoginsStorageController = mockk(relaxed = true) + private lateinit var interactor: SavedLoginsInteractor + + @Before + fun setup() { + interactor = SavedLoginsInteractor(listController, savedLoginsStorageController) + } @Test fun `GIVEN a SavedLogin being clicked, WHEN the interactor is called for it, THEN it should just delegate the controller`() { @@ -23,7 +33,7 @@ class SavedLoginsInteractorTest { interactor.onItemClicked(item) verifyAll { - controller.handleItemClicked(item) + listController.handleItemClicked(item) } } @@ -34,7 +44,7 @@ class SavedLoginsInteractorTest { interactor.onSortingStrategyChanged(sortingStrategy) verifyAll { - controller.handleSort(sortingStrategy) + listController.handleSort(sortingStrategy) } } @@ -43,7 +53,13 @@ class SavedLoginsInteractorTest { interactor.onLearnMoreClicked() verifyAll { - controller.handleLearnMoreClicked() + listController.handleLearnMoreClicked() } } + + @Test + fun loadAndMapLoginsTest() { + interactor.loadAndMapLogins() + verifyAll { savedLoginsStorageController.handleLoadAndMapLogins() } + } } diff --git a/app/src/test/java/org/mozilla/fenix/settings/logins/SavedLoginsStorageControllerTest.kt b/app/src/test/java/org/mozilla/fenix/settings/logins/SavedLoginsStorageControllerTest.kt new file mode 100644 index 000000000..f42e1f774 --- /dev/null +++ b/app/src/test/java/org/mozilla/fenix/settings/logins/SavedLoginsStorageControllerTest.kt @@ -0,0 +1,139 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + 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 org.mozilla.fenix.settings.logins + +import android.content.Context +import android.os.Looper +import androidx.navigation.NavController +import androidx.navigation.NavDestination +import io.mockk.coEvery +import io.mockk.coVerify +import io.mockk.every +import io.mockk.mockk +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.MainScope +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.test.TestCoroutineScope +import mozilla.components.concept.storage.Login +import org.junit.After +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.mozilla.fenix.R +import org.mozilla.fenix.components.Components +import org.mozilla.fenix.ext.components +import org.mozilla.fenix.helpers.FenixRobolectricTestRunner +import org.mozilla.fenix.settings.logins.controller.SavedLoginsStorageController +import org.robolectric.Shadows.shadowOf +import org.robolectric.annotation.LooperMode + +@ExperimentalCoroutinesApi +@LooperMode(LooperMode.Mode.PAUSED) +@RunWith(FenixRobolectricTestRunner::class) +class SavedLoginsStorageControllerTest { + private lateinit var components: Components + private val context: Context = mockk(relaxed = true) + private lateinit var controller: SavedLoginsStorageController + private val navController: NavController = mockk(relaxed = true) + private val loginsFragmentStore: LoginsFragmentStore = mockk(relaxed = true) + private val scope = TestCoroutineScope() + private val loginMock: Login = mockk(relaxed = true) + + @Before + fun setup() { + every { navController.currentDestination } returns NavDestination("").apply { + id = R.id.loginDetailFragment + } + coEvery { context.components.core.passwordsStorage.get(any()) } returns loginMock + every { loginsFragmentStore.dispatch(any()) } returns mockk() + coEvery { context.components.core.passwordsStorage } returns mockk(relaxed = true) + components = mockk(relaxed = true) + + controller = SavedLoginsStorageController( + context = context, + viewLifecycleScope = MainScope(), + navController = navController, + loginsFragmentStore = loginsFragmentStore + ) + } + + @After + fun cleanUp() { + scope.cleanupTestCoroutines() + } + + @Test + fun `WHEN a login is deleted, THEN navigate back to the previous page`() = runBlocking { + val loginId = "id" + // mock for deleteLoginJob: Deferred? + coEvery { context.components.core.passwordsStorage.delete(any()) } returns true + controller.delete(loginId) + + shadow() + + coVerify { context.components.core.passwordsStorage.delete(loginId) } + } + + private fun shadow() { + // solves issue with Roboelectric v4.3 and SDK 28 + // https://github.com/robolectric/robolectric/issues/5356 + shadowOf(Looper.getMainLooper()).idle() + } + + @Test + fun `WHEN fetching the login list, THEN update the state in the store`() { + val loginId = "id" + // for deferredLogin: Deferred>? + coEvery { context.components.core.passwordsStorage.list() } returns listOf() + + controller.fetchLoginDetails(loginId) + + coVerify { context.components.core.passwordsStorage.list() } + } + + @Test + fun `WHEN saving an update to an item, THEN navigate to login detail view`() { + val login = Login( + guid = "id", + origin = "https://www.test.co.gov.org", + username = "user123", + password = "securePassword1", + httpRealm = "httpRealm", + formActionOrigin = "" + ) + coEvery { context.components.core.passwordsStorage.get(any()) } returns loginMock + + controller.save(login.guid!!, login.username, login.password) + + coVerify { context.components.core.passwordsStorage.get(any()) } + } + + @Test + fun `WHEN finding login dupes, THEN update duplicates in the store`() { + val login = Login( + guid = "id", + origin = "https://www.test.co.gov.org", + username = "user123", + password = "securePassword1", + httpRealm = "httpRealm", + formActionOrigin = "" + ) + + coEvery { context.components.core.passwordsStorage.get(any()) } returns login + + // for deferredLogin: Deferred>? + coEvery { + context.components.core.passwordsStorage.getPotentialDupesIgnoringUsername(any()) + } returns listOf() + + controller.findPotentialDuplicates(login.guid!!) + + shadow() + + coVerify { + context.components.core.passwordsStorage.getPotentialDupesIgnoringUsername(login) + } + } +} diff --git a/docs/metrics.md b/docs/metrics.md index 912b814ad..f8b59d646 100644 --- a/docs/metrics.md +++ b/docs/metrics.md @@ -118,8 +118,11 @@ The following metrics are added to the ping: | history.removed_all |[event](https://mozilla.github.io/glean/book/user/metrics/event.html) |A user removed all history items |[1](https://github.com/mozilla-mobile/fenix/pull/3940)||2020-10-01 | | history.shared |[event](https://mozilla.github.io/glean/book/user/metrics/event.html) |A user shared a history item |[1](https://github.com/mozilla-mobile/fenix/pull/3940)||2020-10-01 | | logins.copy_login |[event](https://mozilla.github.io/glean/book/user/metrics/event.html) |A user copied a piece of a login in saved logins |[1](https://github.com/mozilla-mobile/fenix/pull/6352)||2020-10-01 | +| logins.delete_saved_login |[event](https://mozilla.github.io/glean/book/user/metrics/event.html) |A user confirms delete of a saved login |[1](https://github.com/mozilla-mobile/fenix/issues/11208)||2020-10-01 | | logins.open_individual_login |[event](https://mozilla.github.io/glean/book/user/metrics/event.html) |A user accessed an individual login in saved logins |[1](https://github.com/mozilla-mobile/fenix/pull/6352)||2020-10-01 | +| logins.open_login_editor |[event](https://mozilla.github.io/glean/book/user/metrics/event.html) |A user entered the edit screen for an individual saved login |[1](https://github.com/mozilla-mobile/fenix/issues/11208)||2020-10-01 | | logins.open_logins |[event](https://mozilla.github.io/glean/book/user/metrics/event.html) |A user accessed Logins in Settings |[1](https://github.com/mozilla-mobile/fenix/pull/6352)||2020-10-01 | +| logins.save_edited_login |[event](https://mozilla.github.io/glean/book/user/metrics/event.html) |A user saves changes made to an individual login |[1](https://github.com/mozilla-mobile/fenix/issues/11208)||2020-10-01 | | logins.save_logins_setting_changed |[event](https://mozilla.github.io/glean/book/user/metrics/event.html) |A user changed their setting for asking to save logins |[1](https://github.com/mozilla-mobile/fenix/pull/7767)|
  • setting: The new setting for saving logins the user selected. Either `ask_to_save` or `never_save`
|2020-10-01 | | logins.view_password_login |[event](https://mozilla.github.io/glean/book/user/metrics/event.html) |A user viewed a password in an individual saved login |[1](https://github.com/mozilla-mobile/fenix/pull/6352)||2020-10-01 | | media_notification.pause |[event](https://mozilla.github.io/glean/book/user/metrics/event.html) |A user pressed the pause icon on the media notification |[1](https://github.com/mozilla-mobile/fenix/pull/5520)||2020-10-01 | From 2fe17a62209a1f17a96d69c69ae875508c70677b Mon Sep 17 00:00:00 2001 From: Kainalu Hagiwara Date: Thu, 16 Jul 2020 22:26:49 -0700 Subject: [PATCH 03/17] For #12531: Align Settings -> Private browsing to 72dp keyline (#12654) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Hakkı Kaan Çalışkan --- .../main/res/xml/private_browsing_preferences.xml | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/app/src/main/res/xml/private_browsing_preferences.xml b/app/src/main/res/xml/private_browsing_preferences.xml index 1601a1a41..3d75b9871 100644 --- a/app/src/main/res/xml/private_browsing_preferences.xml +++ b/app/src/main/res/xml/private_browsing_preferences.xml @@ -2,20 +2,16 @@ - - + + android:title="@string/preferences_add_private_browsing_shortcut" /> + android:title="@string/preferences_open_links_in_a_private_tab" /> + android:title="@string/preferences_allow_screenshots_in_private_mode" /> From 074b0139163df65976b216c4452b45e4054d1428 Mon Sep 17 00:00:00 2001 From: Aaron Train Date: Fri, 17 Jul 2020 09:31:20 -0400 Subject: [PATCH 04/17] Update Flank to v20.07.0 (#12640) --- taskcluster/docker/ui-tests/Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/taskcluster/docker/ui-tests/Dockerfile b/taskcluster/docker/ui-tests/Dockerfile index aff76f402..b16eaa94c 100644 --- a/taskcluster/docker/ui-tests/Dockerfile +++ b/taskcluster/docker/ui-tests/Dockerfile @@ -12,7 +12,7 @@ USER worker:worker ENV GOOGLE_SDK_DOWNLOAD ./gcloud.tar.gz ENV GOOGLE_SDK_VERSION 233 -ENV FLANK_VERSION v20.06.2 +ENV FLANK_VERSION v20.07.0 ENV TEST_TOOLS /builds/worker/test-tools ENV PATH ${PATH}:${TEST_TOOLS}:${TEST_TOOLS}/google-cloud-sdk/bin From ac3df6bc5eb71a5099f7a6c6fc04459b74e1101e Mon Sep 17 00:00:00 2001 From: Tiger Oakes Date: Fri, 17 Jul 2020 09:04:16 -0700 Subject: [PATCH 05/17] Add tests for adapters in collections (#12649) --- .../CollectionCreationTabListAdapter.kt | 67 +++----- .../collections/SaveCollectionListAdapter.kt | 11 +- .../mozilla/fenix/collections/TabDiffUtil.kt | 63 ++++++++ .../CollectionCreationTabListAdapterTest.kt | 89 +++++++++++ .../SaveCollectionListAdapterTest.kt | 80 ++++++++++ .../fenix/collections/TabDiffUtilTest.kt | 151 ++++++++++++++++++ 6 files changed, 407 insertions(+), 54 deletions(-) create mode 100644 app/src/main/java/org/mozilla/fenix/collections/TabDiffUtil.kt create mode 100644 app/src/test/java/org/mozilla/fenix/collections/CollectionCreationTabListAdapterTest.kt create mode 100644 app/src/test/java/org/mozilla/fenix/collections/SaveCollectionListAdapterTest.kt create mode 100644 app/src/test/java/org/mozilla/fenix/collections/TabDiffUtilTest.kt diff --git a/app/src/main/java/org/mozilla/fenix/collections/CollectionCreationTabListAdapter.kt b/app/src/main/java/org/mozilla/fenix/collections/CollectionCreationTabListAdapter.kt index a936608b2..e74cc340e 100644 --- a/app/src/main/java/org/mozilla/fenix/collections/CollectionCreationTabListAdapter.kt +++ b/app/src/main/java/org/mozilla/fenix/collections/CollectionCreationTabListAdapter.kt @@ -11,22 +11,24 @@ import androidx.core.view.isGone import androidx.core.view.isInvisible import androidx.recyclerview.widget.DiffUtil import androidx.recyclerview.widget.RecyclerView -import kotlinx.android.synthetic.main.collection_tab_list_row.view.* +import kotlinx.android.synthetic.main.collection_tab_list_row.* import org.mozilla.fenix.R import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.loadIntoView import org.mozilla.fenix.home.Tab +import org.mozilla.fenix.utils.view.ViewHolder class CollectionCreationTabListAdapter( private val interactor: CollectionCreationInteractor ) : RecyclerView.Adapter() { + private var tabs: List = listOf() private var selectedTabs: MutableSet = mutableSetOf() private var hideCheckboxes = false override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): TabViewHolder { - val view = - LayoutInflater.from(parent.context).inflate(TabViewHolder.LAYOUT_ID, parent, false) + val view = LayoutInflater.from(parent.context) + .inflate(TabViewHolder.LAYOUT_ID, parent, false) return TabViewHolder(view) } @@ -39,11 +41,11 @@ class CollectionCreationTabListAdapter( is CheckChanged -> { val checkChanged = payloads[0] as CheckChanged if (checkChanged.shouldBeChecked) { - holder.itemView.tab_selected_checkbox.isChecked = true + holder.tab_selected_checkbox.isChecked = true } else if (checkChanged.shouldBeUnchecked) { - holder.itemView.tab_selected_checkbox.isChecked = false + holder.tab_selected_checkbox.isChecked = false } - holder.itemView.tab_selected_checkbox.isGone = checkChanged.shouldHideCheckBox + holder.tab_selected_checkbox.isGone = checkChanged.shouldHideCheckBox } } } @@ -52,7 +54,7 @@ class CollectionCreationTabListAdapter( override fun onBindViewHolder(holder: TabViewHolder, position: Int) { val tab = tabs[position] val isSelected = selectedTabs.contains(tab) - holder.itemView.tab_selected_checkbox.setOnCheckedChangeListener { _, isChecked -> + holder.tab_selected_checkbox.setOnCheckedChangeListener { _, isChecked -> if (isChecked) { selectedTabs.add(tab) interactor.addTabToSelection(tab) @@ -86,57 +88,24 @@ class CollectionCreationTabListAdapter( } } -private class TabDiffUtil( - val old: List, - val new: List, - val oldSelected: Set, - val newSelected: Set, - val oldHideCheckboxes: Boolean, - val newHideCheckboxes: Boolean -) : DiffUtil.Callback() { - override fun areItemsTheSame(oldItemPosition: Int, newItemPosition: Int): Boolean = - old[oldItemPosition].sessionId == new[newItemPosition].sessionId - - override fun areContentsTheSame(oldItemPosition: Int, newItemPosition: Int): Boolean { - val isSameTab = old[oldItemPosition].sessionId == new[newItemPosition].sessionId - val sameSelectedState = oldSelected.contains(old[oldItemPosition]) == newSelected.contains(new[newItemPosition]) - val isSameHideCheckboxes = oldHideCheckboxes == newHideCheckboxes - return isSameTab && sameSelectedState && isSameHideCheckboxes - } - - override fun getChangePayload(oldItemPosition: Int, newItemPosition: Int): Any? { - val shouldBeChecked = newSelected.contains(new[newItemPosition]) && !oldSelected.contains(old[oldItemPosition]) - val shouldBeUnchecked = - !newSelected.contains(new[newItemPosition]) && oldSelected.contains(old[oldItemPosition]) - return CheckChanged(shouldBeChecked, shouldBeUnchecked, newHideCheckboxes) - } - - override fun getOldListSize(): Int = old.size - override fun getNewListSize(): Int = new.size -} - -data class CheckChanged(val shouldBeChecked: Boolean, val shouldBeUnchecked: Boolean, val shouldHideCheckBox: Boolean) - -class TabViewHolder(view: View) : RecyclerView.ViewHolder(view) { - - private val checkbox = view.tab_selected_checkbox!! +class TabViewHolder(view: View) : ViewHolder(view) { init { - view.collection_item_tab.setOnClickListener { - checkbox.isChecked = !checkbox.isChecked + collection_item_tab.setOnClickListener { + tab_selected_checkbox.isChecked = !tab_selected_checkbox.isChecked } } fun bind(tab: Tab, isSelected: Boolean, shouldHideCheckBox: Boolean) { - itemView.hostname.text = tab.hostname - itemView.tab_title.text = tab.title - checkbox.isInvisible = shouldHideCheckBox + hostname.text = tab.hostname + tab_title.text = tab.title + tab_selected_checkbox.isInvisible = shouldHideCheckBox itemView.isClickable = !shouldHideCheckBox - if (checkbox.isChecked != isSelected) { - checkbox.isChecked = isSelected + if (tab_selected_checkbox.isChecked != isSelected) { + tab_selected_checkbox.isChecked = isSelected } - itemView.context.components.core.icons.loadIntoView(itemView.favicon_image, tab.url) + itemView.context.components.core.icons.loadIntoView(favicon_image, tab.url) } companion object { diff --git a/app/src/main/java/org/mozilla/fenix/collections/SaveCollectionListAdapter.kt b/app/src/main/java/org/mozilla/fenix/collections/SaveCollectionListAdapter.kt index fa35f442c..0ee663b53 100644 --- a/app/src/main/java/org/mozilla/fenix/collections/SaveCollectionListAdapter.kt +++ b/app/src/main/java/org/mozilla/fenix/collections/SaveCollectionListAdapter.kt @@ -10,12 +10,13 @@ import android.view.ViewGroup import androidx.core.graphics.BlendModeColorFilterCompat.createBlendModeColorFilterCompat import androidx.core.graphics.BlendModeCompat.SRC_IN import androidx.recyclerview.widget.RecyclerView -import kotlinx.android.synthetic.main.collections_list_item.view.* +import kotlinx.android.synthetic.main.collections_list_item.* import mozilla.components.feature.tab.collections.TabCollection import org.mozilla.fenix.R import org.mozilla.fenix.components.description import org.mozilla.fenix.ext.getIconColor import org.mozilla.fenix.home.Tab +import org.mozilla.fenix.utils.view.ViewHolder class SaveCollectionListAdapter( private val interactor: CollectionCreationInteractor @@ -48,12 +49,12 @@ class SaveCollectionListAdapter( } } -class CollectionViewHolder(view: View) : RecyclerView.ViewHolder(view) { +class CollectionViewHolder(view: View) : ViewHolder(view) { fun bind(collection: TabCollection) { - itemView.collection_item.text = collection.title - itemView.collection_description.text = collection.description(itemView.context) - itemView.collection_icon.colorFilter = + collection_item.text = collection.title + collection_description.text = collection.description(itemView.context) + collection_icon.colorFilter = createBlendModeColorFilterCompat(collection.getIconColor(itemView.context), SRC_IN) } diff --git a/app/src/main/java/org/mozilla/fenix/collections/TabDiffUtil.kt b/app/src/main/java/org/mozilla/fenix/collections/TabDiffUtil.kt new file mode 100644 index 000000000..a555dd8f0 --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/collections/TabDiffUtil.kt @@ -0,0 +1,63 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * 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 org.mozilla.fenix.collections + +import androidx.recyclerview.widget.DiffUtil +import org.mozilla.fenix.home.Tab + +/** + * Diff callback for comparing tab lists with selected state. + */ +internal class TabDiffUtil( + private val old: List, + private val new: List, + private val oldSelected: Set, + private val newSelected: Set, + private val oldHideCheckboxes: Boolean, + private val newHideCheckboxes: Boolean +) : DiffUtil.Callback() { + + /** + * Checks if the tabs in the given positions refer to the same tab (based on ID). + */ + override fun areItemsTheSame(oldItemPosition: Int, newItemPosition: Int): Boolean = + old[oldItemPosition].sessionId == new[newItemPosition].sessionId + + /** + * Checks if the combination of tab ID, selection, and checkbox visibility is the same. + */ + override fun areContentsTheSame(oldItemPosition: Int, newItemPosition: Int): Boolean { + val isSameTab = old[oldItemPosition].sessionId == new[newItemPosition].sessionId + val sameSelectedState = oldItemSelected(oldItemPosition) == newItemSelected(newItemPosition) + val isSameHideCheckboxes = oldHideCheckboxes == newHideCheckboxes + return isSameTab && sameSelectedState && isSameHideCheckboxes + } + + /** + * Returns a change payload indication if the item is now/no longer selected. + */ + override fun getChangePayload(oldItemPosition: Int, newItemPosition: Int): Any? { + val shouldBeChecked = newItemSelected(newItemPosition) && !oldItemSelected(oldItemPosition) + val shouldBeUnchecked = !newItemSelected(newItemPosition) && oldItemSelected(oldItemPosition) + return CheckChanged(shouldBeChecked, shouldBeUnchecked, newHideCheckboxes) + } + + override fun getOldListSize(): Int = old.size + override fun getNewListSize(): Int = new.size + + private fun oldItemSelected(oldItemPosition: Int) = oldSelected.contains(old[oldItemPosition]) + private fun newItemSelected(newItemPosition: Int) = newSelected.contains(new[newItemPosition]) +} + +/** + * @property shouldBeChecked Item was previously unchecked and should be checked. + * @property shouldBeUnchecked Item was previously checked and should be unchecked. + * @property shouldHideCheckBox Checkbox should be visible. + */ +data class CheckChanged( + val shouldBeChecked: Boolean, + val shouldBeUnchecked: Boolean, + val shouldHideCheckBox: Boolean +) diff --git a/app/src/test/java/org/mozilla/fenix/collections/CollectionCreationTabListAdapterTest.kt b/app/src/test/java/org/mozilla/fenix/collections/CollectionCreationTabListAdapterTest.kt new file mode 100644 index 000000000..cc451b1ff --- /dev/null +++ b/app/src/test/java/org/mozilla/fenix/collections/CollectionCreationTabListAdapterTest.kt @@ -0,0 +1,89 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * 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 org.mozilla.fenix.collections + +import android.widget.FrameLayout +import androidx.core.view.isInvisible +import androidx.recyclerview.widget.RecyclerView +import io.mockk.Runs +import io.mockk.every +import io.mockk.just +import io.mockk.mockk +import io.mockk.verify +import kotlinx.android.synthetic.main.collection_tab_list_row.* +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.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.mozilla.fenix.helpers.FenixRobolectricTestRunner +import org.mozilla.fenix.home.Tab + +@RunWith(FenixRobolectricTestRunner::class) +class CollectionCreationTabListAdapterTest { + + private lateinit var interactor: CollectionCreationInteractor + private lateinit var adapter: CollectionCreationTabListAdapter + + @Before + fun setup() { + interactor = mockk() + adapter = CollectionCreationTabListAdapter(interactor) + + every { interactor.selectCollection(any(), any()) } just Runs + } + + @Test + fun `getItemCount should return the number of tab collections`() { + val tab = mockk() + + assertEquals(0, adapter.itemCount) + + adapter.updateData( + tabs = listOf(tab), + selectedTabs = emptySet() + ) + assertEquals(1, adapter.itemCount) + } + + @Test + fun `creates and binds viewholder`() { + val tab = mockk { + every { sessionId } returns "abc" + every { title } returns "Mozilla" + every { hostname } returns "mozilla.org" + every { url } returns "https://mozilla.org" + } + adapter.updateData( + tabs = listOf(tab), + selectedTabs = emptySet() + ) + + val holder = adapter.createViewHolder(FrameLayout(testContext), 0) + adapter.bindViewHolder(holder, 0) + + assertEquals("Mozilla", holder.tab_title.text) + assertEquals("mozilla.org", holder.hostname.text) + assertFalse(holder.tab_selected_checkbox.isInvisible) + assertTrue(holder.itemView.isClickable) + } + + @Test + fun `updateData inserts item`() { + val tab = mockk { + every { sessionId } returns "abc" + } + val observer = mockk(relaxed = true) + adapter.registerAdapterDataObserver(observer) + adapter.updateData( + tabs = listOf(tab), + selectedTabs = emptySet() + ) + + verify { observer.onItemRangeInserted(0, 1) } + } +} diff --git a/app/src/test/java/org/mozilla/fenix/collections/SaveCollectionListAdapterTest.kt b/app/src/test/java/org/mozilla/fenix/collections/SaveCollectionListAdapterTest.kt new file mode 100644 index 000000000..ddcff8572 --- /dev/null +++ b/app/src/test/java/org/mozilla/fenix/collections/SaveCollectionListAdapterTest.kt @@ -0,0 +1,80 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * 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 org.mozilla.fenix.collections + +import android.view.ViewGroup +import android.widget.FrameLayout +import io.mockk.Runs +import io.mockk.every +import io.mockk.just +import io.mockk.mockk +import io.mockk.verify +import kotlinx.android.synthetic.main.collections_list_item.view.* +import mozilla.components.feature.tab.collections.TabCollection +import mozilla.components.support.test.robolectric.testContext +import org.junit.Assert.assertEquals +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.mozilla.fenix.helpers.FenixRobolectricTestRunner + +@RunWith(FenixRobolectricTestRunner::class) +class SaveCollectionListAdapterTest { + + private lateinit var parent: ViewGroup + private lateinit var interactor: CollectionCreationInteractor + private lateinit var adapter: SaveCollectionListAdapter + + @Before + fun setup() { + parent = FrameLayout(testContext) + interactor = mockk() + adapter = SaveCollectionListAdapter(interactor) + + every { interactor.selectCollection(any(), any()) } just Runs + } + + @Test + fun `getItemCount should return the number of tab collections`() { + val collection = mockk() + + assertEquals(0, adapter.itemCount) + + adapter.updateData( + tabCollections = listOf(collection), + selectedTabs = emptySet() + ) + assertEquals(1, adapter.itemCount) + } + + @Test + fun `creates and binds viewholder`() { + val collection = mockk { + every { id } returns 0L + every { title } returns "Collection" + every { tabs } returns listOf( + mockk { + every { url } returns "https://mozilla.org" + }, + mockk { + every { url } returns "https://firefox.com" + } + ) + } + adapter.updateData( + tabCollections = listOf(collection), + selectedTabs = emptySet() + ) + + val holder = adapter.createViewHolder(parent, 0) + adapter.bindViewHolder(holder, 0) + + assertEquals("Collection", holder.itemView.collection_item.text) + assertEquals("mozilla.org, firefox.com", holder.itemView.collection_description.text) + + holder.itemView.performClick() + verify { interactor.selectCollection(collection, emptyList()) } + } +} diff --git a/app/src/test/java/org/mozilla/fenix/collections/TabDiffUtilTest.kt b/app/src/test/java/org/mozilla/fenix/collections/TabDiffUtilTest.kt new file mode 100644 index 000000000..748dbde12 --- /dev/null +++ b/app/src/test/java/org/mozilla/fenix/collections/TabDiffUtilTest.kt @@ -0,0 +1,151 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * 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 org.mozilla.fenix.collections + +import io.mockk.every +import io.mockk.mockk +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test +import org.mozilla.fenix.home.Tab + +class TabDiffUtilTest { + + @Test + fun `list size is returned`() { + val diffUtil = TabDiffUtil( + old = listOf(mockk(), mockk()), + new = listOf(mockk()), + oldSelected = emptySet(), + newSelected = emptySet(), + oldHideCheckboxes = false, + newHideCheckboxes = false + ) + + assertEquals(2, diffUtil.oldListSize) + assertEquals(1, diffUtil.newListSize) + } + + @Test + fun `single lists are the same`() { + val tab = mockk { + every { sessionId } returns "abc" + } + val diffUtil = TabDiffUtil( + old = listOf(tab), + new = listOf(tab), + oldSelected = emptySet(), + newSelected = emptySet(), + oldHideCheckboxes = false, + newHideCheckboxes = false + ) + + assertTrue(diffUtil.areItemsTheSame(0, 0)) + assertTrue(diffUtil.areContentsTheSame(0, 0)) + } + + @Test + fun `selection affects contents`() { + val tab = mockk { + every { sessionId } returns "abc" + } + val diffUtil = TabDiffUtil( + old = listOf(tab), + new = listOf(tab), + oldSelected = emptySet(), + newSelected = setOf(tab), + oldHideCheckboxes = false, + newHideCheckboxes = false + ) + + assertTrue(diffUtil.areItemsTheSame(0, 0)) + assertFalse(diffUtil.areContentsTheSame(0, 0)) + } + + @Test + fun `hide checkboxes affects contents`() { + val tab = mockk { + every { sessionId } returns "abc" + } + val diffUtil = TabDiffUtil( + old = listOf(tab), + new = listOf(tab), + oldSelected = setOf(tab), + newSelected = setOf(tab), + oldHideCheckboxes = false, + newHideCheckboxes = true + ) + + assertTrue(diffUtil.areItemsTheSame(0, 0)) + assertFalse(diffUtil.areContentsTheSame(0, 0)) + } + + @Test + fun `change payload covers no change case`() { + val tab = mockk() + val payload = TabDiffUtil( + old = listOf(tab), + new = listOf(tab), + oldSelected = setOf(tab), + newSelected = setOf(tab), + oldHideCheckboxes = false, + newHideCheckboxes = false + ).getChangePayload(0, 0) + + assertEquals( + CheckChanged( + shouldBeChecked = false, + shouldBeUnchecked = false, + shouldHideCheckBox = false + ), + payload + ) + } + + @Test + fun `include shouldBeChecked in change payload`() { + val tab = mockk() + val payload = TabDiffUtil( + old = listOf(tab), + new = listOf(tab), + oldSelected = emptySet(), + newSelected = setOf(tab), + oldHideCheckboxes = false, + newHideCheckboxes = false + ).getChangePayload(0, 0) + + assertEquals( + CheckChanged( + shouldBeChecked = true, + shouldBeUnchecked = false, + shouldHideCheckBox = false + ), + payload + ) + } + + @Test + fun `include shouldBeUnchecked in change payload`() { + val tab = mockk() + val payload = TabDiffUtil( + old = listOf(tab), + new = listOf(tab), + oldSelected = setOf(tab), + newSelected = emptySet(), + oldHideCheckboxes = false, + newHideCheckboxes = true + ).getChangePayload(0, 0) + + assertEquals( + CheckChanged( + shouldBeChecked = false, + shouldBeUnchecked = true, + shouldHideCheckBox = true + ), + payload + ) + } +} From e358f95eed81239c5081530fc2d206d8cf300547 Mon Sep 17 00:00:00 2001 From: ekager Date: Thu, 16 Jul 2020 14:44:24 -0400 Subject: [PATCH 06/17] For #12364 - Only show PWA prompt the third time a user visits installable site --- .../mozilla/fenix/browser/BrowserFragment.kt | 6 +- .../org/mozilla/fenix/perf/Performance.kt | 2 +- ...ment.kt => PwaOnboardingDialogFragment.kt} | 6 +- ...waObserver.kt => PwaOnboardingObserver.kt} | 17 ++-- .../java/org/mozilla/fenix/utils/Settings.kt | 81 +++++++++++++------ ...t_time.xml => fragment_pwa_onboarding.xml} | 2 +- app/src/main/res/navigation/nav_graph.xml | 11 ++- app/src/main/res/values/preference_keys.xml | 1 + .../org/mozilla/fenix/utils/SettingsTest.kt | 25 ++++++ 9 files changed, 107 insertions(+), 44 deletions(-) rename app/src/main/java/org/mozilla/fenix/shortcut/{FirstTimePwaFragment.kt => PwaOnboardingDialogFragment.kt} (88%) rename app/src/main/java/org/mozilla/fenix/shortcut/{FirstTimePwaObserver.kt => PwaOnboardingObserver.kt} (59%) rename app/src/main/res/layout/{fragment_pwa_first_time.xml => fragment_pwa_onboarding.xml} (98%) diff --git a/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt b/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt index ed9b83c58..5c6bc0492 100644 --- a/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt @@ -40,7 +40,7 @@ import org.mozilla.fenix.ext.navigateSafe import org.mozilla.fenix.ext.requireComponents import org.mozilla.fenix.ext.resetPoliciesAfter import org.mozilla.fenix.ext.settings -import org.mozilla.fenix.shortcut.FirstTimePwaObserver +import org.mozilla.fenix.shortcut.PwaOnboardingObserver import org.mozilla.fenix.trackingprotection.TrackingProtectionOverlay /** @@ -156,9 +156,9 @@ class BrowserFragment : BaseBrowserFragment(), UserInteractionHandler { } session?.register(toolbarSessionObserver, viewLifecycleOwner, autoPause = true) - if (settings.shouldShowFirstTimePwaFragment) { + if (!settings.userKnowsAboutPwas) { session?.register( - FirstTimePwaObserver( + PwaOnboardingObserver( navController = findNavController(), settings = settings, webAppUseCases = context.components.useCases.webAppUseCases diff --git a/app/src/main/java/org/mozilla/fenix/perf/Performance.kt b/app/src/main/java/org/mozilla/fenix/perf/Performance.kt index 4b0a73b22..f1eb26ffb 100644 --- a/app/src/main/java/org/mozilla/fenix/perf/Performance.kt +++ b/app/src/main/java/org/mozilla/fenix/perf/Performance.kt @@ -74,6 +74,6 @@ object Performance { * Disables the first time PWA popup. */ private fun disableFirstTimePWAPopup(context: Context) { - Settings.getInstance(context).userKnowsAboutPWAs = true + Settings.getInstance(context).userKnowsAboutPwas = true } } diff --git a/app/src/main/java/org/mozilla/fenix/shortcut/FirstTimePwaFragment.kt b/app/src/main/java/org/mozilla/fenix/shortcut/PwaOnboardingDialogFragment.kt similarity index 88% rename from app/src/main/java/org/mozilla/fenix/shortcut/FirstTimePwaFragment.kt rename to app/src/main/java/org/mozilla/fenix/shortcut/PwaOnboardingDialogFragment.kt index c42c2842f..43b9099fc 100644 --- a/app/src/main/java/org/mozilla/fenix/shortcut/FirstTimePwaFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/shortcut/PwaOnboardingDialogFragment.kt @@ -16,9 +16,9 @@ import org.mozilla.fenix.R import org.mozilla.fenix.ext.requireComponents /** - * Dialog displayed the first time the user navigates to an installable web app. + * Dialog displayed the third time the user navigates to an installable web app. */ -class FirstTimePwaFragment : DialogFragment() { +class PwaOnboardingDialogFragment : DialogFragment() { override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) setStyle(STYLE_NO_TITLE, R.style.CreateShortcutDialogStyle) @@ -28,7 +28,7 @@ class FirstTimePwaFragment : DialogFragment() { inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle? - ): View? = inflater.inflate(R.layout.fragment_pwa_first_time, container, false) + ): View? = inflater.inflate(R.layout.fragment_pwa_onboarding, container, false) override fun onViewCreated(view: View, savedInstanceState: Bundle?) { super.onViewCreated(view, savedInstanceState) diff --git a/app/src/main/java/org/mozilla/fenix/shortcut/FirstTimePwaObserver.kt b/app/src/main/java/org/mozilla/fenix/shortcut/PwaOnboardingObserver.kt similarity index 59% rename from app/src/main/java/org/mozilla/fenix/shortcut/FirstTimePwaObserver.kt rename to app/src/main/java/org/mozilla/fenix/shortcut/PwaOnboardingObserver.kt index 2c19810d7..5e312df86 100644 --- a/app/src/main/java/org/mozilla/fenix/shortcut/FirstTimePwaObserver.kt +++ b/app/src/main/java/org/mozilla/fenix/shortcut/PwaOnboardingObserver.kt @@ -14,20 +14,23 @@ import org.mozilla.fenix.ext.nav import org.mozilla.fenix.utils.Settings /** - * Displays the [FirstTimePwaFragment] info dialog when a PWA is first opened in the browser. + * Displays the [PwaOnboardingDialogFragment] info dialog when a PWA is opened in the browser for the third time. */ -class FirstTimePwaObserver( +class PwaOnboardingObserver( private val navController: NavController, private val settings: Settings, private val webAppUseCases: WebAppUseCases ) : Session.Observer { override fun onWebAppManifestChanged(session: Session, manifest: WebAppManifest?) { - if (webAppUseCases.isInstallable() && settings.shouldShowFirstTimePwaFragment) { - val directions = BrowserFragmentDirections.actionBrowserFragmentToFirstTimePwaFragment() - navController.nav(R.id.browserFragment, directions) - - settings.userKnowsAboutPWAs = true + if (webAppUseCases.isInstallable() && !settings.userKnowsAboutPwas) { + settings.incrementVisitedInstallableCount() + if (settings.shouldShowPwaOnboarding) { + val directions = + BrowserFragmentDirections.actionBrowserFragmentToPwaOnboardingDialogFragment() + navController.nav(R.id.browserFragment, directions) + settings.userKnowsAboutPwas = true + } } } } diff --git a/app/src/main/java/org/mozilla/fenix/utils/Settings.kt b/app/src/main/java/org/mozilla/fenix/utils/Settings.kt index b6d4530c6..a76825bef 100644 --- a/app/src/main/java/org/mozilla/fenix/utils/Settings.kt +++ b/app/src/main/java/org/mozilla/fenix/utils/Settings.kt @@ -53,6 +53,7 @@ class Settings private constructor( const val showLoginsSecureWarningSyncMaxCount = 1 const val showLoginsSecureWarningMaxCount = 1 const val trackingProtectionOnboardingMaximumCount = 1 + const val pwaVisitsToShowPromptMaxCount = 3 const val FENIX_PREFERENCES = "fenix_preferences" private const val showSearchWidgetCFRMaxCount = 3 @@ -146,9 +147,18 @@ class Settings private constructor( // If any of the prefs have been modified, quit displaying the fenix moved tip fun shouldDisplayFenixMovingTip(): Boolean = - preferences.getBoolean(appContext.getString(R.string.pref_key_migrating_from_fenix_nightly_tip), true) && - preferences.getBoolean(appContext.getString(R.string.pref_key_migrating_from_firefox_nightly_tip), true) && - preferences.getBoolean(appContext.getString(R.string.pref_key_migrating_from_fenix_tip), true) + preferences.getBoolean( + appContext.getString(R.string.pref_key_migrating_from_fenix_nightly_tip), + true + ) && + preferences.getBoolean( + appContext.getString(R.string.pref_key_migrating_from_firefox_nightly_tip), + true + ) && + preferences.getBoolean( + appContext.getString(R.string.pref_key_migrating_from_fenix_tip), + true + ) private val activeSearchCount by intPreference( appContext.getPreferenceKey(R.string.pref_key_search_count), @@ -167,9 +177,9 @@ class Settings private constructor( fun shouldDisplaySearchWidgetCFR(): Boolean = isActiveSearcher && - searchWidgetCFRDismissCount < showSearchWidgetCFRMaxCount && - !searchWidgetInstalled && - !searchWidgetCFRManuallyDismissed + searchWidgetCFRDismissCount < showSearchWidgetCFRMaxCount && + !searchWidgetInstalled && + !searchWidgetCFRManuallyDismissed private val searchWidgetCFRDisplayCount by intPreference( appContext.getPreferenceKey(R.string.pref_key_search_widget_cfr_display_count), @@ -236,10 +246,10 @@ class Settings private constructor( val isCrashReportingEnabled: Boolean get() = isCrashReportEnabledInBuild && - preferences.getBoolean( - appContext.getPreferenceKey(R.string.pref_key_crash_reporter), - true - ) + preferences.getBoolean( + appContext.getPreferenceKey(R.string.pref_key_crash_reporter), + true + ) val isRemoteDebuggingEnabled by booleanPreference( appContext.getPreferenceKey(R.string.pref_key_remote_debugging), @@ -267,7 +277,7 @@ class Settings private constructor( val shouldShowTrackingProtectionOnboarding: Boolean get() = !isOverrideTPPopupsForPerformanceTest && (trackingProtectionOnboardingCount < trackingProtectionOnboardingMaximumCount && - !trackingProtectionOnboardingShownThisSession) + !trackingProtectionOnboardingShownThisSession) var showSecretDebugMenuThisSession = false @@ -418,14 +428,14 @@ class Settings private constructor( BrowsingMode.Normal } } - set(value) { val lastKnownModeWasPrivate = (value == BrowsingMode.Private) preferences.edit() .putBoolean( - appContext.getPreferenceKey(R.string.pref_key_last_known_mode_private), - lastKnownModeWasPrivate) + appContext.getPreferenceKey(R.string.pref_key_last_known_mode_private), + lastKnownModeWasPrivate + ) .apply() field = value @@ -495,7 +505,9 @@ class Settings private constructor( } val accessibilityServicesEnabled: Boolean - get() { return touchExplorationIsEnabled || switchServiceIsEnabled } + get() { + return touchExplorationIsEnabled || switchServiceIsEnabled + } val toolbarSettingString: String get() = when { @@ -569,22 +581,41 @@ class Settings private constructor( default = false ) - val shouldShowFirstTimePwaFragment: Boolean + fun incrementVisitedInstallableCount() { + preferences.edit().putInt( + appContext.getPreferenceKey(R.string.pref_key_install_pwa_visits), + pwaInstallableVisitCount + 1 + ).apply() + } + + @VisibleForTesting(otherwise = PRIVATE) + internal val pwaInstallableVisitCount by intPreference( + appContext.getPreferenceKey(R.string.pref_key_install_pwa_visits), + default = 0 + ) + + private val userNeedsToVisitInstallableSites: Boolean + get() = pwaInstallableVisitCount < pwaVisitsToShowPromptMaxCount + + val shouldShowPwaOnboarding: Boolean get() { + // We only want to show this on the 3rd time a user visits a site + if (userNeedsToVisitInstallableSites) return false + // ShortcutManager::pinnedShortcuts is only available on Oreo+ - if (!userKnowsAboutPWAs && Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { - val alreadyHavePWaInstalled = + if (!userKnowsAboutPwas && Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { + val alreadyHavePwaInstalled = appContext.getSystemService(ShortcutManager::class.java) .pinnedShortcuts.size > 0 // Users know about PWAs onboarding if they already have PWAs installed. - userKnowsAboutPWAs = alreadyHavePWaInstalled + userKnowsAboutPwas = alreadyHavePwaInstalled } // Show dialog only if user does not know abut PWAs - return !userKnowsAboutPWAs + return !userKnowsAboutPwas } - var userKnowsAboutPWAs by booleanPreference( + var userKnowsAboutPwas by booleanPreference( appContext.getPreferenceKey(R.string.pref_key_user_knows_about_pwa), default = false ) @@ -809,8 +840,12 @@ class Settings private constructor( var savedLoginsSortingStrategy: SortingStrategy get() { return when (savedLoginsSortingStrategyString) { - SavedLoginsFragment.SORTING_STRATEGY_ALPHABETICALLY -> SortingStrategy.Alphabetically(appContext) - SavedLoginsFragment.SORTING_STRATEGY_LAST_USED -> SortingStrategy.LastUsed(appContext) + SavedLoginsFragment.SORTING_STRATEGY_ALPHABETICALLY -> SortingStrategy.Alphabetically( + appContext + ) + SavedLoginsFragment.SORTING_STRATEGY_LAST_USED -> SortingStrategy.LastUsed( + appContext + ) else -> SortingStrategy.Alphabetically(appContext) } } diff --git a/app/src/main/res/layout/fragment_pwa_first_time.xml b/app/src/main/res/layout/fragment_pwa_onboarding.xml similarity index 98% rename from app/src/main/res/layout/fragment_pwa_first_time.xml rename to app/src/main/res/layout/fragment_pwa_onboarding.xml index 395a933a3..1c188eb89 100644 --- a/app/src/main/res/layout/fragment_pwa_first_time.xml +++ b/app/src/main/res/layout/fragment_pwa_onboarding.xml @@ -10,7 +10,7 @@ android:layout_height="match_parent" android:background="@drawable/scrim_background" android:fitsSystemWindows="true" - tools:context="org.mozilla.fenix.shortcut.FirstTimePwaFragment"> + tools:context="org.mozilla.fenix.shortcut.PwaOnboardingDialogFragment"> + android:id="@+id/action_browserFragment_to_pwaOnboardingDialogFragment" + app:destination="@id/pwaOnboardingDialogFragment" /> @@ -680,10 +680,9 @@ android:name="org.mozilla.fenix.shortcut.CreateShortcutFragment" tools:layout="@layout/fragment_create_shortcut" /> + android:id="@+id/pwaOnboardingDialogFragment" + android:name="org.mozilla.fenix.shortcut.PwaOnboardingDialogFragment" + tools:layout="@layout/fragment_pwa_onboarding" /> pref_key_private_mode_opened pref_key_open_in_app_opened pref_key_install_pwa_opened + pref_key_install_pwa_visits pref_key_telemetry diff --git a/app/src/test/java/org/mozilla/fenix/utils/SettingsTest.kt b/app/src/test/java/org/mozilla/fenix/utils/SettingsTest.kt index 79f90813e..789a58954 100644 --- a/app/src/test/java/org/mozilla/fenix/utils/SettingsTest.kt +++ b/app/src/test/java/org/mozilla/fenix/utils/SettingsTest.kt @@ -351,6 +351,31 @@ class SettingsTest { assertTrue(settings.shouldShowSearchSuggestions) } + @Test + fun showPwaFragment() { + // When just created + // Then + assertFalse(settings.shouldShowPwaOnboarding) + + // When visited once + settings.incrementVisitedInstallableCount() + + // Then + assertFalse(settings.shouldShowPwaOnboarding) + + // When visited twice + settings.incrementVisitedInstallableCount() + + // Then + assertFalse(settings.shouldShowPwaOnboarding) + + // When visited thrice + settings.incrementVisitedInstallableCount() + + // Then + assertTrue(settings.shouldShowPwaOnboarding) + } + @Test fun sitePermissionsPhoneFeatureCameraAction() { // When just created From e1ef5f55eca96f1e9b1d80cf6d9e4d90d250b31a Mon Sep 17 00:00:00 2001 From: ekager Date: Wed, 15 Jul 2020 16:31:51 -0400 Subject: [PATCH 07/17] For #12453 - Sets secure flags on private tab of tabs tray --- .../fenix/tabtray/TabTrayDialogFragment.kt | 21 +++++++++++++++++-- .../org/mozilla/fenix/tabtray/TabTrayView.kt | 11 ++-------- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayDialogFragment.kt b/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayDialogFragment.kt index 62f5cacf7..b7340d1d4 100644 --- a/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayDialogFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayDialogFragment.kt @@ -9,6 +9,7 @@ import android.os.Bundle import android.view.LayoutInflater import android.view.View import android.view.ViewGroup +import android.view.WindowManager import androidx.appcompat.app.AppCompatDialogFragment import androidx.core.view.isVisible import androidx.core.view.updatePadding @@ -25,6 +26,7 @@ import mozilla.components.browser.session.SessionManager import mozilla.components.browser.state.selector.normalTabs import mozilla.components.browser.state.selector.privateTabs import mozilla.components.browser.state.state.BrowserState +import mozilla.components.browser.state.state.TabSessionState import mozilla.components.feature.tab.collections.TabCollection import mozilla.components.feature.tabs.TabsUseCases import mozilla.components.feature.tabs.tabstray.TabsFeature @@ -37,6 +39,7 @@ import org.mozilla.fenix.components.TabCollectionStorage import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.requireComponents +import org.mozilla.fenix.ext.settings import org.mozilla.fenix.utils.allowUndo @SuppressWarnings("TooManyFunctions", "LargeClass") @@ -48,7 +51,7 @@ class TabTrayDialogFragment : AppCompatDialogFragment() { private val snackbarAnchor: View? get() = if (tabTrayView.fabView.new_tab_button.isVisible) tabTrayView.fabView.new_tab_button - else null + else null private val collectionStorageObserver = object : TabCollectionStorage.Observer { override fun onCollectionCreated(title: String, sessions: List) { @@ -131,7 +134,13 @@ class TabTrayDialogFragment : AppCompatDialogFragment() { startingInLandscape = requireContext().resources.configuration.orientation == Configuration.ORIENTATION_LANDSCAPE, lifecycleScope = viewLifecycleOwner.lifecycleScope - ) { tabsFeature.get()?.filterTabs(it) } + ) { private -> + val filter: (TabSessionState) -> Boolean = { state -> private == state.content.private } + + tabsFeature.get()?.filterTabs(filter) + + setSecureFlagsIfNeeded(private) + } tabsFeature.set( TabsFeature( @@ -171,6 +180,14 @@ class TabTrayDialogFragment : AppCompatDialogFragment() { } } + private fun setSecureFlagsIfNeeded(private: Boolean) { + if (private && context?.settings()?.allowScreenshotsInPrivateMode == false) { + dialog?.window?.addFlags(WindowManager.LayoutParams.FLAG_SECURE) + } else if (!(activity as HomeActivity).browsingModeManager.mode.isPrivate) { + dialog?.window?.clearFlags(WindowManager.LayoutParams.FLAG_SECURE) + } + } + private fun showUndoSnackbarForTab(sessionId: String) { val sessionManager = view?.context?.components?.core?.sessionManager val snapshot = sessionManager diff --git a/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayView.kt b/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayView.kt index f82e5f923..b9acc4858 100644 --- a/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayView.kt +++ b/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayView.kt @@ -27,7 +27,6 @@ import mozilla.components.browser.menu.item.SimpleBrowserMenuItem import mozilla.components.browser.state.selector.normalTabs import mozilla.components.browser.state.selector.privateTabs import mozilla.components.browser.state.state.BrowserState -import mozilla.components.browser.state.state.TabSessionState import mozilla.components.browser.tabstray.BrowserTabsTray import org.mozilla.fenix.R import org.mozilla.fenix.components.metrics.Event @@ -44,7 +43,7 @@ class TabTrayView( isPrivate: Boolean, startingInLandscape: Boolean, lifecycleScope: LifecycleCoroutineScope, - private val filterTabs: ((TabSessionState) -> Boolean) -> Unit + private val filterTabs: (Boolean) -> Unit ) : LayoutContainer, TabLayout.OnTabSelectedListener { val fabView = LayoutInflater.from(container.context) .inflate(R.layout.component_tabstray_fab, container, true) @@ -204,14 +203,8 @@ class TabTrayView( } override fun onTabSelected(tab: TabLayout.Tab?) { - // We need a better way to determine which tab was selected. - val filter: (TabSessionState) -> Boolean = when (tab?.position) { - 1 -> { state -> state.content.private } - else -> { state -> !state.content.private } - } - toggleFabText(isPrivateModeSelected) - filterTabs.invoke(filter) + filterTabs.invoke(isPrivateModeSelected) updateState(view.context.components.core.store.state) scrollToTab(view.context.components.core.store.state.selectedTabId) From 3ac26270641b24548e09d33c0ff7114ca26d1785 Mon Sep 17 00:00:00 2001 From: ekager Date: Fri, 17 Jul 2020 12:35:45 -0400 Subject: [PATCH 08/17] Update Android Components to 51.0.20200717130954 --- buildSrc/src/main/java/AndroidComponents.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/buildSrc/src/main/java/AndroidComponents.kt b/buildSrc/src/main/java/AndroidComponents.kt index 2968a13d6..83590512e 100644 --- a/buildSrc/src/main/java/AndroidComponents.kt +++ b/buildSrc/src/main/java/AndroidComponents.kt @@ -3,5 +3,5 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ object AndroidComponents { - const val VERSION = "51.0.20200716130034" + const val VERSION = "51.0.20200717130954" } From 455e7b8f99ad260ddee95953b8dcd1133599cd7e Mon Sep 17 00:00:00 2001 From: ekager Date: Fri, 17 Jul 2020 13:40:08 -0400 Subject: [PATCH 09/17] Update ContextMenuUseCases to just take a store --- app/src/main/java/org/mozilla/fenix/components/UseCases.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/org/mozilla/fenix/components/UseCases.kt b/app/src/main/java/org/mozilla/fenix/components/UseCases.kt index 08469e9c3..fba92eaaa 100644 --- a/app/src/main/java/org/mozilla/fenix/components/UseCases.kt +++ b/app/src/main/java/org/mozilla/fenix/components/UseCases.kt @@ -63,7 +63,7 @@ class UseCases( val downloadUseCases by lazy { DownloadsUseCases(store) } - val contextMenuUseCases by lazy { ContextMenuUseCases(sessionManager, store) } + val contextMenuUseCases by lazy { ContextMenuUseCases(store) } val engineSessionUseCases by lazy { EngineSessionUseCases(sessionManager) } From aa7655f4d687b5ea97d5418143d3baced489a6bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hakk=C4=B1=20Kaan=20=C3=87al=C4=B1=C5=9Fkan?= Date: Tue, 14 Jul 2020 11:30:08 +0300 Subject: [PATCH 10/17] For #12509: Set height of remove add on button to 36dp --- .../mozilla/fenix/addons/InstalledAddonDetailsFragment.kt | 4 ++-- .../main/res/layout/fragment_installed_add_on_details.xml | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/addons/InstalledAddonDetailsFragment.kt b/app/src/main/java/org/mozilla/fenix/addons/InstalledAddonDetailsFragment.kt index 57f709748..3a946c236 100644 --- a/app/src/main/java/org/mozilla/fenix/addons/InstalledAddonDetailsFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/addons/InstalledAddonDetailsFragment.kt @@ -8,13 +8,13 @@ import android.os.Bundle import android.view.LayoutInflater import android.view.View import android.view.ViewGroup -import android.widget.Switch import androidx.core.view.isVisible import androidx.fragment.app.Fragment import androidx.lifecycle.lifecycleScope import androidx.navigation.Navigation import androidx.navigation.findNavController import androidx.navigation.fragment.findNavController +import com.google.android.material.switchmaterial.SwitchMaterial import kotlinx.android.synthetic.main.fragment_installed_add_on_details.view.* import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch @@ -301,7 +301,7 @@ class InstalledAddonDetailsFragment : Fragment() { view.remove_add_on.isClickable = clickable } - private fun Switch.setState(checked: Boolean) { + private fun SwitchMaterial.setState(checked: Boolean) { val text = if (checked) { R.string.mozac_feature_addons_enabled } else { diff --git a/app/src/main/res/layout/fragment_installed_add_on_details.xml b/app/src/main/res/layout/fragment_installed_add_on_details.xml index c143f53cc..d32b326f4 100644 --- a/app/src/main/res/layout/fragment_installed_add_on_details.xml +++ b/app/src/main/res/layout/fragment_installed_add_on_details.xml @@ -19,7 +19,7 @@ android:visibility="gone" android:orientation="vertical"> - - -