From fc36dfe00cbb7db6e00c8fc774012884a133294e Mon Sep 17 00:00:00 2001 From: dnicules <124223787+dnicules@users.noreply.github.com> Date: Sun, 8 Dec 2024 20:55:30 -0500 Subject: [PATCH 1/5] Fix Uncaught Exception, Crash Fixed uncaught exception which, under a certain edge case (Issue #7688) starring or unstarring a message explicitly in Message View after moving it offline may cause the app to crash. - Catch exception in queueSetFlag created in call to pendingSetFlag.create() - Mimics behavior of starring/unstarring a message in List View --- .../fsck/k9/controller/MessagingController.java | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/legacy/core/src/main/java/com/fsck/k9/controller/MessagingController.java b/legacy/core/src/main/java/com/fsck/k9/controller/MessagingController.java index 6d5b7104638..0520ecebdfc 100644 --- a/legacy/core/src/main/java/com/fsck/k9/controller/MessagingController.java +++ b/legacy/core/src/main/java/com/fsck/k9/controller/MessagingController.java @@ -974,8 +974,18 @@ void destroyPlaceholderMessages(LocalFolder localFolder, List uids) thro } private void queueSetFlag(Account account, long folderId, boolean newState, Flag flag, List uids) { - PendingCommand command = PendingSetFlag.create(folderId, newState, flag, uids); - queuePendingCommand(account, command); + //PendingCommand command = PendingSetFlag.create(folderId, newState, flag, uids); + String commandDescription = null; + try{ + PendingCommand command = PendingSetFlag.create(folderId, newState, flag, uids); + if (command != null) { + commandDescription = command.getCommandName(); + } + queuePendingCommand(account, command); + } catch (Exception e){ + + Timber.e(e, "Error running command '%s'", commandDescription); + } } /** From 4209134a8648b2591ad0752b69fe8242076ad19a Mon Sep 17 00:00:00 2001 From: dnicules <124223787+dnicules@users.noreply.github.com> Date: Sun, 8 Dec 2024 21:09:05 -0500 Subject: [PATCH 2/5] Add verification for moving/copying messages Added verification for moving messages into other folders (in response to Issue #823) - Currently, users can move messages into local-only folders which don't exist server-side, may end up with messages stuck in these local folders and effectively lost - When online, refresh the folder list prior to moving; prevent messages ending up in "dead" unsynced folders which do not exist server-side - When offline, provide the user with a warning saying that "ThunderBird is not connected to the Internet. Any changes you make now may not be synced properly." - Users may proceed anyways, cancel the move/copy/draft/archive/spam operation, or "don't show again" and always proceed anyways --- .../k9/ui/messagelist/MessageListFragment.kt | 197 ++++++++++++++++-- 1 file changed, 183 insertions(+), 14 deletions(-) diff --git a/legacy/ui/legacy/src/main/java/com/fsck/k9/ui/messagelist/MessageListFragment.kt b/legacy/ui/legacy/src/main/java/com/fsck/k9/ui/messagelist/MessageListFragment.kt index 5f70f03b902..09f4939b7fc 100644 --- a/legacy/ui/legacy/src/main/java/com/fsck/k9/ui/messagelist/MessageListFragment.kt +++ b/legacy/ui/legacy/src/main/java/com/fsck/k9/ui/messagelist/MessageListFragment.kt @@ -13,6 +13,7 @@ import android.view.View import android.view.ViewGroup import android.widget.Toast import androidx.annotation.StringRes +import androidx.appcompat.app.AlertDialog import androidx.appcompat.view.ActionMode import androidx.core.os.BundleCompat import androidx.core.os.bundleOf @@ -46,6 +47,7 @@ import com.fsck.k9.helper.Utility import com.fsck.k9.helper.mapToSet import com.fsck.k9.mail.Flag import com.fsck.k9.mail.MessagingException +import com.fsck.k9.network.ConnectivityManager import com.fsck.k9.search.getAccounts import com.fsck.k9.ui.R import com.fsck.k9.ui.changelog.RecentChangesActivity @@ -82,6 +84,7 @@ class MessageListFragment : private val messagingController: MessagingController by inject() private val accountManager: AccountManager by inject() private val clock: Clock by inject() + private val connectivityManager: ConnectivityManager by inject() private val handler = MessageListHandler(this) private val activityListener = MessageListActivityListener() @@ -1035,11 +1038,47 @@ class MessageListFragment : computeBatchDirection() } - private fun onMove(message: MessageReference) { - onMove(listOf(message)) + private interface ConnectivityDialogCallback { + fun onDialogResult(result: Int) } - private fun onMove(messages: List) { + private fun showConnectivityDialog(callback: ConnectivityDialogCallback) { // added + val sharedPreferences = context?.getSharedPreferences("app_preferences", Context.MODE_PRIVATE); + val dontShowAgain = sharedPreferences?.getBoolean("dont_show_connectivity_warning", false) ?: false; + //sharedPreferences?.edit()?.putBoolean("dont_show_connectivity_warning", false)?.apply() + + //callback.onDialogResult(0); + if (dontShowAgain) { + // Proceed directly if "Don't show again" is checked + callback.onDialogResult(1); + return; + } + //callback.onDialogResult(0); + val builder = AlertDialog.Builder(requireContext()) + builder.setTitle("No Internet Connection") + .setMessage("ThunderBird is not connected to the Internet. Any changes you make now may not be synced properly.") + .setPositiveButton("Proceed") { _, _ -> + callback.onDialogResult(1); + + //Log.d("onMove", "User chose to proceed") + } + .setNegativeButton("Cancel") { _, _ -> + callback.onDialogResult(-1); + + //Log.d("onMove", "User chose to cancel") + } + .setNeutralButton("Don't show again") { _, _ -> + // Save the preference to not show the dialog again + callback.onDialogResult(1); + sharedPreferences?.edit()?.putBoolean("dont_show_connectivity_warning", true)?.apply() + //Log.d("onMove", "User chose 'Don't show again'") + } + .show() + + //return; + } + + private fun proceedWithMove(messages: List) { if (!checkCopyOrMovePossible(messages, FolderOperation.MOVE)) return val folderId = when { @@ -1058,11 +1097,32 @@ class MessageListFragment : ) } - private fun onCopy(message: MessageReference) { - onCopy(listOf(message)) + private fun onMove(message: MessageReference) { + onMove(listOf(message)) } - private fun onCopy(messages: List) { + private fun onMove(messages: List) { + + if (!preMove()) { + showConnectivityDialog( + object : ConnectivityDialogCallback { + override fun onDialogResult(result: Int) { + if (result == -1) { + return + } else if (result == 1) { + proceedWithMove(messages); + return; + } + } + }, + ) + } else { + proceedWithMove(messages); + return; + } + } + + private fun proceedWithCopy(messages: List) { if (!checkCopyOrMovePossible(messages, FolderOperation.COPY)) return val folderId = when { @@ -1081,6 +1141,30 @@ class MessageListFragment : ) } + private fun onCopy(message: MessageReference) { + onCopy(listOf(message)) + } + + private fun onCopy(messages: List) { + if (!preMove()) { + showConnectivityDialog( + object : ConnectivityDialogCallback { + override fun onDialogResult(result: Int) { + if (result == -1) { + return + } else if (result == 1) { + proceedWithCopy(messages); + return; + } + } + }, + ) + } else { + proceedWithCopy(messages); + return; + } + } + private fun displayFolderChoice( operation: FolderOperation, requestCode: Int, @@ -1172,11 +1256,43 @@ class MessageListFragment : } private fun copy(messages: List, folderId: Long) { - copyOrMove(messages, folderId, FolderOperation.COPY) + if (!preMove()) { + showConnectivityDialog( + object : ConnectivityDialogCallback { + override fun onDialogResult(result: Int) { + if (result == -1) { + return + } else if (result == 1) { + copyOrMove(messages, folderId, FolderOperation.COPY) + return; + } + } + }, + ) + } else { + copyOrMove(messages, folderId, FolderOperation.COPY); + return; + } } private fun move(messages: List, folderId: Long) { - copyOrMove(messages, folderId, FolderOperation.MOVE) + if (!preMove()) { + showConnectivityDialog( + object : ConnectivityDialogCallback { + override fun onDialogResult(result: Int) { + if (result == -1) { + return + } else if (result == 1) { + copyOrMove(messages, folderId, FolderOperation.MOVE) + return; + } + } + }, + ) + } else { + copyOrMove(messages, folderId, FolderOperation.MOVE); + return; + } } private fun copyOrMove(messages: List, destinationFolderId: Long, operation: FolderOperation) { @@ -1206,8 +1322,25 @@ class MessageListFragment : } private fun onMoveToDraftsFolder(messages: List) { - messagingController.moveToDraftsFolder(account, currentFolder!!.databaseId, messages) - activeMessages = null + if (!preMove()) { + showConnectivityDialog( + object : ConnectivityDialogCallback { + override fun onDialogResult(result: Int) { + if (result == -1) { + return + } else if (result == 1) { + messagingController.moveToDraftsFolder(account, currentFolder!!.databaseId, messages) + activeMessages = null + return; + } + } + }, + ) + } else { + messagingController.moveToDraftsFolder(account, currentFolder!!.databaseId, messages) + activeMessages = null + return; + } } override fun doPositiveClick(dialogId: Int) { @@ -1354,8 +1487,26 @@ class MessageListFragment : } fun onMove() { - selectedMessage?.let { message -> - onMove(message) + if (!preMove()) { + showConnectivityDialog( + object : ConnectivityDialogCallback { + override fun onDialogResult(result: Int) { + if (result == -1) { + return + } else if (result == 1) { + selectedMessage?.let { message -> + onMove(message) + } + return; + } + } + }, + ) + } else { + selectedMessage?.let { message -> + onMove(message) + } + return; } } @@ -1366,8 +1517,26 @@ class MessageListFragment : } fun onCopy() { - selectedMessage?.let { message -> - onCopy(message) + if (!preMove()) { + showConnectivityDialog( + object : ConnectivityDialogCallback { + override fun onDialogResult(result: Int) { + if (result == -1) { + return + } else if (result == 1) { + selectedMessage?.let { message -> + onCopy(message) + } + return; + } + } + }, + ) + } else { + selectedMessage?.let { message -> + onCopy(message) + } + return; } } From 1d19196153b84d86cef104f368f3c2993c1b480b Mon Sep 17 00:00:00 2001 From: dnicules <124223787+dnicules@users.noreply.github.com> Date: Sun, 8 Dec 2024 21:16:20 -0500 Subject: [PATCH 3/5] Add verification for moving/copying messages Added verification for moving messages into other folders (in response to Issue #823) - Currently, users can move messages into local-only folders which don't exist server-side, may end up with messages stuck in these local folders and effectively lost - When online, refresh the folder list prior to moving; prevent messages ending up in "dead" unsynced folders which do not exist server-side - When offline, provide the user with a warning saying that "ThunderBird is not connected to the Internet. Any changes you make now may not be synced properly." - Users may proceed anyways, cancel the move/copy/draft/archive/spam operation, or "don't show again" and always proceed anyways --- .../k9/ui/messageview/MessageViewFragment.kt | 133 +++++++++++++++++- 1 file changed, 129 insertions(+), 4 deletions(-) diff --git a/legacy/ui/legacy/src/main/java/com/fsck/k9/ui/messageview/MessageViewFragment.kt b/legacy/ui/legacy/src/main/java/com/fsck/k9/ui/messageview/MessageViewFragment.kt index 7d403504922..9423a13563d 100644 --- a/legacy/ui/legacy/src/main/java/com/fsck/k9/ui/messageview/MessageViewFragment.kt +++ b/legacy/ui/legacy/src/main/java/com/fsck/k9/ui/messageview/MessageViewFragment.kt @@ -17,6 +17,7 @@ import android.view.View import android.view.ViewGroup import android.view.inputmethod.InputMethodManager import android.widget.Toast +import androidx.appcompat.app.AlertDialog import androidx.core.app.ActivityCompat import androidx.core.content.ContextCompat import androidx.fragment.app.DialogFragment @@ -45,6 +46,7 @@ import com.fsck.k9.mail.Flag import com.fsck.k9.mailstore.AttachmentViewInfo import com.fsck.k9.mailstore.LocalMessage import com.fsck.k9.mailstore.MessageViewInfo +import com.fsck.k9.network.ConnectivityManager import com.fsck.k9.ui.R import com.fsck.k9.ui.base.extensions.withArguments import com.fsck.k9.ui.choosefolder.ChooseFolderActivity @@ -70,6 +72,8 @@ class MessageViewFragment : private val shareIntentBuilder: ShareIntentBuilder by inject() private val generalSettingsManager: GeneralSettingsManager by inject() + private val connectivityManager: ConnectivityManager by inject() + private lateinit var messageTopView: MessageTopView private var message: LocalMessage? = null @@ -435,7 +439,47 @@ class MessageViewFragment : invalidateMenu() } - private fun onRefile(destinationFolderId: Long?) { + private interface ConnectivityDialogCallback { + fun onDialogResult(result: Int) + } + + private fun showConnectivityDialog(callback: ConnectivityDialogCallback) { // added + val sharedPreferences = context?.getSharedPreferences("app_preferences", Context.MODE_PRIVATE); + val dontShowAgain = sharedPreferences?.getBoolean("dont_show_connectivity_warning", false) ?: false; + //sharedPreferences?.edit()?.putBoolean("dont_show_connectivity_warning", false)?.apply() + + //callback.onDialogResult(0); + if (dontShowAgain) { + // Proceed directly if "Don't show again" is checked + callback.onDialogResult(1); + return; + } + //callback.onDialogResult(0); + val builder = AlertDialog.Builder(requireContext()) + builder.setTitle("No Internet Connection") + .setMessage("ThunderBird is not connected to the Internet. Any changes you make now may not be synced properly.") + .setPositiveButton("Proceed") { _, _ -> + callback.onDialogResult(1); + + //Log.d("onMove", "User chose to proceed") + } + .setNegativeButton("Cancel") { _, _ -> + callback.onDialogResult(-1); + + //Log.d("onMove", "User chose to cancel") + } + .setNeutralButton("Don't show again") { _, _ -> + // Save the preference to not show the dialog again + callback.onDialogResult(1); + sharedPreferences?.edit()?.putBoolean("dont_show_connectivity_warning", true)?.apply() + //Log.d("onMove", "User chose 'Don't show again'") + } + .show() + + //return; + } + + private fun proceedWithRefile(destinationFolderId: Long?) { if (destinationFolderId == null || !messagingController.isMoveCapable(account)) { return } @@ -453,6 +497,26 @@ class MessageViewFragment : } } + private fun onRefile(destinationFolderId: Long?) { + if (!preMove()) { + showConnectivityDialog( + object : ConnectivityDialogCallback { + override fun onDialogResult(result: Int) { + if (result == -1) { + return + } else if (result == 1) { + proceedWithRefile(destinationFolderId); + return; + } + } + }, + ) + } else { + proceedWithRefile(destinationFolderId); + return; + } + } + private fun refileMessage(destinationFolderId: Long) { fragmentListener.performNavigationAfterMessageRemoval() @@ -502,7 +566,7 @@ class MessageViewFragment : fragmentListener.onEditAsNewMessage(message.makeMessageReference()) } - fun onMove() { + private fun proceedWithMove() { check(messagingController.isMoveCapable(account)) checkNotNull(message) @@ -514,7 +578,27 @@ class MessageViewFragment : startRefileActivity(FolderOperation.MOVE, ACTIVITY_CHOOSE_FOLDER_MOVE) } - fun onCopy() { + fun onMove() { + if (!preMove()) { + showConnectivityDialog( + object : ConnectivityDialogCallback { + override fun onDialogResult(result: Int) { + if (result == -1) { + return + } else if (result == 1) { + proceedWithMove(); + return; + } + } + }, + ) + } else { + proceedWithMove(); + return; + } + } + + private fun proceedWithCopy() { check(messagingController.isCopyCapable(account)) checkNotNull(message) @@ -526,7 +610,27 @@ class MessageViewFragment : startRefileActivity(FolderOperation.COPY, ACTIVITY_CHOOSE_FOLDER_COPY) } - private fun onMoveToDrafts() { + fun onCopy() { + if (!preMove()) { + showConnectivityDialog( + object : ConnectivityDialogCallback { + override fun onDialogResult(result: Int) { + if (result == -1) { + return + } else if (result == 1) { + proceedWithCopy(); + return; + } + } + }, + ) + } else { + proceedWithCopy(); + return; + } + } + + private fun proceedWithDraft() { fragmentListener.performNavigationAfterMessageRemoval() val account = account @@ -535,6 +639,27 @@ class MessageViewFragment : messagingController.moveToDraftsFolder(account, folderId, messages) } + private fun onMoveToDrafts() { + + if (!preMove()) { + showConnectivityDialog( + object : ConnectivityDialogCallback { + override fun onDialogResult(result: Int) { + if (result == -1) { + return + } else if (result == 1) { + proceedWithDraft(); + return; + } + } + }, + ) + } else { + proceedWithDraft(); + return; + } + } + fun onArchive() { if (!account.hasArchiveFolder()) return From 6d2155c329dcacecb9dcd656beaafa1f7cb8fa34 Mon Sep 17 00:00:00 2001 From: dnicules <124223787+dnicules@users.noreply.github.com> Date: Mon, 9 Dec 2024 22:47:39 -0500 Subject: [PATCH 4/5] Updated from Google Doc --- issueTestCases.txt | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 issueTestCases.txt diff --git a/issueTestCases.txt b/issueTestCases.txt new file mode 100644 index 00000000000..20b12c1ae96 --- /dev/null +++ b/issueTestCases.txt @@ -0,0 +1,25 @@ +Task 1: +Working cases: starring/unstarring messages moved anywhere offline so long as it happens in the LIST VIEW +Failing cases: starring/unstarring messages moved anywhere offline, but the action is performed in MESSAGE VIEW +- Move to spam, then star/unstar +- Move to drafts, then star/unstar +- Move to outbox, then star/unstar +- Move to archive, then star/unstar +- Move to arbitrary folder (e.g. "Important"), then star/unstar + + +Task 2: +No working cases +Failing cases: any "ghost folders" existing in the app but NOT on the IMAP server will allow a user to move a message into the folder but not out of the folder (both online and offline) +Test cases: + +-- ONLINE -- +- Move/copy message into a ghost folder from List View +- Move/copy message into a ghost folder from Message View + - In both cases, message list should get refreshed before prompting user for target folder, and the app will not allow the user to complete the operation (folder no longer exists) + +-- OFFLINE -- +- Move/copy message into ghost folder from List View +- Move/copy message into ghost folder from Message View + - In each case, pop-up should appear prompting user to cancel the operation, proceed anyways, or to not show the pop-up again and to proceed + - If "don't show again" is selected, the pop-up should not appear if a user closes, reopens the app and repeats behavior (i.e. memory between sessions) From 46252790dc27b1b0501f6f1996fd6021be79f0af Mon Sep 17 00:00:00 2001 From: dnicules <124223787+dnicules@users.noreply.github.com> Date: Mon, 9 Dec 2024 22:49:46 -0500 Subject: [PATCH 5/5] Update issueTestCases.txt --- issueTestCases.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/issueTestCases.txt b/issueTestCases.txt index 20b12c1ae96..9c5e142f8b8 100644 --- a/issueTestCases.txt +++ b/issueTestCases.txt @@ -11,7 +11,7 @@ Failing cases: starring/unstarring messages moved anywhere offline, but the acti Task 2: No working cases Failing cases: any "ghost folders" existing in the app but NOT on the IMAP server will allow a user to move a message into the folder but not out of the folder (both online and offline) -Test cases: +Test cases (for move, copy, archive, spam, move to drafts, move to outbox, move to arbitrary folder): -- ONLINE -- - Move/copy message into a ghost folder from List View