From 62aa1b87d0b313355970dee69427bd93f5b84b28 Mon Sep 17 00:00:00 2001 From: cketti Date: Wed, 14 Aug 2013 18:05:57 +0200 Subject: [PATCH 1/2] Fetch attachments while MessageCompose activity is running Android allows other apps to access protected content of an app without requesting the necessary permission when the app returns an Intent with FLAG_GRANT_READ_URI_PERMISSION. This regularly happens as a result of ACTION_GET_CONTENT, i.e. what we use to pick content to be attached to a message. Accessing that content only works while the receiving activity is running. Afterwards accessing the content throws a SecurityException because of the missing permission. This commit changes K-9 Mail's behavior to copy the content to a temporary file in K-9's cache directory while the activity is still running. Fixes issue 4847, 5821 This also fixes bugs related to the fact that K-9 Mail didn't save a copy of attached content in the message database. Fixes issue 1187, 3330, 4930 --- res/layout/message_compose_attachment.xml | 85 ++++--- res/values/strings.xml | 2 + src/com/fsck/k9/activity/K9Activity.java | 4 +- src/com/fsck/k9/activity/MessageCompose.java | 225 ++++++++++++------ .../loader/AttachmentContentLoader.java | 81 +++++++ .../activity/loader/AttachmentInfoLoader.java | 100 ++++++++ src/com/fsck/k9/activity/misc/Attachment.java | 129 ++++++++++ src/com/fsck/k9/mail/store/LocalStore.java | 35 ++- 8 files changed, 548 insertions(+), 113 deletions(-) create mode 100644 src/com/fsck/k9/activity/loader/AttachmentContentLoader.java create mode 100644 src/com/fsck/k9/activity/loader/AttachmentInfoLoader.java create mode 100644 src/com/fsck/k9/activity/misc/Attachment.java diff --git a/res/layout/message_compose_attachment.xml b/res/layout/message_compose_attachment.xml index 263d5a13c..60546e184 100644 --- a/res/layout/message_compose_attachment.xml +++ b/res/layout/message_compose_attachment.xml @@ -1,36 +1,55 @@ - - - + xmlns:android="http://schemas.android.com/apk/res/android" + android:layout_width="fill_parent" + android:layout_height="54dip" + android:paddingRight="6dip" + android:paddingTop="6dip" + android:paddingBottom="6dip"> + + + + + + + + + + + + + diff --git a/res/values/strings.xml b/res/values/strings.xml index b306f4420..fd4bd3f0f 100644 --- a/res/values/strings.xml +++ b/res/values/strings.xml @@ -1147,4 +1147,6 @@ Please submit bug reports, contribute new features and ask questions at on %s Mark all as read + + Loading attachment… diff --git a/src/com/fsck/k9/activity/K9Activity.java b/src/com/fsck/k9/activity/K9Activity.java index 64083758d..82ffc3835 100644 --- a/src/com/fsck/k9/activity/K9Activity.java +++ b/src/com/fsck/k9/activity/K9Activity.java @@ -3,12 +3,12 @@ package com.fsck.k9.activity; import android.os.Bundle; import android.view.MotionEvent; -import com.actionbarsherlock.app.SherlockActivity; +import com.actionbarsherlock.app.SherlockFragmentActivity; import com.fsck.k9.activity.K9ActivityCommon.K9ActivityMagic; import com.fsck.k9.activity.misc.SwipeGestureDetector.OnSwipeGestureListener; -public class K9Activity extends SherlockActivity implements K9ActivityMagic { +public class K9Activity extends SherlockFragmentActivity implements K9ActivityMagic { private K9ActivityCommon mBase; diff --git a/src/com/fsck/k9/activity/MessageCompose.java b/src/com/fsck/k9/activity/MessageCompose.java index 5d9d3759a..999bb6947 100644 --- a/src/com/fsck/k9/activity/MessageCompose.java +++ b/src/com/fsck/k9/activity/MessageCompose.java @@ -5,19 +5,18 @@ import android.annotation.TargetApi; import android.app.AlertDialog; import android.app.AlertDialog.Builder; import android.app.Dialog; -import android.content.ContentResolver; import android.content.Context; import android.content.DialogInterface; import android.content.Intent; import android.content.pm.ActivityInfo; -import android.database.Cursor; import android.net.Uri; import android.os.AsyncTask; import android.os.Build; import android.os.Bundle; import android.os.Handler; import android.os.Parcelable; -import android.provider.OpenableColumns; +import android.support.v4.app.LoaderManager; +import android.support.v4.content.Loader; import android.text.TextWatcher; import android.text.util.Rfc822Tokenizer; import android.util.Log; @@ -55,6 +54,9 @@ import com.fsck.k9.Identity; import com.fsck.k9.K9; import com.fsck.k9.Preferences; import com.fsck.k9.R; +import com.fsck.k9.activity.loader.AttachmentContentLoader; +import com.fsck.k9.activity.loader.AttachmentInfoLoader; +import com.fsck.k9.activity.misc.Attachment; import com.fsck.k9.controller.MessagingController; import com.fsck.k9.controller.MessagingListener; import com.fsck.k9.crypto.CryptoProvider; @@ -86,14 +88,13 @@ import org.htmlcleaner.CleanerProperties; import org.htmlcleaner.HtmlCleaner; import org.htmlcleaner.SimpleHtmlSerializer; import org.htmlcleaner.TagNode; -import java.io.File; -import java.io.Serializable; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.Date; import java.util.HashMap; import java.util.HashSet; +import java.util.Iterator; import java.util.List; import java.util.Locale; import java.util.Map; @@ -147,6 +148,8 @@ public class MessageCompose extends K9Activity implements OnClickListener { private static final String STATE_KEY_QUOTED_TEXT_FORMAT = "com.fsck.k9.activity.MessageCompose.quotedTextFormat"; + private static final String LOADER_ARG_ATTACHMENT = "attachment"; + private static final int MSG_PROGRESS_ON = 1; private static final int MSG_PROGRESS_OFF = 2; private static final int MSG_SKIPPED_ATTACHMENTS = 3; @@ -218,6 +221,7 @@ public class MessageCompose extends K9Activity implements OnClickListener { * have already been added from the restore of the view state. */ private boolean mSourceMessageProcessed = false; + private int mMaxLoaderId = 0; enum Action { COMPOSE, @@ -365,14 +369,6 @@ public class MessageCompose extends K9Activity implements OnClickListener { private ContextThemeWrapper mThemeContext; - static class Attachment implements Serializable { - private static final long serialVersionUID = 3642382876618963734L; - public String name; - public String contentType; - public long size; - public Uri uri; - } - /** * Compose a new message using the given account. If account is null the default account * will be used. @@ -1077,12 +1073,13 @@ public class MessageCompose extends K9Activity implements OnClickListener { @Override protected void onSaveInstanceState(Bundle outState) { super.onSaveInstanceState(outState); - ArrayList attachments = new ArrayList(); + ArrayList attachments = new ArrayList(); for (int i = 0, count = mAttachments.getChildCount(); i < count; i++) { View view = mAttachments.getChildAt(i); Attachment attachment = (Attachment) view.getTag(); - attachments.add(attachment.uri); + attachments.add(attachment); } + outState.putParcelableArrayList(STATE_KEY_ATTACHMENTS, attachments); outState.putBoolean(STATE_KEY_CC_SHOWN, mCcWrapper.getVisibility() == View.VISIBLE); outState.putBoolean(STATE_KEY_BCC_SHOWN, mBccWrapper.getVisibility() == View.VISIBLE); @@ -1104,11 +1101,22 @@ public class MessageCompose extends K9Activity implements OnClickListener { @Override protected void onRestoreInstanceState(Bundle savedInstanceState) { super.onRestoreInstanceState(savedInstanceState); - ArrayList attachments = savedInstanceState.getParcelableArrayList(STATE_KEY_ATTACHMENTS); + mAttachments.removeAllViews(); - for (Parcelable p : attachments) { - Uri uri = (Uri) p; - addAttachment(uri); + mMaxLoaderId = 0; + + ArrayList attachments = savedInstanceState.getParcelableArrayList(STATE_KEY_ATTACHMENTS); + for (Attachment attachment : attachments) { + addAttachmentView(attachment); + if (attachment.loaderId > mMaxLoaderId) { + mMaxLoaderId = attachment.loaderId; + } + + if (attachment.state == Attachment.LoadingState.URI_ONLY) { + initAttachmentInfoLoader(attachment); + } else if (attachment.state == Attachment.LoadingState.METADATA) { + initAttachmentContentLoader(attachment); + } } mReadReceipt = savedInstanceState @@ -1474,8 +1482,11 @@ public class MessageCompose extends K9Activity implements OnClickListener { for (int i = 0, count = mAttachments.getChildCount(); i < count; i++) { Attachment attachment = (Attachment) mAttachments.getChildAt(i).getTag(); - MimeBodyPart bp = new MimeBodyPart( - new LocalStore.LocalAttachmentBody(attachment.uri, getApplication())); + if (attachment.state != Attachment.LoadingState.COMPLETE) { + continue; + } + + MimeBodyPart bp = new MimeBodyPart(new LocalStore.TempFileBody(attachment.filename)); /* * Correctly encode the filename here. Otherwise the whole @@ -1911,71 +1922,131 @@ public class MessageCompose extends K9Activity implements OnClickListener { } private void addAttachment(Uri uri, String contentType) { - long size = -1; - String name = null; - - ContentResolver contentResolver = getContentResolver(); - - Cursor metadataCursor = contentResolver.query( - uri, - new String[] { OpenableColumns.DISPLAY_NAME, OpenableColumns.SIZE }, - null, - null, - null); - - if (metadataCursor != null) { - try { - if (metadataCursor.moveToFirst()) { - name = metadataCursor.getString(0); - size = metadataCursor.getInt(1); - } - } finally { - metadataCursor.close(); - } - } - - if (name == null) { - name = uri.getLastPathSegment(); - } - - String usableContentType = contentType; - if ((usableContentType == null) || (usableContentType.indexOf('*') != -1)) { - usableContentType = contentResolver.getType(uri); - } - if (usableContentType == null) { - usableContentType = MimeUtility.getMimeTypeByExtension(name); - } - - if (size <= 0) { - String uriString = uri.toString(); - if (uriString.startsWith("file://")) { - Log.v(K9.LOG_TAG, uriString.substring("file://".length())); - File f = new File(uriString.substring("file://".length())); - size = f.length(); - } else { - Log.v(K9.LOG_TAG, "Not a file: " + uriString); - } - } else { - Log.v(K9.LOG_TAG, "old attachment.size: " + size); - } - Log.v(K9.LOG_TAG, "new attachment.size: " + size); - Attachment attachment = new Attachment(); + attachment.state = Attachment.LoadingState.URI_ONLY; attachment.uri = uri; - attachment.contentType = usableContentType; - attachment.name = name; - attachment.size = size; + attachment.contentType = contentType; + attachment.loaderId = ++mMaxLoaderId; + + addAttachmentView(attachment); + + initAttachmentInfoLoader(attachment); + } + + private void initAttachmentInfoLoader(Attachment attachment) { + LoaderManager loaderManager = getSupportLoaderManager(); + Bundle bundle = new Bundle(); + bundle.putParcelable(LOADER_ARG_ATTACHMENT, attachment); + loaderManager.initLoader(attachment.loaderId, bundle, mAttachmentInfoLoaderCallback); + } + + private void initAttachmentContentLoader(Attachment attachment) { + LoaderManager loaderManager = getSupportLoaderManager(); + Bundle bundle = new Bundle(); + bundle.putParcelable(LOADER_ARG_ATTACHMENT, attachment); + loaderManager.initLoader(attachment.loaderId, bundle, mAttachmentContentLoaderCallback); + } + + private void addAttachmentView(Attachment attachment) { + boolean hasMetadata = (attachment.state != Attachment.LoadingState.URI_ONLY); + boolean isLoadingComplete = (attachment.state == Attachment.LoadingState.COMPLETE); View view = getLayoutInflater().inflate(R.layout.message_compose_attachment, mAttachments, false); - TextView nameView = (TextView)view.findViewById(R.id.attachment_name); - ImageButton delete = (ImageButton)view.findViewById(R.id.attachment_delete); - nameView.setText(attachment.name); - delete.setOnClickListener(this); + TextView nameView = (TextView) view.findViewById(R.id.attachment_name); + View progressBar = view.findViewById(R.id.progressBar); + + if (hasMetadata) { + nameView.setText(attachment.name); + } else { + nameView.setText(R.string.loading_attachment); + } + + progressBar.setVisibility(isLoadingComplete ? View.GONE : View.VISIBLE); + + ImageButton delete = (ImageButton) view.findViewById(R.id.attachment_delete); + delete.setOnClickListener(MessageCompose.this); delete.setTag(view); + view.setTag(attachment); mAttachments.addView(view); } + private View getAttachmentView(int loaderId) { + for (int i = 0, childCount = mAttachments.getChildCount(); i < childCount; i++) { + View view = mAttachments.getChildAt(i); + Attachment tag = (Attachment) view.getTag(); + if (tag != null && tag.loaderId == loaderId) { + return view; + } + } + + return null; + } + + private LoaderManager.LoaderCallbacks mAttachmentInfoLoaderCallback = + new LoaderManager.LoaderCallbacks() { + @Override + public Loader onCreateLoader(int id, Bundle args) { + Attachment attachment = args.getParcelable(LOADER_ARG_ATTACHMENT); + return new AttachmentInfoLoader(MessageCompose.this, attachment); + } + + @Override + public void onLoadFinished(Loader loader, Attachment attachment) { + int loaderId = loader.getId(); + + View view = getAttachmentView(loaderId); + if (view != null) { + view.setTag(attachment); + + TextView nameView = (TextView) view.findViewById(R.id.attachment_name); + nameView.setText(attachment.name); + + attachment.loaderId = ++mMaxLoaderId; + initAttachmentContentLoader(attachment); + } + + getSupportLoaderManager().destroyLoader(loaderId); + } + + @Override + public void onLoaderReset(Loader loader) { + } + }; + + private LoaderManager.LoaderCallbacks mAttachmentContentLoaderCallback = + new LoaderManager.LoaderCallbacks() { + @Override + public Loader onCreateLoader(int id, Bundle args) { + Attachment attachment = args.getParcelable(LOADER_ARG_ATTACHMENT); + return new AttachmentContentLoader(MessageCompose.this, attachment); + } + + @Override + public void onLoadFinished(Loader loader, Attachment attachment) { + int loaderId = loader.getId(); + + View view = getAttachmentView(loaderId); + if (view != null) { + if (attachment.state == Attachment.LoadingState.COMPLETE) { + view.setTag(attachment); + + View progressBar = view.findViewById(R.id.progressBar); + progressBar.setVisibility(View.GONE); + } else { + mAttachments.removeView(view); + } + } + + getSupportLoaderManager().destroyLoader(loaderId); + } + + @Override + public void onLoaderReset(Loader loader) { + } + }; + + @Override protected void onActivityResult(int requestCode, int resultCode, Intent data) { // if a CryptoSystem activity is returning, then mPreventDraftSaving was set to true diff --git a/src/com/fsck/k9/activity/loader/AttachmentContentLoader.java b/src/com/fsck/k9/activity/loader/AttachmentContentLoader.java new file mode 100644 index 000000000..7d677ecbc --- /dev/null +++ b/src/com/fsck/k9/activity/loader/AttachmentContentLoader.java @@ -0,0 +1,81 @@ +package com.fsck.k9.activity.loader; + +import android.content.Context; +import android.support.v4.content.AsyncTaskLoader; +import android.util.Log; + +import com.fsck.k9.K9; +import com.fsck.k9.activity.misc.Attachment; + +import org.apache.commons.io.FileUtils; +import org.apache.commons.io.IOUtils; + +import java.io.File; +import java.io.FileOutputStream; +import java.io.IOException; +import java.io.InputStream; + +/** + * Loader to fetch the content of an attachment. + * + * This will copy the data to a temporary file in our app's cache directory. + */ +public class AttachmentContentLoader extends AsyncTaskLoader { + private static final String FILENAME_PREFIX = "attachment"; + + private final Attachment mAttachment; + + public AttachmentContentLoader(Context context, Attachment attachment) { + super(context); + mAttachment = attachment; + } + + @Override + protected void onStartLoading() { + if (mAttachment.state == Attachment.LoadingState.COMPLETE) { + deliverResult(mAttachment); + } + + if (takeContentChanged() || mAttachment.state == Attachment.LoadingState.METADATA) { + forceLoad(); + } + } + + @Override + public Attachment loadInBackground() { + Context context = getContext(); + + try { + File file = File.createTempFile(FILENAME_PREFIX, null, context.getCacheDir()); + file.deleteOnExit(); + + if (K9.DEBUG) { + Log.v(K9.LOG_TAG, "Saving attachment to " + file.getAbsolutePath()); + } + + InputStream in = context.getContentResolver().openInputStream(mAttachment.uri); + try { + FileOutputStream out = new FileOutputStream(file); + try { + IOUtils.copy(in, out); + } finally { + out.close(); + } + } finally { + in.close(); + } + + mAttachment.filename = file.getAbsolutePath(); + mAttachment.state = Attachment.LoadingState.COMPLETE; + + return mAttachment; + } catch (IOException e) { + e.printStackTrace(); + } + + mAttachment.filename = null; + mAttachment.state = Attachment.LoadingState.CANCELLED; + + return mAttachment; + } +} diff --git a/src/com/fsck/k9/activity/loader/AttachmentInfoLoader.java b/src/com/fsck/k9/activity/loader/AttachmentInfoLoader.java new file mode 100644 index 000000000..74c874ae8 --- /dev/null +++ b/src/com/fsck/k9/activity/loader/AttachmentInfoLoader.java @@ -0,0 +1,100 @@ +package com.fsck.k9.activity.loader; + +import android.content.ContentResolver; +import android.content.Context; +import android.database.Cursor; +import android.net.Uri; +import android.provider.OpenableColumns; +import android.support.v4.content.AsyncTaskLoader; +import android.util.Log; + +import com.fsck.k9.K9; +import com.fsck.k9.activity.misc.Attachment; +import com.fsck.k9.mail.internet.MimeUtility; + +import java.io.File; + +/** + * Loader to fetch metadata of an attachment. + */ +public class AttachmentInfoLoader extends AsyncTaskLoader { + private final Attachment mAttachment; + + public AttachmentInfoLoader(Context context, Attachment attachment) { + super(context); + mAttachment = attachment; + } + + @Override + protected void onStartLoading() { + if (mAttachment.state == Attachment.LoadingState.METADATA) { + deliverResult(mAttachment); + } + + if (takeContentChanged() || mAttachment.state == Attachment.LoadingState.URI_ONLY) { + forceLoad(); + } + } + + @Override + public Attachment loadInBackground() { + Uri uri = mAttachment.uri; + String contentType = mAttachment.contentType; + + long size = -1; + String name = null; + + ContentResolver contentResolver = getContext().getContentResolver(); + + Cursor metadataCursor = contentResolver.query( + uri, + new String[] { OpenableColumns.DISPLAY_NAME, OpenableColumns.SIZE }, + null, + null, + null); + + if (metadataCursor != null) { + try { + if (metadataCursor.moveToFirst()) { + name = metadataCursor.getString(0); + size = metadataCursor.getInt(1); + } + } finally { + metadataCursor.close(); + } + } + + if (name == null) { + name = uri.getLastPathSegment(); + } + + String usableContentType = contentType; + if ((usableContentType == null) || (usableContentType.indexOf('*') != -1)) { + usableContentType = contentResolver.getType(uri); + } + if (usableContentType == null) { + usableContentType = MimeUtility.getMimeTypeByExtension(name); + } + + if (size <= 0) { + String uriString = uri.toString(); + if (uriString.startsWith("file://")) { + Log.v(K9.LOG_TAG, uriString.substring("file://".length())); + File f = new File(uriString.substring("file://".length())); + size = f.length(); + } else { + Log.v(K9.LOG_TAG, "Not a file: " + uriString); + } + } else { + Log.v(K9.LOG_TAG, "old attachment.size: " + size); + } + Log.v(K9.LOG_TAG, "new attachment.size: " + size); + + mAttachment.contentType = usableContentType; + mAttachment.name = name; + mAttachment.size = size; + mAttachment.state = Attachment.LoadingState.METADATA; + + return mAttachment; + } +} diff --git a/src/com/fsck/k9/activity/misc/Attachment.java b/src/com/fsck/k9/activity/misc/Attachment.java new file mode 100644 index 000000000..e64165783 --- /dev/null +++ b/src/com/fsck/k9/activity/misc/Attachment.java @@ -0,0 +1,129 @@ +package com.fsck.k9.activity.misc; + +import android.net.Uri; +import android.os.Parcel; +import android.os.Parcelable; + +/** + * Container class for information about an attachment. + * + * This is used by {@link com.fsck.k9.activity.MessageCompose} to fetch and manage attachments. + */ +public class Attachment implements Parcelable { + /** + * The URI pointing to the source of the attachment. + * + * In most cases this will be a {@code content://}-URI. + */ + public Uri uri; + + /** + * The current loading state. + */ + public LoadingState state; + + /** + * The ID of the loader that is used to load the metadata or contents. + */ + public int loaderId; + + /** + * The content type of the attachment. + * + * Only valid when {@link #state} is {@link LoadingState#METADATA} or + * {@link LoadingState#COMPLETE}. + */ + public String contentType; + + /** + * The (file)name of the attachment. + * + * Only valid when {@link #state} is {@link LoadingState#METADATA} or + * {@link LoadingState#COMPLETE}. + */ + public String name; + + /** + * The size of the attachment. + * + * Only valid when {@link #state} is {@link LoadingState#METADATA} or + * {@link LoadingState#COMPLETE}. + */ + public long size; + + /** + * The name of the temporary file containing the local copy of the attachment. + * + * Only valid when {@link #state} is {@link LoadingState#COMPLETE}. + */ + public String filename; + + + public Attachment() {} + + public static enum LoadingState { + /** + * The only thing we know about this attachment is {@link #uri}. + */ + URI_ONLY, + + /** + * The metadata of this attachment have been loaded. + * + * {@link #contentType}, {@link #name}, and {@link #size} should contain usable values. + */ + METADATA, + + /** + * The contents of the attachments have been copied to the temporary file {@link #filename}. + */ + COMPLETE, + + /** + * Something went wrong while trying to fetch the attachment's contents. + */ + CANCELLED + } + + + // === Parcelable === + + @Override + public int describeContents() { + return 0; + } + + @Override + public void writeToParcel(Parcel dest, int flags) { + dest.writeParcelable(uri, flags); + dest.writeSerializable(state); + dest.writeInt(loaderId); + dest.writeString(contentType); + dest.writeString(name); + dest.writeLong(size); + dest.writeString(filename); + } + + public static final Parcelable.Creator CREATOR = + new Parcelable.Creator() { + @Override + public Attachment createFromParcel(Parcel in) { + return new Attachment(in); + } + + @Override + public Attachment[] newArray(int size) { + return new Attachment[size]; + } + }; + + public Attachment(Parcel in) { + uri = in.readParcelable(Uri.class.getClassLoader()); + state = (LoadingState) in.readSerializable(); + loaderId = in.readInt(); + contentType = in.readString(); + name = in.readString(); + size = in.readLong(); + filename = in.readString(); + } +} diff --git a/src/com/fsck/k9/mail/store/LocalStore.java b/src/com/fsck/k9/mail/store/LocalStore.java index 5575499a7..de1c4272e 100644 --- a/src/com/fsck/k9/mail/store/LocalStore.java +++ b/src/com/fsck/k9/mail/store/LocalStore.java @@ -3,6 +3,7 @@ package com.fsck.k9.mail.store; import java.io.ByteArrayInputStream; import java.io.File; +import java.io.FileInputStream; import java.io.FileNotFoundException; import java.io.FileOutputStream; import java.io.IOException; @@ -90,6 +91,7 @@ public class LocalStore extends Store implements Serializable { private static final Message[] EMPTY_MESSAGE_ARRAY = new Message[0]; private static final String[] EMPTY_STRING_ARRAY = new String[0]; private static final Flag[] EMPTY_FLAG_ARRAY = new Flag[0]; + private static final byte[] EMPTY_BYTE_ARRAY = new byte[0]; /* * a String containing the columns getMessages expects to work with @@ -3984,8 +3986,39 @@ public class LocalStore extends Store implements Serializable { } } + public static class TempFileBody implements Body { + private final File mFile; + + public TempFileBody(String filename) { + mFile = new File(filename); + } + + @Override + public InputStream getInputStream() throws MessagingException { + try { + return new FileInputStream(mFile); + } catch (FileNotFoundException e) { + return new ByteArrayInputStream(EMPTY_BYTE_ARRAY); + } + } + + @Override + public void writeTo(OutputStream out) throws IOException, MessagingException { + InputStream in = getInputStream(); + try { + Base64OutputStream base64Out = new Base64OutputStream(out); + try { + IOUtils.copy(in, base64Out); + } finally { + base64Out.close(); + } + } finally { + in.close(); + } + } + } + public static class LocalAttachmentBody implements Body { - private static final byte[] EMPTY_BYTE_ARRAY = new byte[0]; private Application mApplication; private Uri mUri; From 677d6c923d7dc7aa00c35a32fe0067a71611f70d Mon Sep 17 00:00:00 2001 From: cketti Date: Wed, 25 Sep 2013 03:46:11 +0200 Subject: [PATCH 2/2] Don't stop the activity before attachments have been fetched Display a progress dialog when the user tries to send the message or save a draft and the attachments haven't been fetched completely. --- res/values/strings.xml | 3 + src/com/fsck/k9/activity/MessageCompose.java | 147 +++++++++++++++++- .../fsck/k9/fragment/MessageViewFragment.java | 4 +- .../k9/fragment/ProgressDialogFragment.java | 24 ++- 4 files changed, 172 insertions(+), 6 deletions(-) diff --git a/res/values/strings.xml b/res/values/strings.xml index fd4bd3f0f..638c6507c 100644 --- a/res/values/strings.xml +++ b/res/values/strings.xml @@ -1149,4 +1149,7 @@ Please submit bug reports, contribute new features and ask questions at Mark all as read Loading attachment… + Sending message + Saving draft + Fetching attachment… diff --git a/src/com/fsck/k9/activity/MessageCompose.java b/src/com/fsck/k9/activity/MessageCompose.java index 999bb6947..ab999c0ec 100644 --- a/src/com/fsck/k9/activity/MessageCompose.java +++ b/src/com/fsck/k9/activity/MessageCompose.java @@ -61,6 +61,7 @@ import com.fsck.k9.controller.MessagingController; import com.fsck.k9.controller.MessagingListener; import com.fsck.k9.crypto.CryptoProvider; import com.fsck.k9.crypto.PgpData; +import com.fsck.k9.fragment.ProgressDialogFragment; import com.fsck.k9.helper.ContactItem; import com.fsck.k9.helper.Contacts; import com.fsck.k9.helper.HtmlConverter; @@ -94,7 +95,6 @@ import java.util.Collections; import java.util.Date; import java.util.HashMap; import java.util.HashSet; -import java.util.Iterator; import java.util.List; import java.util.Locale; import java.util.Map; @@ -103,7 +103,9 @@ import java.util.StringTokenizer; import java.util.regex.Matcher; import java.util.regex.Pattern; -public class MessageCompose extends K9Activity implements OnClickListener { +public class MessageCompose extends K9Activity implements OnClickListener, + ProgressDialogFragment.CancelListener { + private static final int DIALOG_SAVE_OR_DISCARD_DRAFT_MESSAGE = 1; private static final int DIALOG_REFUSE_TO_SAVE_DRAFT_MARKED_ENCRYPTED = 2; private static final int DIALOG_CONTINUE_WITHOUT_PUBLIC_KEY = 3; @@ -147,14 +149,19 @@ public class MessageCompose extends K9Activity implements OnClickListener { "com.fsck.k9.activity.MessageCompose.forcePlainText"; private static final String STATE_KEY_QUOTED_TEXT_FORMAT = "com.fsck.k9.activity.MessageCompose.quotedTextFormat"; + private static final String STATE_KEY_NUM_ATTACHMENTS_LOADING = "numAttachmentsLoading"; + private static final String STATE_KEY_WAITING_FOR_ATTACHMENTS = "waitingForAttachments"; private static final String LOADER_ARG_ATTACHMENT = "attachment"; + private static final String FRAGMENT_WAITING_FOR_ATTACHMENT = "waitingForAttachment"; + private static final int MSG_PROGRESS_ON = 1; private static final int MSG_PROGRESS_OFF = 2; private static final int MSG_SKIPPED_ATTACHMENTS = 3; private static final int MSG_SAVED_DRAFT = 4; private static final int MSG_DISCARDED_DRAFT = 5; + private static final int MSG_PERFORM_STALLED_ACTION = 6; private static final int ACTIVITY_REQUEST_PICK_ATTACHMENT = 1; private static final int CONTACT_PICKER_TO = 4; @@ -326,6 +333,23 @@ public class MessageCompose extends K9Activity implements OnClickListener { */ private long mDraftId = INVALID_DRAFT_ID; + /** + * Number of attachments currently being fetched. + */ + private int mNumAttachmentsLoading = 0; + + private enum WaitingAction { + NONE, + SEND, + SAVE + } + + /** + * Specifies what action to perform once attachments have been fetched. + */ + private WaitingAction mWaitingForAttachments = WaitingAction.NONE; + + private Handler mHandler = new Handler() { @Override public void handleMessage(android.os.Message msg) { @@ -354,6 +378,9 @@ public class MessageCompose extends K9Activity implements OnClickListener { getString(R.string.message_discarded_toast), Toast.LENGTH_LONG).show(); break; + case MSG_PERFORM_STALLED_ACTION: + performStalledAction(); + break; default: super.handleMessage(msg); break; @@ -1080,6 +1107,8 @@ public class MessageCompose extends K9Activity implements OnClickListener { attachments.add(attachment); } + outState.putInt(STATE_KEY_NUM_ATTACHMENTS_LOADING, mNumAttachmentsLoading); + outState.putString(STATE_KEY_WAITING_FOR_ATTACHMENTS, mWaitingForAttachments.name()); outState.putParcelableArrayList(STATE_KEY_ATTACHMENTS, attachments); outState.putBoolean(STATE_KEY_CC_SHOWN, mCcWrapper.getVisibility() == View.VISIBLE); outState.putBoolean(STATE_KEY_BCC_SHOWN, mBccWrapper.getVisibility() == View.VISIBLE); @@ -1105,6 +1134,16 @@ public class MessageCompose extends K9Activity implements OnClickListener { mAttachments.removeAllViews(); mMaxLoaderId = 0; + mNumAttachmentsLoading = savedInstanceState.getInt(STATE_KEY_NUM_ATTACHMENTS_LOADING); + mWaitingForAttachments = WaitingAction.NONE; + try { + String waitingFor = savedInstanceState.getString(STATE_KEY_WAITING_FOR_ATTACHMENTS); + mWaitingForAttachments = WaitingAction.valueOf(waitingFor); + } catch (Exception e) { + Log.w(K9.LOG_TAG, "Couldn't read value \" + STATE_KEY_WAITING_FOR_ATTACHMENTS +" + + "\" from saved instance state", e); + } + ArrayList attachments = savedInstanceState.getParcelableArrayList(STATE_KEY_ATTACHMENTS); for (Attachment attachment : attachments) { addAttachmentView(attachment); @@ -1784,6 +1823,20 @@ public class MessageCompose extends K9Activity implements OnClickListener { Toast.makeText(this, getString(R.string.message_compose_error_no_recipients), Toast.LENGTH_LONG).show(); return; } + + if (mWaitingForAttachments != WaitingAction.NONE) { + return; + } + + if (mNumAttachmentsLoading > 0) { + mWaitingForAttachments = WaitingAction.SEND; + showWaitingForAttachmentDialog(); + } else { + performSend(); + } + } + + private void performSend() { final CryptoProvider crypto = mAccount.getCryptoProvider(); if (mEncryptCheckbox.isChecked() && !mPgpData.hasEncryptionKeys()) { // key selection before encryption @@ -1847,6 +1900,19 @@ public class MessageCompose extends K9Activity implements OnClickListener { } private void onSave() { + if (mWaitingForAttachments != WaitingAction.NONE) { + return; + } + + if (mNumAttachmentsLoading > 0) { + mWaitingForAttachments = WaitingAction.SAVE; + showWaitingForAttachmentDialog(); + } else { + performSend(); + } + } + + private void performSave() { saveIfNeeded(); finish(); } @@ -1987,6 +2053,7 @@ public class MessageCompose extends K9Activity implements OnClickListener { new LoaderManager.LoaderCallbacks() { @Override public Loader onCreateLoader(int id, Bundle args) { + onFetchAttachmentStarted(); Attachment attachment = args.getParcelable(LOADER_ARG_ATTACHMENT); return new AttachmentInfoLoader(MessageCompose.this, attachment); } @@ -2004,6 +2071,8 @@ public class MessageCompose extends K9Activity implements OnClickListener { attachment.loaderId = ++mMaxLoaderId; initAttachmentContentLoader(attachment); + } else { + onFetchAttachmentFinished(); } getSupportLoaderManager().destroyLoader(loaderId); @@ -2011,6 +2080,7 @@ public class MessageCompose extends K9Activity implements OnClickListener { @Override public void onLoaderReset(Loader loader) { + onFetchAttachmentFinished(); } }; @@ -2038,14 +2108,48 @@ public class MessageCompose extends K9Activity implements OnClickListener { } } + onFetchAttachmentFinished(); + getSupportLoaderManager().destroyLoader(loaderId); } @Override public void onLoaderReset(Loader loader) { + onFetchAttachmentFinished(); } }; + private void onFetchAttachmentStarted() { + mNumAttachmentsLoading += 1; + } + + private void onFetchAttachmentFinished() { + // We're not allowed to perform fragment transactions when called from onLoadFinished(). + // So we use the Handler to call performStalledAction(). + mHandler.sendEmptyMessage(MSG_PERFORM_STALLED_ACTION); + } + + private void performStalledAction() { + mNumAttachmentsLoading -= 1; + + WaitingAction waitingFor = mWaitingForAttachments; + mWaitingForAttachments = WaitingAction.NONE; + + if (waitingFor != WaitingAction.NONE) { + dismissWaitingForAttachmentDialog(); + } + + switch (waitingFor) { + case SEND: { + performSend(); + break; + } + case SAVE: { + performSave(); + break; + } + } + } @Override protected void onActivityResult(int requestCode, int resultCode, Intent data) { @@ -2380,6 +2484,45 @@ public class MessageCompose extends K9Activity implements OnClickListener { } } + private void showWaitingForAttachmentDialog() { + String title; + + switch (mWaitingForAttachments) { + case SEND: { + title = getString(R.string.fetching_attachment_dialog_title_send); + break; + } + case SAVE: { + title = getString(R.string.fetching_attachment_dialog_title_save); + break; + } + default: { + return; + } + } + + ProgressDialogFragment fragment = ProgressDialogFragment.newInstance(title, + getString(R.string.fetching_attachment_dialog_message)); + fragment.show(getSupportFragmentManager(), FRAGMENT_WAITING_FOR_ATTACHMENT); + } + + public void onCancel(ProgressDialogFragment fragment) { + attachmentProgressDialogCancelled(); + } + + void attachmentProgressDialogCancelled() { + mWaitingForAttachments = WaitingAction.NONE; + } + + private void dismissWaitingForAttachmentDialog() { + ProgressDialogFragment fragment = (ProgressDialogFragment) + getSupportFragmentManager().findFragmentByTag(FRAGMENT_WAITING_FOR_ATTACHMENT); + + if (fragment != null) { + fragment.dismiss(); + } + } + @Override public Dialog onCreateDialog(int id) { switch (id) { diff --git a/src/com/fsck/k9/fragment/MessageViewFragment.java b/src/com/fsck/k9/fragment/MessageViewFragment.java index d2bd16d89..711fcb93d 100644 --- a/src/com/fsck/k9/fragment/MessageViewFragment.java +++ b/src/com/fsck/k9/fragment/MessageViewFragment.java @@ -752,8 +752,8 @@ public class MessageViewFragment extends SherlockFragment implements OnClickList break; } case R.id.dialog_attachment_progress: { - String title = getString(R.string.dialog_attachment_progress_title); - fragment = ProgressDialogFragment.newInstance(title); + String message = getString(R.string.dialog_attachment_progress_title); + fragment = ProgressDialogFragment.newInstance(null, message); break; } default: { diff --git a/src/com/fsck/k9/fragment/ProgressDialogFragment.java b/src/com/fsck/k9/fragment/ProgressDialogFragment.java index 671758a86..467bbcc83 100644 --- a/src/com/fsck/k9/fragment/ProgressDialogFragment.java +++ b/src/com/fsck/k9/fragment/ProgressDialogFragment.java @@ -2,19 +2,22 @@ package com.fsck.k9.fragment; import android.app.Dialog; import android.app.ProgressDialog; +import android.content.DialogInterface; import android.os.Bundle; import com.actionbarsherlock.app.SherlockDialogFragment; public class ProgressDialogFragment extends SherlockDialogFragment { - private static final String ARG_TITLE = "title"; + protected static final String ARG_TITLE = "title"; + protected static final String ARG_MESSAGE = "message"; - public static ProgressDialogFragment newInstance(String title) { + public static ProgressDialogFragment newInstance(String title, String message) { ProgressDialogFragment fragment = new ProgressDialogFragment(); Bundle args = new Bundle(); args.putString(ARG_TITLE, title); + args.putString(ARG_MESSAGE, message); fragment.setArguments(args); return fragment; @@ -25,11 +28,28 @@ public class ProgressDialogFragment extends SherlockDialogFragment { public Dialog onCreateDialog(Bundle savedInstanceState) { Bundle args = getArguments(); String title = args.getString(ARG_TITLE); + String message = args.getString(ARG_MESSAGE); ProgressDialog dialog = new ProgressDialog(getActivity()); dialog.setIndeterminate(true); dialog.setTitle(title); + dialog.setMessage(message); return dialog; } + + @Override + public void onCancel(DialogInterface dialog) { + CancelListener listener = (CancelListener) getActivity(); + if (listener != null && listener instanceof CancelListener) { + listener.onCancel(this); + } + + super.onCancel(dialog); + } + + + public interface CancelListener { + void onCancel(ProgressDialogFragment fragment); + } }