Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initial focus in message compose screen #8664

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions legacy/ui/legacy/src/main/java/com/fsck/k9/UiKoinModules.kt
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import com.fsck.k9.ui.account.accountUiModule
import com.fsck.k9.ui.base.uiBaseModule
import com.fsck.k9.ui.changelog.changelogUiModule
import com.fsck.k9.ui.choosefolder.chooseFolderUiModule
import com.fsck.k9.ui.compose.composeModule
import com.fsck.k9.ui.endtoend.endToEndUiModule
import com.fsck.k9.ui.folders.foldersUiModule
import com.fsck.k9.ui.identity.identityUiModule
Expand All @@ -36,6 +37,7 @@ val uiModules = listOf(
chooseFolderUiModule,
contactsModule,
accountModule,
composeModule,
viewModule,
changelogUiModule,
messageSourceModule,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import java.io.File;
import java.util.Date;
import java.util.HashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.regex.Pattern;
Expand All @@ -19,7 +18,6 @@
import android.content.IntentSender.SendIntentException;
import android.content.pm.ActivityInfo;
import android.net.Uri;
import android.nfc.NfcAdapter;
import android.os.AsyncTask;
import android.os.Bundle;
import android.os.Handler;
Expand Down Expand Up @@ -110,6 +108,8 @@
import com.fsck.k9.ui.R;
import com.fsck.k9.ui.base.K9Activity;
import app.k9mail.legacy.ui.theme.ThemeManager;
import com.fsck.k9.ui.compose.IntentData;
import com.fsck.k9.ui.compose.IntentDataMapper;
import com.fsck.k9.ui.compose.QuotedMessageMvpView;
import com.fsck.k9.ui.compose.QuotedMessagePresenter;
import com.fsck.k9.ui.compose.WrapUriTextWatcher;
Expand All @@ -118,16 +118,15 @@
import com.google.android.material.dialog.MaterialAlertDialogBuilder;
import com.google.android.material.textview.MaterialTextView;
import org.openintents.openpgp.OpenPgpApiManager;
import org.openintents.openpgp.util.OpenPgpApi;
import org.openintents.openpgp.util.OpenPgpIntentStarter;
import timber.log.Timber;


@SuppressWarnings("deprecation") // TODO get rid of activity dialogs and indeterminate progress bars
public class MessageCompose extends K9Activity implements OnClickListener,
CancelListener, AttachmentDownloadCancelListener, OnFocusChangeListener,
OnOpenPgpInlineChangeListener, OnOpenPgpSignOnlyChangeListener, MessageBuilder.Callback,
AttachmentPresenter.AttachmentsChangedListener, OnOpenPgpDisableListener {
CancelListener, AttachmentDownloadCancelListener, OnFocusChangeListener,
OnOpenPgpInlineChangeListener, OnOpenPgpSignOnlyChangeListener, MessageBuilder.Callback,
AttachmentPresenter.AttachmentsChangedListener, OnOpenPgpDisableListener {

private static final int DIALOG_SAVE_OR_DISCARD_DRAFT_MESSAGE = 1;
private static final int DIALOG_CONFIRM_DISCARD_ON_BACK = 2;
Expand All @@ -140,7 +139,7 @@ public class MessageCompose extends K9Activity implements OnClickListener,
public static final String ACTION_FORWARD = "com.fsck.k9.intent.action.FORWARD";
public static final String ACTION_FORWARD_AS_ATTACHMENT = "com.fsck.k9.intent.action.FORWARD_AS_ATTACHMENT";
public static final String ACTION_EDIT_DRAFT = "com.fsck.k9.intent.action.EDIT_DRAFT";
private static final String ACTION_AUTOCRYPT_PEER = "org.autocrypt.PEER_ACTION";
public static final String ACTION_AUTOCRYPT_PEER = "org.autocrypt.PEER_ACTION";

public static final String EXTRA_ACCOUNT = "account";
public static final String EXTRA_MESSAGE_REFERENCE = "message_reference";
Expand Down Expand Up @@ -185,6 +184,8 @@ public class MessageCompose extends K9Activity implements OnClickListener,
private final MessagingController messagingController = DI.get(MessagingController.class);
private final Preferences preferences = DI.get(Preferences.class);

private final IntentDataMapper indentDataMapper = DI.get(IntentDataMapper.class);

private final Contacts contacts = DI.get(Contacts.class);

private QuotedMessagePresenter quotedMessagePresenter;
Expand Down Expand Up @@ -222,6 +223,7 @@ public class MessageCompose extends K9Activity implements OnClickListener,

private Action action;

private RecipientMvpView recipientMvpView;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of creating a class member that could leak, it's better to keep it as is and change the messageLoaderCallbacks to a private class and inject the recipientMvpView when creating it.

    private MessageLoaderCallbacks messageLoaderCallbacks = new MessageLoaderCallbacks() {
    private class MessageComposeMessageLoaderCallback implements MessageLoaderCallbacks {

        private final RecipientMvpView recipientMvpView;

        public MessageComposeMessageLoaderCallback(RecipientMvpView recipientMvpView) {
            this.recipientMvpView = recipientMvpView;
        }
    ...

This is possible as recipientMvpView is only needed in onCreate.

private boolean requestReadReceipt = false;

private MaterialTextView chooseIdentityView;
Expand Down Expand Up @@ -304,7 +306,7 @@ public void onCreate(Bundle savedInstanceState) {
ReplyToView replyToView = new ReplyToView(this);
replyToPresenter = new ReplyToPresenter(replyToView);

RecipientMvpView recipientMvpView = new RecipientMvpView(this);
recipientMvpView = new RecipientMvpView(this);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above regarding recipientMvpView

ComposePgpInlineDecider composePgpInlineDecider = new ComposePgpInlineDecider();
ComposePgpEnableByDefaultDecider composePgpEnableByDefaultDecider = new ComposePgpEnableByDefaultDecider();

Expand Down Expand Up @@ -374,14 +376,44 @@ public void onTextChanged(CharSequence s, int start, int before, int count) {
relatedMessageProcessed = savedInstanceState.getBoolean(STATE_KEY_SOURCE_MESSAGE_PROCED, false);
}

IntentData intentData = indentDataMapper.initFromIntent(intent);

if (intentData.getMailToUri() != null) {
Uri uri = intentData.getMailToUri();
if (MailTo.isMailTo(uri)) {
MailTo mailTo = MailTo.parse(uri);
initializeFromMailto(mailTo);
}
}

if (intentData.getExtraText() != null && messageContentView.getText().length() == 0) {
messageContentView.setText(CrLfConverter.toLf(intentData.getExtraText()));
}

if (intentData.getExtraStream() != null) {
attachmentPresenter.addExternalAttachment(intentData.getExtraStream(), intentData.getIntentType());
}

if (intentData.getSubject() != null && subjectView.getText().length() == 0) {
subjectView.setText(intentData.getSubject());
}

if (intentData.getShouldInitFromSendOrViewIntent()) {
recipientPresenter.initFromSendOrViewIntent(intent);
}

if (intentData.getTrustId() != null) {
recipientPresenter.initFromTrustIdAction(intentData.getTrustId());
}

if (initFromIntent(intent)) {
if (intentData.getStartedByExternalIntent()) {
action = Action.COMPOSE;
changesMadeSinceLastSave = true;
} else {
String action = intent.getAction();
if (ACTION_COMPOSE.equals(action)) {
this.action = Action.COMPOSE;
recipientMvpView.requestFocusOnToField();
} else if (ACTION_REPLY.equals(action)) {
this.action = Action.REPLY;
} else if (ACTION_REPLY_ALL.equals(action)) {
Expand Down Expand Up @@ -450,15 +482,6 @@ public void onTextChanged(CharSequence s, int start, int before, int count) {
relatedFlag = Flag.FORWARDED;
}

if (action == Action.REPLY || action == Action.REPLY_ALL ||
action == Action.EDIT_DRAFT) {
//change focus to message body.
messageContentView.requestFocus();
} else {
// Explicitly set focus to "To:" input field (see issue 2998)
recipientMvpView.requestFocusOnToField();
}

updateMessageFormat();

// Set font size of input controls
Expand All @@ -480,109 +503,7 @@ public void onTextChanged(CharSequence s, int start, int before, int count) {
setProgressBarIndeterminateVisibility(true);
currentMessageBuilder.reattachCallback(this);
}
}

/**
* Handle external intents that trigger the message compose activity.
*
* <p>
* Supported external intents:
* <ul>
* <li>{@link Intent#ACTION_VIEW}</li>
* <li>{@link Intent#ACTION_SENDTO}</li>
* <li>{@link Intent#ACTION_SEND}</li>
* <li>{@link Intent#ACTION_SEND_MULTIPLE}</li>
* </ul>
* </p>
*
* @param intent
* The (external) intent that started the activity.
*
* @return {@code true}, if this activity was started by an external intent. {@code false},
* otherwise.
*/
private boolean initFromIntent(final Intent intent) {
boolean startedByExternalIntent = false;
final String action = intent.getAction();

if (Intent.ACTION_VIEW.equals(action) || Intent.ACTION_SENDTO.equals(action) || NfcAdapter.ACTION_NDEF_DISCOVERED.equals(action)) {
/*
* Someone has clicked a mailto: link, or scanned an NFC tag. The address is in the URI.
*/
if (intent.getData() != null) {
Uri uri = intent.getData();
if (MailTo.isMailTo(uri)) {
MailTo mailTo = MailTo.parse(uri);
initializeFromMailto(mailTo);
}
}

/*
* Note: According to the documentation ACTION_VIEW and ACTION_SENDTO don't accept
* EXTRA_* parameters.
* And previously we didn't process these EXTRAs. But it looks like nobody bothers to
* read the official documentation and just copies wrong sample code that happens to
* work with the AOSP Email application. And because even big players get this wrong,
* we're now finally giving in and read the EXTRAs for those actions (below).
*/
}

if (Intent.ACTION_SEND.equals(action) || Intent.ACTION_SEND_MULTIPLE.equals(action) ||
Intent.ACTION_SENDTO.equals(action) || Intent.ACTION_VIEW.equals(action)) {
startedByExternalIntent = true;

/*
* Note: Here we allow a slight deviation from the documented behavior.
* EXTRA_TEXT is used as message body (if available) regardless of the MIME
* type of the intent. In addition one or multiple attachments can be added
* using EXTRA_STREAM.
*/
CharSequence text = intent.getCharSequenceExtra(Intent.EXTRA_TEXT);
// Only use EXTRA_TEXT if the body hasn't already been set by the mailto URI
if (text != null && messageContentView.getText().length() == 0) {
messageContentView.setText(CrLfConverter.toLf(text));
}

String type = intent.getType();
if (Intent.ACTION_SEND.equals(action)) {
Uri stream = IntentCompat.getParcelableExtra(intent,Intent.EXTRA_STREAM, Uri.class);
if (stream != null) {
attachmentPresenter.addExternalAttachment(stream, type);
}
} else {
List<Parcelable> list = IntentCompat.getParcelableArrayListExtra(
intent,
Intent.EXTRA_STREAM,
Parcelable.class
);
if (list != null) {
for (Parcelable parcelable : list) {
Uri stream = (Uri) parcelable;
if (stream != null) {
attachmentPresenter.addExternalAttachment(stream, type);
}
}
}
}

String subject = intent.getStringExtra(Intent.EXTRA_SUBJECT);
// Only use EXTRA_SUBJECT if the subject hasn't already been set by the mailto URI
if (subject != null && subjectView.getText().length() == 0) {
subjectView.setText(subject);
}

recipientPresenter.initFromSendOrViewIntent(intent);
}

if (ACTION_AUTOCRYPT_PEER.equals(action)) {
String trustId = intent.getStringExtra(OpenPgpApi.EXTRA_AUTOCRYPT_PEER_ID);
if (trustId != null) {
recipientPresenter.initFromTrustIdAction(trustId);
startedByExternalIntent = true;
}
}

return startedByExternalIntent;
}

@Override
Expand Down Expand Up @@ -1677,6 +1598,14 @@ public void onMessageDataLoadFailed() {
public void onMessageViewInfoLoadFinished(MessageViewInfo messageViewInfo) {
internalMessageHandler.sendEmptyMessage(MSG_PROGRESS_OFF);
loadLocalMessageForDisplay(messageViewInfo, action);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above regarding recipientMvpView

if(!recipientPresenter.isToAddressAdded()) {
recipientMvpView.requestFocusOnToField();
} else if (subjectView.getText().length() == 0) {
subjectView.requestFocus();
} else {
messageContentView.requestFocus();
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@ class RecipientMvpView(private val activity: MessageCompose) : View.OnFocusChang
val bccRecipients: List<Recipient>
get() = bccView.objects

val isToAddressAdded: Boolean
get() = presenter.isToAddressAdded()

val isCcTextEmpty: Boolean
get() = ccView.text.isEmpty()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ class RecipientPresenter(
private val replyToParser: ReplyToParser,
private val draftStateHeaderParser: AutocryptDraftStateHeaderParser,
) {
private var isToAddressAdded: Boolean = false
private lateinit var account: Account
private var alwaysBccAddresses: Array<Address>? = null
private var hasContactPicker: Boolean? = null
Expand Down Expand Up @@ -254,8 +255,11 @@ class RecipientPresenter(

private fun addToAddresses(vararg toAddresses: Address) {
addRecipientsFromAddresses(RecipientType.TO, *toAddresses)
isToAddressAdded = true
}

fun isToAddressAdded() = isToAddressAdded

private fun addCcAddresses(vararg ccAddresses: Address) {
if (ccAddresses.isNotEmpty()) {
addRecipientsFromAddresses(RecipientType.CC, *ccAddresses)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
package com.fsck.k9.ui.compose

import android.content.Intent
import android.net.Uri
import android.nfc.NfcAdapter
import android.os.Parcelable
import androidx.core.content.IntentCompat
import com.fsck.k9.activity.MessageCompose
import org.openintents.openpgp.util.OpenPgpApi

@Suppress("NestedBlockDepth", "MaxLineLength")
class IntentDataMapper {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally this would be covered by tests.


fun initFromIntent(intent: Intent): IntentData {
val action: String? = intent.action
var intentData = IntentData()

if (
Intent.ACTION_VIEW == action ||
Intent.ACTION_SENDTO == action ||
NfcAdapter.ACTION_NDEF_DISCOVERED == action
) {
intent.data?.let { data ->
intentData = intentData.copy(mailToUri = data)
}
}

if ((
Intent.ACTION_SEND == action || Intent.ACTION_SEND_MULTIPLE == action ||
Intent.ACTION_SENDTO == action || Intent.ACTION_VIEW == action
)
) {
intentData = intentData.copy(
startedByExternalIntent = true,
extraText = intent.getCharSequenceExtra(Intent.EXTRA_TEXT),
intentType = intent.type,
)

if ((Intent.ACTION_SEND == action)) {
val extraStream = IntentCompat.getParcelableExtra(
intent,
Intent.EXTRA_STREAM,
Uri::class.java,
)
intentData = intentData.copy(extraStream = extraStream)
} else {
val list: List<Parcelable>? = IntentCompat.getParcelableArrayListExtra(
intent,
Intent.EXTRA_STREAM,
Parcelable::class.java,
)
list?.let {
for (parcelable in it) {
intentData = intentData.copy(extraStream = parcelable as Uri)
}
}
}
intentData = intentData.copy(
subject = intent.getStringExtra(Intent.EXTRA_SUBJECT),
shouldInitFromSendOrViewIntent = true,
)
}

if ((MessageCompose.ACTION_AUTOCRYPT_PEER == action)) {
intentData = intentData.copy(
trustId = intent.getStringExtra(OpenPgpApi.EXTRA_AUTOCRYPT_PEER_ID),
startedByExternalIntent = true,
)
}
return intentData
}
}

data class IntentData(
val startedByExternalIntent: Boolean = false,
val shouldInitFromSendOrViewIntent: Boolean = false,
val mailToUri: Uri? = null,
val extraText: CharSequence? = null,
val intentType: String? = null,
val extraStream: Uri? = null,
val subject: String? = null,
val trustId: String? = null,
)
Loading
Loading