Repository: thundernest/k-9
Issue: Do not save unmodified new message as draft

### Expected behavior

After having pressing the compose new message button, the compose view opens. If you close/background the app at this point, without changing/inputting anything at all, K-9 should not have saved the message as a draft.

### Actual behavior
The empty new message is saved as a draft.

### Steps to reproduce
1. From the accounts view, press the compose new message button near the bottom centre of the screen (envelope icon with a little plus sign).
2. Background the app and wait for Android to terminate it, or close it from app selection screen.
3. Check drafts folder. An empty draft has been stored (i.e., no subject, no text, no recipients).

### Environment
K-9 Mail version: 5.600

Android version: 10

Account type (IMAP, POP3, WebDAV/Exchange): IMAP

I do not believe attaching error logs is necessary in this case since this is not an application error per se, more of a UI issue.

The reason why the current behaviour is an annoyance is that the compose new button is placed so that it is easy to press it when what you want to do is to change to another app. This is because the Android 10 gesture for switching between apps is to swipe up from the bottom of the screen, and the compose new button is placed at the centre near the bottom. Thus I seem to end up accidentally entering the new message view quite frequently. If I then re-try the swipe gesture successfully immediately after, i.e., when the new message view has opened, I end up with an empty draft being saved. Because of this my drafts folder is accumulating a number of these empty drafts over time.

To me it does not seem to be any value in saving empty/unmodified drafts, so the solution is simple: just don't.

Changed files:
app/ui/src/test/java/com/fsck/k9/activity/compose/RecipientPresenterTest.java, app/ui/src/main/java/com/fsck/k9/activity/compose/RecipientPresenter.java, app/ui/src/main/java/com/fsck/k9/activity/MessageCompose.java, app/ui/src/main/java/com/fsck/k9/activity/compose/RecipientMvpView.java

diff --git a/app/ui/src/main/java/com/fsck/k9/activity/MessageCompose.java b/app/ui/src/main/java/com/fsck/k9/activity/MessageCompose.java
index ef8be534c..d078a3a5c 100644
--- a/app/ui/src/main/java/com/fsck/k9/activity/MessageCompose.java
+++ b/app/ui/src/main/java/com/fsck/k9/activity/MessageCompose.java
@@ -120,8 +120,7 @@ import timber.log.Timber;
 public class MessageCompose extends K9Activity implements OnClickListener,
         CancelListener, AttachmentDownloadCancelListener, OnFocusChangeListener,
         OnOpenPgpInlineChangeListener, OnOpenPgpSignOnlyChangeListener, MessageBuilder.Callback,
-        AttachmentPresenter.AttachmentsChangedListener, RecipientPresenter.RecipientsChangedListener,
-        OnOpenPgpDisableListener {
+        AttachmentPresenter.AttachmentsChangedListener, OnOpenPgpDisableListener {
 
     private static final int DIALOG_SAVE_OR_DISCARD_DRAFT_MESSAGE = 1;
     private static final int DIALOG_CONFIRM_DISCARD_ON_BACK = 2;
@@ -309,7 +308,7 @@ public class MessageCompose extends K9Activity implements OnClickListener,
         OpenPgpApiManager openPgpApiManager = new OpenPgpApiManager(getApplicationContext(), this);
         recipientPresenter = new RecipientPresenter(getApplicationContext(), getSupportLoaderManager(),
                 openPgpApiManager, recipientMvpView, account, composePgpInlineDecider, composePgpEnableByDefaultDecider,
-                AutocryptStatusInteractor.getInstance(), new ReplyToParser(), this,
+                AutocryptStatusInteractor.getInstance(), new ReplyToParser(),
                 DI.get(AutocryptDraftStateHeaderParser.class));
         recipientPresenter.asyncUpdateCryptoStatus();
 
@@ -950,11 +949,6 @@ public class MessageCompose extends K9Activity implements OnClickListener,
         changesMadeSinceLastSave = true;
     }
 
-    @Override
-    public void onRecipientsChanged() {
-        changesMadeSinceLastSave = true;
-    }
-
     @Override
     public void onClick(View view) {
         if (view.getId() == R.id.identity) {
diff --git a/app/ui/src/main/java/com/fsck/k9/activity/compose/RecipientMvpView.java b/app/ui/src/main/java/com/fsck/k9/activity/compose/RecipientMvpView.java
index 644a5fc02..13be2049f 100644
--- a/app/ui/src/main/java/com/fsck/k9/activity/compose/RecipientMvpView.java
+++ b/app/ui/src/main/java/com/fsck/k9/activity/compose/RecipientMvpView.java
@@ -3,7 +3,9 @@ package com.fsck.k9.activity.compose;
 
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Set;
 
 import android.app.PendingIntent;
 import androidx.loader.app.LoaderManager;
@@ -13,6 +15,7 @@ import android.text.TextWatcher;
 import android.view.View;
 import android.view.View.OnClickListener;
 import android.view.View.OnFocusChangeListener;
+import android.widget.TextView;
 import android.widget.Toast;
 import android.widget.ViewAnimator;
 
@@ -49,6 +52,7 @@ public class RecipientMvpView implements OnFocusChangeListener, OnClickListener
     private final ToolableViewAnimator cryptoStatusView;
     private final ViewAnimator recipientExpanderContainer;
     private final ToolableViewAnimator cryptoSpecialModeIndicator;
+    private final Set<TextWatcher> textWatchers = new HashSet<>();
     private RecipientPresenter presenter;
 
 
@@ -146,11 +150,25 @@ public class RecipientMvpView implements OnFocusChangeListener, OnClickListener
     }
 
     public void addTextChangedListener(TextWatcher textWatcher) {
+        textWatchers.add(textWatcher);
+
         toView.addTextChangedListener(textWatcher);
         ccView.addTextChangedListener(textWatcher);
         bccView.addTextChangedListener(textWatcher);
     }
 
+    private void removeAllTextChangedListeners(TextView view) {
+        for (TextWatcher textWatcher : textWatchers) {
+            view.removeTextChangedListener(textWatcher);
+        }
+    }
+
+    private void addAllTextChangedListeners(TextView view) {
+        for (TextWatcher textWatcher : textWatchers) {
+            view.addTextChangedListener(textWatcher);
+        }
+    }
+
     public void setRecipientTokensShowCryptoEnabled(boolean isEnabled) {
         toView.setShowCryptoEnabled(isEnabled);
         ccView.setShowCryptoEnabled(isEnabled);
@@ -199,18 +217,34 @@ public class RecipientMvpView implements OnFocusChangeListener, OnClickListener
         }
     }
 
-    public void removeBccAddresses(Address[] addressesToRemove) {
+    public void silentlyAddBccAddresses(Recipient... recipients) {
+        removeAllTextChangedListeners(bccView);
+
+        // The actual modification of the view is deferred via View.post()…
+        bccView.addRecipients(recipients);
+
+        // … so we do the same to add back the listeners.
+        bccView.post(() -> addAllTextChangedListeners(bccView));
+    }
+
+    public void silentlyRemoveBccAddresses(Address[] addressesToRemove) {
         if (addressesToRemove.length == 0) {
             return;
         }
 
         List<Recipient> bccRecipients = new ArrayList<>(getBccRecipients());
         for (Recipient recipient : bccRecipients) {
+            removeAllTextChangedListeners(bccView);
+
             for (Address address : addressesToRemove) {
                 if (recipient.address.equals(address)) {
+                    // The actual modification of the view is deferred via View.post()…
                     bccView.removeObject(recipient);
                 }
             }
+
+            // … so we do the same to add back the listeners.
+            bccView.post(() -> addAllTextChangedListeners(bccView));
         }
     }
 
diff --git a/app/ui/src/main/java/com/fsck/k9/activity/compose/RecipientPresenter.java b/app/ui/src/main/java/com/fsck/k9/activity/compose/RecipientPresenter.java
index 4b70877f6..2df86333f 100644
--- a/app/ui/src/main/java/com/fsck/k9/activity/compose/RecipientPresenter.java
+++ b/app/ui/src/main/java/com/fsck/k9/activity/compose/RecipientPresenter.java
@@ -73,7 +73,6 @@ public class RecipientPresenter {
     private final ComposePgpEnableByDefaultDecider composePgpEnableByDefaultDecider;
     private final ComposePgpInlineDecider composePgpInlineDecider;
     private final AutocryptStatusInteractor autocryptStatusInteractor;
-    private final RecipientsChangedListener listener;
     private final OpenPgpApiManager openPgpApiManager;
     private final AutocryptDraftStateHeaderParser draftStateHeaderParser;
     private ReplyToParser replyToParser;
@@ -96,15 +95,13 @@ public class RecipientPresenter {
             ComposePgpInlineDecider composePgpInlineDecider,
             ComposePgpEnableByDefaultDecider composePgpEnableByDefaultDecider,
             AutocryptStatusInteractor autocryptStatusInteractor,
-            ReplyToParser replyToParser, RecipientsChangedListener recipientsChangedListener,
-            AutocryptDraftStateHeaderParser draftStateHeaderParser) {
+            ReplyToParser replyToParser, AutocryptDraftStateHeaderParser draftStateHeaderParser) {
         this.recipientMvpView = recipientMvpView;
         this.context = context;
         this.autocryptStatusInteractor = autocryptStatusInteractor;
         this.composePgpInlineDecider = composePgpInlineDecider;
         this.composePgpEnableByDefaultDecider = composePgpEnableByDefaultDecider;
         this.replyToParser = replyToParser;
-        this.listener = recipientsChangedListener;
         this.openPgpApiManager = openPgpApiManager;
         this.draftStateHeaderParser = draftStateHeaderParser;
 
@@ -295,12 +292,22 @@ public class RecipientPresenter {
 
     public void addAlwaysBcc() {
         alwaysBccAddresses = Address.parse(account.getAlwaysBcc());
-        addRecipientsFromAddresses(RecipientType.BCC, alwaysBccAddresses);
+
+        new RecipientLoader(context, account.getOpenPgpProvider(), alwaysBccAddresses) {
+            @Override
+            public void deliverResult(List<Recipient> result) {
+                Recipient[] recipientArray = result.toArray(new Recipient[result.size()]);
+                recipientMvpView.silentlyAddBccAddresses(recipientArray);
+
+                stopLoading();
+                abandon();
+            }
+        }.startLoading();
     }
 
     private void removeAlwaysBcc() {
         if (alwaysBccAddresses != null) {
-            recipientMvpView.removeBccAddresses(alwaysBccAddresses);
+            recipientMvpView.silentlyRemoveBccAddresses(alwaysBccAddresses);
         }
     }
 
@@ -495,47 +502,38 @@ public class RecipientPresenter {
 
     void onToTokenAdded() {
         asyncUpdateCryptoStatus();
-        listener.onRecipientsChanged();
     }
 
     void onToTokenRemoved() {
         asyncUpdateCryptoStatus();
-        listener.onRecipientsChanged();
     }
 
     void onToTokenChanged() {
         asyncUpdateCryptoStatus();
-        listener.onRecipientsChanged();
     }
 
     void onCcTokenAdded() {
         asyncUpdateCryptoStatus();
-        listener.onRecipientsChanged();
     }
 
     void onCcTokenRemoved() {
         asyncUpdateCryptoStatus();
-        listener.onRecipientsChanged();
     }
 
     void onCcTokenChanged() {
         asyncUpdateCryptoStatus();
-        listener.onRecipientsChanged();
     }
 
     void onBccTokenAdded() {
         asyncUpdateCryptoStatus();
-        listener.onRecipientsChanged();
     }
 
     void onBccTokenRemoved() {
         asyncUpdateCryptoStatus();
-        listener.onRecipientsChanged();
     }
 
     void onBccTokenChanged() {
         asyncUpdateCryptoStatus();
-        listener.onRecipientsChanged();
     }
 
     public void onCryptoModeChanged(CryptoMode cryptoMode) {
@@ -847,10 +845,6 @@ public class RecipientPresenter {
         return cachedCryptoStatus == null || !cachedCryptoStatus.isEncryptionEnabled();
     }
 
-    public interface RecipientsChangedListener {
-        void onRecipientsChanged();
-    }
-
     private final OpenPgpApiManagerCallback openPgpCallback = new OpenPgpApiManagerCallback() {
         @Override
         public void onOpenPgpProviderStatusChanged() {
diff --git a/app/ui/src/test/java/com/fsck/k9/activity/compose/RecipientPresenterTest.java b/app/ui/src/test/java/com/fsck/k9/activity/compose/RecipientPresenterTest.java
index 285c6aa6a..8e1fd8515 100644
--- a/app/ui/src/test/java/com/fsck/k9/activity/compose/RecipientPresenterTest.java
+++ b/app/ui/src/test/java/com/fsck/k9/activity/compose/RecipientPresenterTest.java
@@ -66,7 +66,6 @@ public class RecipientPresenterTest extends K9RobolectricTest {
     private ComposePgpEnableByDefaultDecider composePgpEnableByDefaultDecider;
     private Account account;
     private RecipientMvpView recipientMvpView;
-    private RecipientPresenter.RecipientsChangedListener listener;
     private AutocryptStatusInteractor autocryptStatusInteractor;
     private RecipientAutocryptStatus noRecipientsAutocryptResult;
     private OpenPgpApiManager openPgpApiManager;
@@ -86,13 +85,12 @@ public class RecipientPresenterTest extends K9RobolectricTest {
         autocryptStatusInteractor = mock(AutocryptStatusInteractor.class);
         replyToParser = mock(ReplyToParser.class);
         LoaderManager loaderManager = mock(LoaderManager.class);
-        listener = mock(RecipientPresenter.RecipientsChangedListener.class);
 
         when(openPgpApiManager.getOpenPgpProviderState()).thenReturn(OpenPgpProviderState.UNCONFIGURED);
 
         recipientPresenter = new RecipientPresenter(
                 context, loaderManager, openPgpApiManager, recipientMvpView, account, composePgpInlineDecider,
-                composePgpEnableByDefaultDecider, autocryptStatusInteractor, replyToParser, listener,
+                composePgpEnableByDefaultDecider, autocryptStatusInteractor, replyToParser,
                 DI.get(AutocryptDraftStateHeaderParser.class)
         );
 
@@ -279,60 +277,6 @@ public class RecipientPresenterTest extends K9RobolectricTest {
         assertTrue(status.isPgpInlineModeEnabled());
     }
 
-    @Test
-    public void onToTokenAdded_notifiesListenerOfRecipientChange() {
-        recipientPresenter.onToTokenAdded();
-        verify(listener).onRecipientsChanged();
-    }
-
-    @Test
-    public void onToTokenChanged_notifiesListenerOfRecipientChange() {
-        recipientPresenter.onToTokenChanged();
-        verify(listener).onRecipientsChanged();
-    }
-
-    @Test
-    public void onToTokenRemoved_notifiesListenerOfRecipientChange() {
-        recipientPresenter.onToTokenRemoved();
-        verify(listener).onRecipientsChanged();
-    }
-
-    @Test
-    public void onCcTokenAdded_notifiesListenerOfRecipientChange() {
-        recipientPresenter.onCcTokenAdded();
-        verify(listener).onRecipientsChanged();
-    }
-
-    @Test
-    public void onCcTokenChanged_notifiesListenerOfRecipientChange() {
-        recipientPresenter.onCcTokenChanged();
-        verify(listener).onRecipientsChanged();
-    }
-
-    @Test
-    public void onCcTokenRemoved_notifiesListenerOfRecipientChange() {
-        recipientPresenter.onCcTokenRemoved();
-        verify(listener).onRecipientsChanged();
-    }
-
-    @Test
-    public void onBccTokenAdded_notifiesListenerOfRecipientChange() {
-        recipientPresenter.onBccTokenAdded();
-        verify(listener).onRecipientsChanged();
-    }
-
-    @Test
-    public void onBccTokenChanged_notifiesListenerOfRecipientChange() {
-        recipientPresenter.onBccTokenChanged();
-        verify(listener).onRecipientsChanged();
-    }
-
-    @Test
-    public void onBccTokenRemoved_notifiesListenerOfRecipientChange() {
-        recipientPresenter.onBccTokenRemoved();
-        verify(listener).onRecipientsChanged();
-    }
-
     private void runBackgroundTask() {
         boolean taskRun = Robolectric.getBackgroundThreadScheduler().runOneTask();
         assertTrue(taskRun);