Combine settings for navigation action after removing a message

app/core/src/main/java/com/fsck/k9/K9.kt

@@ -183,11 +183,7 @@ object K9 : EarlyInit {
     @JvmStatic
     var isUseMessageViewFixedWidthFont = false
 
-    @JvmStatic
-    var isMessageViewReturnToList = false
-
-    @JvmStatic
-    var isMessageViewShowNext = false
+    var messageViewPostRemoveNavigation: PostRemoveNavigation = PostRemoveNavigation.ReturnToMessageList
 
     @JvmStatic
     var isUseVolumeKeysForNavigation = false
@@ -331,8 +327,8 @@ object K9 : EarlyInit {
         isChangeContactNameColor = storage.getBoolean("changeRegisteredNameColor", false)
         contactNameColor = storage.getInt("registeredNameColor", 0xFF1093F5.toInt())
         isUseMessageViewFixedWidthFont = storage.getBoolean("messageViewFixedWidthFont", false)
-        isMessageViewReturnToList = storage.getBoolean("messageViewReturnToList", false)
-        isMessageViewShowNext = storage.getBoolean("messageViewShowNext", false)
+        messageViewPostRemoveNavigation =
+            storage.getEnum("messageViewPostDeleteAction", PostRemoveNavigation.ReturnToMessageList)
         isHideUserAgent = storage.getBoolean("hideUserAgent", false)
         isHideTimeZone = storage.getBoolean("hideTimeZone", false)
 
@@ -405,8 +401,7 @@ object K9 : EarlyInit {
         editor.putBoolean("changeRegisteredNameColor", isChangeContactNameColor)
         editor.putInt("registeredNameColor", contactNameColor)
         editor.putBoolean("messageViewFixedWidthFont", isUseMessageViewFixedWidthFont)
-        editor.putBoolean("messageViewReturnToList", isMessageViewReturnToList)
-        editor.putBoolean("messageViewShowNext", isMessageViewShowNext)
+        editor.putEnum("messageViewPostDeleteAction", messageViewPostRemoveNavigation)
         editor.putBoolean("hideUserAgent", isHideUserAgent)
         editor.putBoolean("hideTimeZone", isHideTimeZone)
 
@@ -535,4 +530,14 @@ object K9 : EarlyInit {
         NEVER,
         WHEN_IN_LANDSCAPE,
     }
+
+    /**
+     * The navigation actions that can be to performed after the user has deleted or moved a message from the message
+     * view screen.
+     */
+    enum class PostRemoveNavigation {
+        ReturnToMessageList,
+        ShowPreviousMessage,
+        ShowNextMessage,
+    }
 }

app/core/src/main/java/com/fsck/k9/preferences/GeneralSettingsDescriptions.java

@@ -17,6 +17,7 @@
 import com.fsck.k9.K9;
 import com.fsck.k9.K9.BACKGROUND_OPS;
 import com.fsck.k9.K9.NotificationQuickDelete;
+import com.fsck.k9.K9.PostRemoveNavigation;
 import com.fsck.k9.K9.SplitViewMode;
 import com.fsck.k9.SwipeAction;
 import com.fsck.k9.UiDensity;
@@ -37,6 +38,7 @@
 import com.fsck.k9.preferences.upgrader.GeneralSettingsUpgraderTo58;
 import com.fsck.k9.preferences.upgrader.GeneralSettingsUpgraderTo69;
 import com.fsck.k9.preferences.upgrader.GeneralSettingsUpgraderTo79;
+import com.fsck.k9.preferences.upgrader.GeneralSettingsUpgraderTo89;
 
 import static com.fsck.k9.K9.LockScreenNotificationVisibility;
 
@@ -138,10 +140,12 @@ public class GeneralSettingsDescriptions {
                 new V(1, new BooleanSetting(false))
         ));
         s.put("messageViewReturnToList", Settings.versions(
-                new V(1, new BooleanSetting(false))
+                new V(1, new BooleanSetting(false)),
+                new V(89, null)
         ));
         s.put("messageViewShowNext", Settings.versions(
-                new V(1, new BooleanSetting(false))
+                new V(1, new BooleanSetting(false)),
+                new V(89, null)
         ));
         s.put("quietTimeEnabled", Settings.versions(
                 new V(1, new BooleanSetting(false))
@@ -285,6 +289,9 @@ public class GeneralSettingsDescriptions {
         s.put("fontSizeMessageViewAccountName", Settings.versions(
             new V(87, new FontSizeSetting(FontSizes.FONT_DEFAULT))
         ));
+        s.put("messageViewPostDeleteAction", Settings.versions(
+            new V(89, new EnumSetting<>(PostRemoveNavigation.class, PostRemoveNavigation.ReturnToMessageList))
+        ));
 
         SETTINGS = Collections.unmodifiableMap(s);
 
@@ -294,6 +301,7 @@ public class GeneralSettingsDescriptions {
         u.put(58, new GeneralSettingsUpgraderTo58());
         u.put(69, new GeneralSettingsUpgraderTo69());
         u.put(79, new GeneralSettingsUpgraderTo79());
+        u.put(89, new GeneralSettingsUpgraderTo89());
 
         UPGRADERS = Collections.unmodifiableMap(u);
     }

app/core/src/main/java/com/fsck/k9/preferences/Settings.java

@@ -36,7 +36,7 @@ public class Settings {
      *
      * @see SettingsExporter
      */
-    public static final int VERSION = 88;
+    public static final int VERSION = 89;
 
     static Map<String, Object> validate(int version, Map<String, TreeMap<Integer, SettingsDescription>> settings,
             Map<String, String> importedSettings, boolean useDefaultValues) {

app/core/src/main/java/com/fsck/k9/preferences/upgrader/GeneralSettingsUpgraderTo89.kt

@@ -0,0 +1,23 @@
+package com.fsck.k9.preferences.upgrader
+
+import com.fsck.k9.preferences.Settings.SettingsUpgrader
+
+/**
+ * Combine `messageViewReturnToList` and `messageViewShowNext` into `messageViewPostDeleteAction`.
+ */
+class GeneralSettingsUpgraderTo89 : SettingsUpgrader {
+    override fun upgrade(settings: MutableMap<String, Any>): Set<String> {
+        val messageViewReturnToList = settings["messageViewReturnToList"] as? Boolean
+        val messageViewShowNext = settings["messageViewShowNext"] as? Boolean
+
+        if (messageViewReturnToList == true) {
+            settings["messageViewPostDeleteAction"] = "ReturnToMessageList"
+        } else if (messageViewShowNext == true) {
+            settings["messageViewPostDeleteAction"] = "ShowNextMessage"
+        } else {
+            settings["messageViewPostDeleteAction"] = "ShowPreviousMessage"
+        }
+
+        return setOf("messageViewReturnToList", "messageViewShowNext")
+    }
+}

app/core/src/main/res/values/arrays_general_settings_values.xml

@@ -221,4 +221,10 @@
         <item>move</item>
     </string-array>
 
+    <string-array name="post_remove_navigation_values" translatable="false">
+        <item>ReturnToMessageList</item>
+        <item>ShowPreviousMessage</item>
+        <item>ShowNextMessage</item>
+    </string-array>
+
 </resources>

app/storage/src/main/java/com/fsck/k9/preferences/K9StoragePersister.java

@@ -21,7 +21,7 @@
 
 
 public class K9StoragePersister implements StoragePersister {
-    private static final int DB_VERSION = 20;
+    private static final int DB_VERSION = 21;
     private static final String DB_NAME = "preferences_storage";
 
     private final Context context;

app/storage/src/main/java/com/fsck/k9/preferences/migrations/StorageMigrationTo21.kt

@@ -0,0 +1,26 @@
+package com.fsck.k9.preferences.migrations
+
+import android.database.sqlite.SQLiteDatabase
+
+/**
+ * Combine `messageViewReturnToList` and `messageViewShowNext` into `messageViewPostDeleteAction`.
+ */
+class StorageMigrationTo21(
+    private val db: SQLiteDatabase,
+    private val migrationsHelper: StorageMigrationsHelper,
+) {
+    fun createPostRemoveNavigationSetting() {
+        val messageViewReturnToList = migrationsHelper.readValue(db, "messageViewReturnToList").toBoolean()
+        val messageViewShowNext = migrationsHelper.readValue(db, "messageViewShowNext").toBoolean()
+
+        val postRemoveNavigation = when {
+            messageViewReturnToList -> "ReturnToMessageList"
+            messageViewShowNext -> "ShowNextMessage"
+            else -> "ShowPreviousMessage"
+        }
+
+        migrationsHelper.writeValue(db, "messageViewPostDeleteAction", postRemoveNavigation)
+        migrationsHelper.writeValue(db, "messageViewReturnToList", null)
+        migrationsHelper.writeValue(db, "messageViewShowNext", null)
+    }
+}

app/storage/src/main/java/com/fsck/k9/preferences/migrations/StorageMigrations.kt

@@ -3,6 +3,7 @@ package com.fsck.k9.preferences.migrations
 import android.database.sqlite.SQLiteDatabase
 
 internal object StorageMigrations {
+    @Suppress("MagicNumber", "CyclomaticComplexMethod")
     @JvmStatic
     fun upgradeDatabase(db: SQLiteDatabase, migrationsHelper: StorageMigrationsHelper) {
         val oldVersion = db.version
@@ -26,5 +27,6 @@ internal object StorageMigrations {
         if (oldVersion < 18) StorageMigrationTo18(db, migrationsHelper).rewriteImapCompressionSettings()
         if (oldVersion < 19) StorageMigrationTo19(db, migrationsHelper).markGmailAccounts()
         if (oldVersion < 20) StorageMigrationTo20(db, migrationsHelper).fixIdentities()
+        if (oldVersion < 21) StorageMigrationTo21(db, migrationsHelper).createPostRemoveNavigationSetting()
     }
 }

app/ui/legacy/src/main/java/com/fsck/k9/activity/MessageList.kt

@@ -31,6 +31,7 @@ import app.k9mail.core.android.common.contact.ContactRepository
 import app.k9mail.feature.launcher.FeatureLauncherActivity
 import com.fsck.k9.Account
 import com.fsck.k9.K9
+import com.fsck.k9.K9.PostRemoveNavigation
 import com.fsck.k9.K9.SplitViewMode
 import com.fsck.k9.Preferences
 import com.fsck.k9.account.BackgroundAccountRemover
@@ -54,7 +55,6 @@ import com.fsck.k9.ui.managefolders.ManageFoldersActivity
 import com.fsck.k9.ui.messagelist.DefaultFolderProvider
 import com.fsck.k9.ui.messagelist.MessageListFragment
 import com.fsck.k9.ui.messagelist.MessageListFragment.MessageListFragmentListener
-import com.fsck.k9.ui.messageview.Direction
 import com.fsck.k9.ui.messageview.MessageViewContainerFragment
 import com.fsck.k9.ui.messageview.MessageViewContainerFragment.MessageViewContainerListener
 import com.fsck.k9.ui.messageview.MessageViewFragment.MessageViewFragmentListener
@@ -109,12 +109,6 @@ open class MessageList :
     private var search: LocalSearch? = null
     private var singleFolderMode = false
 
-    private val lastDirection: Direction
-        get() {
-            return messageViewContainerFragment?.lastDirection
-                ?: if (K9.isMessageViewShowNext) Direction.NEXT else Direction.PREVIOUS
-        }
-
     private var messageListActivityConfig: MessageListActivityConfig? = null
 
     /**
@@ -1151,9 +1145,11 @@ open class MessageList :
         messageListFragment.setActiveMessage(messageReference)
     }
 
-    override fun showNextMessageOrReturn() {
-        if (K9.isMessageViewReturnToList || !showLogicalNextMessage()) {
-            returnToMessageList()
+    override fun performNavigationAfterMessageRemoval() {
+        when (K9.messageViewPostRemoveNavigation) {
+            PostRemoveNavigation.ReturnToMessageList -> returnToMessageList()
+            PostRemoveNavigation.ShowPreviousMessage -> showPreviousMessageOrReturn()
+            PostRemoveNavigation.ShowNextMessage -> showNextMessageOrReturn()
         }
     }
 
@@ -1165,16 +1161,15 @@ open class MessageList :
         }
     }
 
-    private fun showLogicalNextMessage(): Boolean {
-        val couldMoveInLastDirection = when (lastDirection) {
-            Direction.NEXT -> showNextMessage()
-            Direction.PREVIOUS -> showPreviousMessage()
+    private fun showPreviousMessageOrReturn() {
+        if (!showPreviousMessage()) {
+            returnToMessageList()
         }
+    }
 
-        return if (couldMoveInLastDirection) {
-            true
-        } else {
-            showNextMessage() || showPreviousMessage()
+    private fun showNextMessageOrReturn() {
+        if (!showNextMessage()) {
+            returnToMessageList()
         }
     }
 

app/ui/legacy/src/main/java/com/fsck/k9/ui/messageview/MessageViewContainerFragment.kt

@@ -36,15 +36,10 @@ class MessageViewContainerFragment : Fragment() {
 
     private var activeMessageReference: MessageReference? = null
 
-    var lastDirection: Direction? = null
-        private set
-
     private lateinit var fragmentListener: MessageViewContainerListener
     private lateinit var viewPager: ViewPager2
     private lateinit var adapter: MessageViewContainerAdapter
 
-    private var currentPosition: Int? = null
-
     private val messageViewFragment: MessageViewFragment
         get() {
             check(isResumed)
@@ -64,14 +59,12 @@ class MessageViewContainerFragment : Fragment() {
 
         setHasOptionsMenu(true)
 
-        if (savedInstanceState == null) {
-            messageReference = MessageReference.parse(arguments?.getString(ARG_REFERENCE))
+        messageReference = if (savedInstanceState == null) {
+            MessageReference.parse(arguments?.getString(ARG_REFERENCE))
                 ?: error("Missing argument $ARG_REFERENCE")
         } else {
-            messageReference = MessageReference.parse(savedInstanceState.getString(STATE_MESSAGE_REFERENCE))
+            MessageReference.parse(savedInstanceState.getString(STATE_MESSAGE_REFERENCE))
                 ?: error("Missing state $STATE_MESSAGE_REFERENCE")
-
-            lastDirection = savedInstanceState.getSerializable(STATE_LAST_DIRECTION) as Direction?
         }
 
         showAccountChip = arguments?.getBoolean(ARG_SHOW_ACCOUNT_CHIP) ?: showAccountChip
@@ -123,7 +116,6 @@ class MessageViewContainerFragment : Fragment() {
     override fun onSaveInstanceState(outState: Bundle) {
         super.onSaveInstanceState(outState)
         outState.putString(STATE_MESSAGE_REFERENCE, messageReference.toIdentityString())
-        outState.putSerializable(STATE_LAST_DIRECTION, lastDirection)
     }
 
     fun setViewModel(viewModel: MessageListViewModel) {
@@ -161,25 +153,15 @@ class MessageViewContainerFragment : Fragment() {
         val newMessageReference = adapter.getMessageReference(position) ?: return
         if (newMessageReference == activeMessageReference) {
             // If the position of current message changes (e.g. because messages were added or removed from the list),
-            // we keep track of the new position but otherwise ignore the event.
-            currentPosition = position
+            // we ignore the event.
             return
         }
 
-        rememberNavigationDirection(position)
-
         messageReference = newMessageReference
         activeMessageReference = newMessageReference
         fragmentListener.setActiveMessage(newMessageReference)
     }
 
-    private fun rememberNavigationDirection(newPosition: Int) {
-        currentPosition?.let { currentPosition ->
-            lastDirection = if (newPosition < currentPosition) Direction.PREVIOUS else Direction.NEXT
-        }
-        currentPosition = newPosition
-    }
-
     fun showPreviousMessage(): Boolean {
         val newPosition = viewPager.currentItem - 1
         return if (newPosition >= 0) {
@@ -318,7 +300,6 @@ class MessageViewContainerFragment : Fragment() {
         private const val ARG_SHOW_ACCOUNT_CHIP = "showAccountChip"
 
         private const val STATE_MESSAGE_REFERENCE = "messageReference"
-        private const val STATE_LAST_DIRECTION = "lastDirection"
 
         fun newInstance(reference: MessageReference, showAccountChip: Boolean): MessageViewContainerFragment {
             return MessageViewContainerFragment().withArguments(

app/ui/legacy/src/main/java/com/fsck/k9/ui/messageview/MessageViewFragment.kt

@@ -425,7 +425,7 @@ class MessageViewFragment :
     private fun delete() {
         disableDeleteMenuItem()
 
-        fragmentListener.showNextMessageOrReturn()
+        fragmentListener.performNavigationAfterMessageRemoval()
 
         messagingController.deleteMessage(messageReference)
     }
@@ -454,7 +454,7 @@ class MessageViewFragment :
     }
 
     private fun refileMessage(destinationFolderId: Long) {
-        fragmentListener.showNextMessageOrReturn()
+        fragmentListener.performNavigationAfterMessageRemoval()
 
         val sourceFolderId = messageReference.folderId
         messagingController.moveMessage(account, sourceFolderId, messageReference, destinationFolderId)
@@ -527,7 +527,7 @@ class MessageViewFragment :
     }
 
     private fun onMoveToDrafts() {
-        fragmentListener.showNextMessageOrReturn()
+        fragmentListener.performNavigationAfterMessageRemoval()
 
         val account = account
         val folderId = messageReference.folderId
@@ -613,7 +613,7 @@ class MessageViewFragment :
 
         account.setLastSelectedFolderId(destinationFolderId)
 
-        fragmentListener.showNextMessageOrReturn()
+        fragmentListener.performNavigationAfterMessageRemoval()
 
         moveMessage(messageReference, destinationFolderId)
     }
@@ -863,7 +863,7 @@ class MessageViewFragment :
         fun onReplyAll(messageReference: MessageReference, decryptionResultForReply: Parcelable?)
         fun onReply(messageReference: MessageReference, decryptionResultForReply: Parcelable?)
         fun setProgress(enable: Boolean)
-        fun showNextMessageOrReturn()
+        fun performNavigationAfterMessageRemoval()
     }
 
     private val messageLoaderCallbacks: MessageLoaderCallbacks = object : MessageLoaderCallbacks {