blallo
/
fenix
1
0
Fork 0
Code Releases Activity

Fix how we reflect FxA state in preference 

This cleans up how we're displaying account state in the main preference UI.
Before when it worked, it worked mostly accidentally.
'launch' wrapper around "update ui" methods would trigger a race condition
between binding the account pref view holder and actually updating that view
with values. Sometimes the "update view with values" would happen after view
was bound, and the UI will be correct. Most of the time it would happen before,
and so there will be nothing to update and we'd get into an inconsistent state.

This also splits up the "accountpreference" into two: account is good,
and account needs re-auth. This greatly simplifies their management.
master
Grisha Kruglov 2019-06-03 18:58:39 -07:00 committed by Jeff Boek
parent 0e1d81126d
commit fe3c163a20
10 changed files with 202 additions and 163 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
app/src/main/java/org/mozilla/fenix/components/toolbar/DefaultToolbarMenu.kt
View File

@ -116,7 +116,7 @@ class DefaultToolbarMenu(
highlight = if (hasAccountProblem) {
BrowserMenuHighlightableItem.Highlight(
imageResource = R.drawable.ic_alert,
backgroundResource = R.color.sync_error_color
backgroundResource = R.color.sync_error_background_color
)
} else null
) {

35
app/src/main/java/org/mozilla/fenix/settings/AccountAuthErrorPreference.kt 100644
View File

@ -0,0 +1,35 @@
/* 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
import android.content.Context
import android.util.AttributeSet
import android.view.View
import android.widget.TextView
import androidx.preference.Preference
import androidx.preference.PreferenceViewHolder
import org.mozilla.fenix.R
class AccountAuthErrorPreference : Preference {
var email: String? = null
constructor(context: Context) : super(context)
constructor(context: Context, attrs: AttributeSet?) : super(context, attrs)
constructor(context: Context, attrs: AttributeSet?, attributeSetId: Int) : super(context, attrs, attributeSetId)
init {
layoutResource = R.layout.account_auth_error_preference
}
override fun onBindViewHolder(holder: PreferenceViewHolder) {
super.onBindViewHolder(holder)
val emailView = holder.findViewById(R.id.email) as TextView
emailView.text = email.orEmpty()
emailView.visibility = when (email.isNullOrEmpty()) {
true -> View.GONE
false -> View.VISIBLE
}
}
}

31
app/src/main/java/org/mozilla/fenix/settings/AccountPreference.kt
View File

@ -7,26 +7,18 @@ package org.mozilla.fenix.settings
import android.content.Context
import android.util.AttributeSet
import android.view.View
import android.widget.ImageView
import android.widget.TextView
import androidx.preference.Preference
import androidx.preference.PreferenceViewHolder
import org.mozilla.fenix.R
class AccountPreference : Preference {
var title: TextView? = null
var summary: TextView? = null
var errorIcon: ImageView? = null
var background: View? = null
var displayName: String? = null
var email: String? = null
constructor(context: Context) : super(context)
constructor(context: Context, attrs: AttributeSet?) : super(context, attrs)
constructor(context: Context, attrs: AttributeSet?, attributeSetId: Int) : super(
context,
attrs,
attributeSetId
)
constructor(context: Context, attrs: AttributeSet?, attributeSetId: Int) : super(context, attrs, attributeSetId)
init {
layoutResource = R.layout.account_preference
@ -34,9 +26,18 @@ class AccountPreference : Preference {
override fun onBindViewHolder(holder: PreferenceViewHolder) {
super.onBindViewHolder(holder)
title = holder.findViewById(R.id.title) as TextView
summary = holder.findViewById(R.id.summary) as TextView
errorIcon = holder.findViewById(R.id.error_icon) as ImageView
background = holder.itemView
val displayNameView = holder.findViewById(R.id.displayName) as TextView
val emailView = holder.findViewById(R.id.email) as TextView
displayNameView.text = displayName.orEmpty()
displayNameView.visibility = when (displayName.isNullOrEmpty()) {
true -> View.GONE
false -> View.VISIBLE
}
// There is a potential for a race condition here. We might not have the user profile by the time we display
// this field, in which case we won't have the email address (or the display name, but that we may just not have
// at all even after fetching the profile). We don't hide the email field or change its text if email is missing
// because in the layout a default value ("Firefox Account") is specified, which will be displayed instead.
email?.let { emailView.text = it }
}
}

188
app/src/main/java/org/mozilla/fenix/settings/SettingsFragment.kt
View File

@ -5,15 +5,14 @@
package org.mozilla.fenix.settings
import android.content.ActivityNotFoundException
import android.content.Context
import android.content.Intent
import android.content.SharedPreferences
import android.net.Uri
import android.os.Bundle
import android.provider.Settings
import android.view.View
import android.widget.Toast
import androidx.appcompat.app.AppCompatActivity
import androidx.core.content.ContextCompat
import androidx.navigation.Navigation
import androidx.preference.Preference
import androidx.preference.Preference.OnPreferenceClickListener
@ -21,12 +20,10 @@ import androidx.preference.PreferenceCategory
import androidx.preference.PreferenceFragmentCompat
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.Job
import kotlinx.coroutines.launch
import mozilla.components.concept.sync.AccountObserver
import mozilla.components.concept.sync.OAuthAccount
import mozilla.components.concept.sync.Profile
import mozilla.components.service.fxa.FxaUnauthorizedException
import org.mozilla.fenix.BrowserDirection
import org.mozilla.fenix.Config
import org.mozilla.fenix.FenixApplication
@ -35,6 +32,7 @@ import org.mozilla.fenix.R
import org.mozilla.fenix.R.string.pref_key_about
import org.mozilla.fenix.R.string.pref_key_accessibility
import org.mozilla.fenix.R.string.pref_key_account
import org.mozilla.fenix.R.string.pref_key_account_auth_error
import org.mozilla.fenix.R.string.pref_key_account_category
import org.mozilla.fenix.R.string.pref_key_data_choices
import org.mozilla.fenix.R.string.pref_key_delete_browsing_data
@ -53,18 +51,12 @@ import org.mozilla.fenix.R.string.pref_key_tracking_protection_settings
import org.mozilla.fenix.R.string.pref_key_your_rights
import org.mozilla.fenix.components.metrics.Event
import org.mozilla.fenix.ext.components
import org.mozilla.fenix.ext.getColorFromAttr
import org.mozilla.fenix.ext.getPreferenceKey
import org.mozilla.fenix.ext.requireComponents
import org.mozilla.fenix.utils.ItsNotBrokenSnack
import kotlin.coroutines.CoroutineContext
@SuppressWarnings("TooManyFunctions", "LargeClass")
class SettingsFragment : PreferenceFragmentCompat(), CoroutineScope, AccountObserver {
private lateinit var job: Job
override val coroutineContext: CoroutineContext
get() = Dispatchers.Main + job
class SettingsFragment : PreferenceFragmentCompat(), AccountObserver {
private val preferenceChangeListener =
SharedPreferences.OnSharedPreferenceChangeListener { sharedPreferences, key ->
try {
@ -83,10 +75,15 @@ class SettingsFragment : PreferenceFragmentCompat(), CoroutineScope, AccountObse
override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
job = Job()
setupAccountUI()
updateSignInVisibility()
displayAccountErrorIfNecessary()
// Observe account changes to keep the UI up-to-date.
requireComponents.backgroundServices.accountManager.register(this, owner = this, autoPause = true)
// It's important to update the account UI state in onCreate, even though we also call it in onResume, since
// that ensures we'll never display an incorrect state in the UI. For example, if user is signed-in, and we
// don't perform this call in onCreate, we'll briefly display a "Sign In" preference, which will then get
// replaced by the correct account information once this call is ran in onResume shortly after.
updateAccountUIState(context!!, requireComponents.backgroundServices.accountManager.accountProfile())
preferenceManager.sharedPreferences.registerOnSharedPreferenceChangeListener(preferenceChangeListener)
}
@ -131,9 +128,8 @@ class SettingsFragment : PreferenceFragmentCompat(), CoroutineScope, AccountObse
aboutPreference?.title = getString(R.string.preferences_about, appName)
setupPreferences()
setupAccountUI()
updateSignInVisibility()
displayAccountErrorIfNecessary()
updateAccountUIState(context!!, requireComponents.backgroundServices.accountManager.accountProfile())
}
@Suppress("ComplexMethod")
@ -182,11 +178,10 @@ class SettingsFragment : PreferenceFragmentCompat(), CoroutineScope, AccountObse
navigateToAbout()
}
resources.getString(pref_key_account) -> {
if (requireComponents.backgroundServices.accountManager.accountNeedsReauth()) {
navigateToAccountProblem()
} else {
navigateToAccountSettings()
}
navigateToAccountSettings()
}
resources.getString(pref_key_account_auth_error) -> {
navigateToAccountProblem()
}
resources.getString(pref_key_delete_browsing_data) -> {
navigateToDeleteBrowsingData()
@ -215,19 +210,9 @@ class SettingsFragment : PreferenceFragmentCompat(), CoroutineScope, AccountObse
override fun onDestroy() {
super.onDestroy()
job.cancel()
preferenceManager.sharedPreferences.unregisterOnSharedPreferenceChangeListener(preferenceChangeListener)
}
private fun setupAccountUI() {
val accountManager = requireComponents.backgroundServices.accountManager
// Observe account changes to keep the UI up-to-date.
accountManager.register(this, owner = this)
updateAuthState(accountManager.authenticatedAccount())
accountManager.accountProfile()?.let { updateAccountProfile(it) }
}
private fun getClickListenerForSignIn(): OnPreferenceClickListener {
return OnPreferenceClickListener {
val directions = SettingsFragmentDirections.actionSettingsFragmentToTurnOnSyncFragment()
@ -335,116 +320,85 @@ class SettingsFragment : PreferenceFragmentCompat(), CoroutineScope, AccountObse
}
override fun onAuthenticated(account: OAuthAccount) {
updateAuthState(account)
updateSignInVisibility()
}
override fun onError(error: Exception) {
// TODO we could display some error states in this UI.
when (error) {
is FxaUnauthorizedException -> {
CoroutineScope(Dispatchers.Main).launch {
context?.let {
updateAccountUIState(it, it.components.backgroundServices.accountManager.accountProfile())
}
}
}
override fun onError(error: Exception) {}
override fun onLoggedOut() {
updateAuthState()
updateSignInVisibility()
CoroutineScope(Dispatchers.Main).launch {
context?.let {
updateAccountUIState(it, it.components.backgroundServices.accountManager.accountProfile())
}
}
}
override fun onProfileUpdated(profile: Profile) {
updateAccountProfile(profile)
CoroutineScope(Dispatchers.Main).launch {
context?.let {
updateAccountUIState(it, profile)
}
}
}
override fun onAuthenticationProblems() {
displayAccountErrorIfNecessary()
CoroutineScope(Dispatchers.Main).launch {
context?.let {
updateAccountUIState(it, it.components.backgroundServices.accountManager.accountProfile())
}
}
}
// --- Account UI helpers ---
private fun updateAuthState(account: OAuthAccount? = null) {
// Cache the user's auth state to improve performance of sign in visibility
org.mozilla.fenix.utils.Settings.getInstance(context!!).setHasCachedAccount(account != null)
}
private fun updateSignInVisibility() {
val hasCachedAccount = org.mozilla.fenix.utils.Settings.getInstance(context!!).hasCachedAccount
/**
* Updates the UI to reflect current account state.
* Possible conditions are logged-in without problems, logged-out, and logged-in but needs to re-authenticate.
*/
private fun updateAccountUIState(context: Context, profile: Profile?) {
val preferenceSignIn =
findPreference<Preference>(context!!.getPreferenceKey(pref_key_sign_in))
findPreference<Preference>(context.getPreferenceKey(pref_key_sign_in))
val preferenceFirefoxAccount =
findPreference<Preference>(context!!.getPreferenceKey(pref_key_account))
findPreference<AccountPreference>(context.getPreferenceKey(pref_key_account))
val preferenceFirefoxAccountAuthError =
findPreference<AccountAuthErrorPreference>(context.getPreferenceKey(pref_key_account_auth_error))
val accountPreferenceCategory =
findPreference<PreferenceCategory>(context!!.getPreferenceKey(pref_key_account_category))
findPreference<PreferenceCategory>(context.getPreferenceKey(pref_key_account_category))
if (hasCachedAccount) {
val accountManager = requireComponents.backgroundServices.accountManager
val account = accountManager.authenticatedAccount()
// Signed-in, no problems.
if (account != null && !accountManager.accountNeedsReauth()) {
preferenceSignIn?.isVisible = false
preferenceSignIn?.onPreferenceClickListener = null
preferenceFirefoxAccountAuthError?.isVisible = false
preferenceFirefoxAccount?.isVisible = true
accountPreferenceCategory?.isVisible = true
preferenceFirefoxAccount?.displayName = profile?.displayName
preferenceFirefoxAccount?.email = profile?.email
// Signed-in, need to re-authenticate.
} else if (account != null && accountManager.accountNeedsReauth()) {
preferenceFirefoxAccount?.isVisible = false
preferenceFirefoxAccountAuthError?.isVisible = true
accountPreferenceCategory?.isVisible = true
preferenceSignIn?.isVisible = false
preferenceSignIn?.onPreferenceClickListener = null
preferenceFirefoxAccountAuthError?.email = profile?.email
// Signed-out.
} else {
preferenceSignIn?.isVisible = true
preferenceSignIn?.onPreferenceClickListener = getClickListenerForSignIn()
preferenceFirefoxAccount?.isVisible = false
preferenceFirefoxAccountAuthError?.isVisible = false
accountPreferenceCategory?.isVisible = false
}
}
private fun updateAccountProfile(profile: Profile) {
launch {
context?.let { context ->
val preferenceFirefoxAccount =
findPreference<AccountPreference>(context.getPreferenceKey(pref_key_account))
preferenceFirefoxAccount?.title?.setTextColor(
R.attr.primaryText.getColorFromAttr(context)
)
preferenceFirefoxAccount?.title?.text = profile.displayName.orEmpty()
preferenceFirefoxAccount?.title?.visibility =
if (preferenceFirefoxAccount?.title?.text.isNullOrEmpty()) View.GONE else View.VISIBLE
preferenceFirefoxAccount?.summary?.setTextColor(
R.attr.primaryText.getColorFromAttr(context)
)
preferenceFirefoxAccount?.summary?.text = profile.email.orEmpty()
preferenceFirefoxAccount?.icon = ContextCompat.getDrawable(context, R.drawable.ic_shortcuts)
preferenceFirefoxAccount?.errorIcon?.visibility = View.GONE
preferenceFirefoxAccount?.background?.background = null
}
}
}
private fun displayAccountErrorIfNecessary() {
launch {
context?.let { context ->
if (!context.components.backgroundServices.accountManager.accountNeedsReauth()) { return@launch }
val preferenceFirefoxAccount =
findPreference<AccountPreference>(context.getPreferenceKey(pref_key_account))
preferenceFirefoxAccount?.title?.setTextColor(
ContextCompat.getColor(
context,
R.color.sync_error_text_color
)
)
preferenceFirefoxAccount?.title?.text = context.getString(R.string.preferences_account_sync_error)
preferenceFirefoxAccount?.title?.visibility =
if (preferenceFirefoxAccount?.title?.text.isNullOrEmpty()) View.GONE else View.VISIBLE
preferenceFirefoxAccount?.summary?.setTextColor(
ContextCompat.getColor(
context,
R.color.sync_error_text_color
)
)
preferenceFirefoxAccount?.summary?.text =
context.components.backgroundServices.accountManager.accountProfile()?.email.orEmpty()
preferenceFirefoxAccount?.icon = ContextCompat.getDrawable(context, R.drawable.ic_account_warning)
preferenceFirefoxAccount?.errorIcon?.visibility = View.VISIBLE
preferenceFirefoxAccount?.background?.background =
ContextCompat.getDrawable(context, R.color.sync_error_color)
}
}
}
}

9
app/src/main/java/org/mozilla/fenix/utils/Settings.kt
View File

@ -152,9 +152,6 @@ class Settings private constructor(context: Context) {
else -> appContext.getString(R.string.preference_light_theme)
}
val hasCachedAccount: Boolean
get() = preferences.getBoolean(appContext.getPreferenceKey(R.string.pref_key_cached_account), false)
private val autoBounceQuickActionSheetCount: Int
get() = (preferences.getInt(appContext.getPreferenceKey(R.string.pref_key_bounce_quick_action), 0))
@ -229,12 +226,6 @@ class Settings private constructor(context: Context) {
)
}
fun setHasCachedAccount(isCached: Boolean) {
preferences.edit()
.putBoolean(appContext.getPreferenceKey(R.string.pref_key_cached_account), isCached)
.apply()
}
private val SitePermissionsRules.Action.id: Int
get() {
return when (this) {

67
app/src/main/res/layout/account_auth_error_preference.xml 100644
View File

@ -0,0 +1,67 @@
<?xml version="1.0" encoding="utf-8"?>
<!-- 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/. -->
<LinearLayout
xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:app="http://schemas.android.com/apk/res-auto"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:gravity="center_vertical"
android:paddingStart="16dp"
android:paddingEnd="16dp"
android:background="@color/sync_error_background_color">
<FrameLayout
android:id="@+id/icon_frame"
android:layout_width="wrap_content"
android:layout_height="wrap_content">
<androidx.preference.internal.PreferenceImageView
android:id="@android:id/icon"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
app:maxWidth="48dp"
app:maxHeight="48dp"/>
</FrameLayout>
<RelativeLayout
android:layout_width="0dp"
android:layout_height="wrap_content"
android:layout_marginStart="15dp"
android:layout_marginEnd="6dp"
android:paddingStart="16dp"
android:paddingTop="16dp"
android:paddingBottom="16dp"
android:layout_weight="1">
<TextView
android:id="@+id/errorSummary"
android:text="@string/preferences_account_sync_error"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:textColor="@color/sync_error_text_color"
android:textSize="16sp"
android:ellipsize="marquee"
android:fadingEdge="horizontal"
android:visibility="visible"/>
<TextView
android:id="@+id/email"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_below="@id/errorSummary"
android:layout_alignStart="@id/errorSummary"
android:textColor="@color/sync_error_text_color"
android:maxLines="4"
android:visibility="gone"/>
</RelativeLayout>
<LinearLayout
android:id="@android:id/widget_frame"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:gravity="center_vertical"
android:orientation="vertical"/>
</LinearLayout>

22
app/src/main/res/layout/account_preference.xml
View File

@ -35,21 +35,20 @@
android:paddingBottom="16dp"
android:layout_weight="1">
<TextView android:id="@+id/title"
<TextView android:id="@+id/displayName"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:textColor="?primaryText"
android:singleLine="true"
android:textSize="16sp"
android:ellipsize="marquee"
android:fadingEdge="horizontal"
android:visibility="gone"/>
<TextView android:id="@+id/summary"
<TextView android:id="@+id/email"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_below="@id/title"
android:layout_alignStart="@id/title"
android:layout_below="@id/displayName"
android:layout_alignStart="@id/displayName"
android:textColor="?primaryText"
android:text="@string/preferences_account_default_name"
android:maxLines="4"/>
@ -62,17 +61,4 @@
android:gravity="center_vertical"
android:orientation="vertical"/>
<ImageView
xmlns:android="http://schemas.android.com/apk/res/android"
android:id="@+id/error_icon"
android:src="@drawable/ic_alert"
android:padding="16dp"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:clickable="false"
android:focusable="false"
android:gravity="center_vertical"
android:orientation="vertical"
android:visibility="gone"/>
</LinearLayout>

2
app/src/main/res/values/colors.xml
View File

@ -139,7 +139,7 @@
<color name="disabled_text">#cccccc</color>
<color name="violet_05">#E7DFFF</color>
<color name="text_scale_example_text_color">#232749</color>
<color name="sync_error_color">#FFF36E</color>
<color name="sync_error_background_color">#FFF36E</color>
<color name="sync_error_text_color">#960E18</color>
<!-- Reader View colors -->

1
app/src/main/res/values/preference_keys.xml
View File

@ -22,6 +22,7 @@
<string name="pref_key_your_rights" translatable="false">pref_key_your_rights</string>
<string name="pref_key_account" translatable="false">pref_key_account</string>
<string name="pref_key_sign_in" translatable="false">pref_key_sign_in</string>
<string name="pref_key_account_auth_error" translatable="false">pref_key_account_auth_error</string>
<string name="pref_key_private_mode" translatable="false">pref_key_private_mode</string>
<string name="pref_key_theme" translatable="false">pref_key_theme</string>
<string name="pref_key_leakcanary" translatable="false">pref_key_leakcanary</string>

8
app/src/main/res/xml/preferences.xml
View File

@ -14,14 +14,18 @@
android:title="@string/preferences_sign_in" />
<androidx.preference.PreferenceCategory
android:key = "@string/pref_key_account_category"
android:key="@string/pref_key_account_category"
android:title="@string/preferences_category_account"
app:iconSpaceReserved="false"
app:isPreferenceVisible="false">
<org.mozilla.fenix.settings.AccountPreference
android:icon="@drawable/ic_shortcuts"
android:key="@string/pref_key_account"/>
android:key="@string/pref_key_account" />
<org.mozilla.fenix.settings.AccountAuthErrorPreference
android:icon="@drawable/ic_account_warning"
android:key="@string/pref_key_account_auth_error"/>
</androidx.preference.PreferenceCategory>
<androidx.preference.PreferenceCategory