In NfcOperationsActivity, show NFC errors inline, in others show as Notify

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateKeyActivity.java

@@ -75,11 +75,8 @@ public class CreateKeyActivity extends BaseNfcActivity {
         // React on NDEF_DISCOVERED from Manifest
         // NOTE: ACTION_NDEF_DISCOVERED and not ACTION_TAG_DISCOVERED like in BaseNfcActivity
         if (NfcAdapter.ACTION_NDEF_DISCOVERED.equals(getIntent().getAction())) {
-            try {
+
-                handleTagDiscoveredIntent(getIntent());
+            handleIntentInBackground(getIntent());
-            } catch (IOException e) {
-                handleNfcError(e);
-            }
 
             setTitle(R.string.title_manage_my_keys);
 

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/NfcOperationActivity.java

@@ -10,6 +10,7 @@ import android.content.Intent;
 import android.os.AsyncTask;
 import android.os.Bundle;
 import android.view.WindowManager;
+import android.widget.TextView;
 import android.widget.ViewAnimator;
 
 import org.sufficientlysecure.keychain.Constants;
@@ -23,6 +24,7 @@ import org.sufficientlysecure.keychain.service.PassphraseCacheService;
 import org.sufficientlysecure.keychain.service.input.CryptoInputParcel;
 import org.sufficientlysecure.keychain.service.input.RequiredInputParcel;
 import org.sufficientlysecure.keychain.ui.base.BaseNfcActivity;
+import org.sufficientlysecure.keychain.ui.util.Notify;
 import org.sufficientlysecure.keychain.util.Log;
 import org.sufficientlysecure.keychain.util.Passphrase;
 import org.sufficientlysecure.keychain.util.Preferences;
@@ -47,6 +49,7 @@ public class NfcOperationActivity extends BaseNfcActivity {
     public static final String RESULT_DATA = "result_data";
 
     public ViewAnimator vAnimator;
+    public TextView vErrorText;
 
     private RequiredInputParcel mRequiredInput;
     private Intent mServiceIntent;
@@ -66,6 +69,7 @@ public class NfcOperationActivity extends BaseNfcActivity {
 
         vAnimator = (ViewAnimator) findViewById(R.id.view_animator);
         vAnimator.setDisplayedChild(0);
+        vErrorText = (TextView) findViewById(R.id.nfc_activity_error_text);
 
         Intent intent = getIntent();
         Bundle data = intent.getExtras();
@@ -229,6 +233,12 @@ public class NfcOperationActivity extends BaseNfcActivity {
         }.execute();
     }
 
+    @Override
+    protected void onNfcError(String error) {
+        vErrorText.setText(error);
+        vAnimator.setDisplayedChild(3);
+    }
+
     private boolean shouldPutKey(byte[] fingerprint, int idx) throws IOException {
         byte[] cardFingerprint = nfcGetFingerprint(idx);
         // Slot is empty, or contains this key already. PUT KEY operation is safe

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/base/BaseNfcActivity.java

@@ -79,6 +79,95 @@ public abstract class BaseNfcActivity extends BaseActivity {
     private String mNfcUserId;
     private byte[] mNfcAid;
 
+    /**
+     * Override to change UI before NFC handling (UI thread)
+     */
+    protected void onNfcPreExecute() {
+    }
+
+    /**
+     * Override to implement NFC operations (background thread)
+     */
+    protected void doNfcInBackground() throws IOException {
+        mNfcFingerprints = nfcGetFingerprints();
+        mNfcUserId = nfcGetUserId();
+        mNfcAid = nfcGetAid();
+    }
+
+    /**
+     * Override to handle result of NFC operations (UI thread)
+     */
+    protected void onNfcPostExecute() throws IOException {
+
+        final long subKeyId = KeyFormattingUtils.getKeyIdFromFingerprint(mNfcFingerprints);
+
+        try {
+            CachedPublicKeyRing ring = new ProviderHelper(this).getCachedPublicKeyRing(
+                    KeyRings.buildUnifiedKeyRingsFindBySubkeyUri(subKeyId));
+            long masterKeyId = ring.getMasterKeyId();
+
+            Intent intent = new Intent(this, ViewKeyActivity.class);
+            intent.setData(KeyRings.buildGenericKeyRingUri(masterKeyId));
+            intent.putExtra(ViewKeyActivity.EXTRA_NFC_AID, mNfcAid);
+            intent.putExtra(ViewKeyActivity.EXTRA_NFC_USER_ID, mNfcUserId);
+            intent.putExtra(ViewKeyActivity.EXTRA_NFC_FINGERPRINTS, mNfcFingerprints);
+            startActivity(intent);
+        } catch (PgpKeyNotFoundException e) {
+            Intent intent = new Intent(this, CreateKeyActivity.class);
+            intent.putExtra(CreateKeyActivity.EXTRA_NFC_AID, mNfcAid);
+            intent.putExtra(CreateKeyActivity.EXTRA_NFC_USER_ID, mNfcUserId);
+            intent.putExtra(CreateKeyActivity.EXTRA_NFC_FINGERPRINTS, mNfcFingerprints);
+            startActivity(intent);
+        }
+    }
+
+    /**
+     * Override to use something different than Notify (UI thread)
+     */
+    protected void onNfcError(String error) {
+        Notify.create(this, error, Style.WARN).show();
+    }
+
+    public void handleIntentInBackground(final Intent intent) {
+        // Actual NFC operations are executed in doInBackground to not block the UI thread
+        new AsyncTask<Void, Void, Exception>() {
+            @Override
+            protected void onPreExecute() {
+                super.onPreExecute();
+                onNfcPreExecute();
+            }
+
+            @Override
+            protected Exception doInBackground(Void... params) {
+                try {
+                    handleTagDiscoveredIntent(intent);
+                } catch (CardException e) {
+                    return e;
+                } catch (IOException e) {
+                    return e;
+                }
+
+                return null;
+            }
+
+            @Override
+            protected void onPostExecute(Exception exception) {
+                super.onPostExecute(exception);
+
+                if (exception != null) {
+                    handleNfcError(exception);
+                    return;
+                }
+
+                try {
+                    onNfcPostExecute();
+                } catch (IOException e) {
+                    handleNfcError(e);
+                }
+            }
+        }.execute();
+    }
+
     @Override
     protected void onCreate(Bundle savedInstanceState) {
         super.onCreate(savedInstanceState);
@@ -98,49 +187,11 @@ public abstract class BaseNfcActivity extends BaseActivity {
     @Override
     public void onNewIntent(final Intent intent) {
         if (NfcAdapter.ACTION_TAG_DISCOVERED.equals(intent.getAction())) {
-
+            handleIntentInBackground(intent);
-            // Actual NFC operations are executed in doInBackground to not block the UI thread
-            new AsyncTask<Void, Void, Exception>() {
-                @Override
-                protected void onPreExecute() {
-                    super.onPreExecute();
-                    onNfcPreExecute();
-                }
-
-                @Override
-                protected Exception doInBackground(Void... params) {
-                    try {
-                        handleTagDiscoveredIntent(intent);
-                    } catch (CardException e) {
-                        return e;
-                    } catch (IOException e) {
-                        return e;
-                    }
-
-                    return null;
-                }
-
-                @Override
-                protected void onPostExecute(Exception exception) {
-                    super.onPostExecute(exception);
-
-                    if (exception != null) {
-                        handleNfcError(exception);
-                        return;
-                    }
-
-                    try {
-                        onNfcPostExecute();
-                    } catch (IOException e) {
-                        handleNfcError(e);
-                    }
-                }
-            }.execute();
-
         }
     }
 
-    public void handleNfcError(Exception e) {
+    private void handleNfcError(Exception e) {
         Log.e(Constants.TAG, "nfc error", e);
 
         short status;
@@ -151,7 +202,7 @@ public abstract class BaseNfcActivity extends BaseActivity {
         }
         // When entering a PIN, a status of 63CX indicates X attempts remaining.
         if ((status & (short)0xFFF0) == 0x63C0) {
-            Notify.create(this, getString(R.string.error_pin, status & 0x000F), Style.WARN).show();
+            onNfcError(getString(R.string.error_pin, status & 0x000F));
             return;
         }
 
@@ -160,63 +211,60 @@ public abstract class BaseNfcActivity extends BaseActivity {
             // These errors should not occur in everyday use; if they are returned, it means we
             // made a mistake sending data to the card, or the card is misbehaving.
             case 0x6A80: {
-                Notify.create(this, getString(R.string.error_nfc_bad_data), Style.WARN).show();
+                onNfcError(getString(R.string.error_nfc_bad_data));
                 break;
             }
             case 0x6883: {
-                Notify.create(this, getString(R.string.error_nfc_chaining_error), Style.WARN).show();
+                onNfcError(getString(R.string.error_nfc_chaining_error));
                 break;
             }
             case 0x6B00: {
-                Notify.create(this, getString(R.string.error_nfc_header, "P1/P2"), Style.WARN).show();
+                onNfcError(getString(R.string.error_nfc_header, "P1/P2"));
                 break;
             }
             case 0x6D00: {
-                Notify.create(this, getString(R.string.error_nfc_header, "INS"), Style.WARN).show();
+                onNfcError(getString(R.string.error_nfc_header, "INS"));
                 break;
             }
             case 0x6E00: {
-                Notify.create(this, getString(R.string.error_nfc_header, "CLA"), Style.WARN).show();
+                onNfcError(getString(R.string.error_nfc_header, "CLA"));
                 break;
             }
             // These error conditions are more likely to be experienced by an end user.
             case 0x6285: {
-                Notify.create(this, getString(R.string.error_nfc_terminated), Style.WARN).show();
+                onNfcError(getString(R.string.error_nfc_terminated));
                 break;
             }
             case 0x6700: {
-                Notify.create(this, getString(R.string.error_nfc_wrong_length), Style.WARN).show();
+                onNfcError(getString(R.string.error_nfc_wrong_length));
                 break;
             }
             case 0x6982: {
-                Notify.create(this, getString(R.string.error_nfc_security_not_satisfied),
+                onNfcError(getString(R.string.error_nfc_security_not_satisfied));
-                        Style.WARN).show();
                 break;
             }
             case 0x6983: {
-                Notify.create(this, getString(R.string.error_nfc_authentication_blocked),
+                onNfcError(getString(R.string.error_nfc_authentication_blocked));
-                        Style.WARN).show();
                 break;
             }
             case 0x6985: {
-                Notify.create(this, getString(R.string.error_nfc_conditions_not_satisfied),
+                onNfcError(getString(R.string.error_nfc_conditions_not_satisfied));
-                        Style.WARN).show();
                 break;
             }
             // 6A88 is "Not Found" in the spec, but Yubikey also returns 6A83 for this in some cases.
             case 0x6A88:
             case 0x6A83: {
-                Notify.create(this, getString(R.string.error_nfc_data_not_found), Style.WARN).show();
+                onNfcError(getString(R.string.error_nfc_data_not_found));
                 break;
             }
             // 6F00 is a JavaCard proprietary status code, SW_UNKNOWN, and usually represents an
             // unhandled exception on the smart card.
             case 0x6F00: {
-                Notify.create(this, getString(R.string.error_nfc_unknown), Style.WARN).show();
+                onNfcError(getString(R.string.error_nfc_unknown));
                 break;
             }
             default: {
-                Notify.create(this, getString(R.string.error_nfc, e.getMessage()), Style.WARN).show();
+                onNfcError(getString(R.string.error_nfc, e.getMessage()));
                 break;
             }
         }
@@ -356,39 +404,6 @@ public abstract class BaseNfcActivity extends BaseActivity {
 
     }
 
-    protected void onNfcPreExecute() {
-    }
-
-    protected void doNfcInBackground() throws IOException {
-        mNfcFingerprints = nfcGetFingerprints();
-        mNfcUserId = nfcGetUserId();
-        mNfcAid = nfcGetAid();
-    }
-
-    protected void onNfcPostExecute() throws IOException {
-
-        final long subKeyId = KeyFormattingUtils.getKeyIdFromFingerprint(mNfcFingerprints);
-
-        try {
-            CachedPublicKeyRing ring = new ProviderHelper(this).getCachedPublicKeyRing(
-                    KeyRings.buildUnifiedKeyRingsFindBySubkeyUri(subKeyId));
-            long masterKeyId = ring.getMasterKeyId();
-
-            Intent intent = new Intent(this, ViewKeyActivity.class);
-            intent.setData(KeyRings.buildGenericKeyRingUri(masterKeyId));
-            intent.putExtra(ViewKeyActivity.EXTRA_NFC_AID, mNfcAid);
-            intent.putExtra(ViewKeyActivity.EXTRA_NFC_USER_ID, mNfcUserId);
-            intent.putExtra(ViewKeyActivity.EXTRA_NFC_FINGERPRINTS, mNfcFingerprints);
-            startActivity(intent);
-        } catch (PgpKeyNotFoundException e) {
-            Intent intent = new Intent(this, CreateKeyActivity.class);
-            intent.putExtra(CreateKeyActivity.EXTRA_NFC_AID, mNfcAid);
-            intent.putExtra(CreateKeyActivity.EXTRA_NFC_USER_ID, mNfcUserId);
-            intent.putExtra(CreateKeyActivity.EXTRA_NFC_FINGERPRINTS, mNfcFingerprints);
-            startActivity(intent);
-        }
-    }
-
     /** Return the key id from application specific data stored on tag, or null
      * if it doesn't exist.
      *

OpenKeychain/src/main/res/layout/nfc_activity.xml

@@ -1,6 +1,7 @@
 <?xml version="1.0" encoding="utf-8"?>
 <LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
     xmlns:custom="http://schemas.android.com/apk/res-auto"
+    xmlns:tools="http://schemas.android.com/tools"
     android:layout_width="match_parent"
     android:layout_height="wrap_content">
 
@@ -17,7 +18,7 @@
         android:paddingLeft="24dp"
         android:paddingRight="24dp"
         android:paddingTop="16dp"
-        custom:initialView="0">
+        custom:initialView="3">
 
         <RelativeLayout
             android:layout_width="match_parent"
@@ -114,6 +115,44 @@
 
         </RelativeLayout>
 
+        <RelativeLayout
+            android:layout_width="match_parent"
+            android:layout_height="wrap_content">
+
+            <TextView
+                android:id="@+id/nfc_activity_text_placeholder"
+                android:layout_width="wrap_content"
+                android:layout_height="wrap_content"
+                android:text=""
+                android:textAppearance="@android:style/TextAppearance.Medium" />
+
+            <!-- placeholder to retain dialog size -->
+            <ImageView
+                android:id="@+id/nfc_image_placeholder3"
+                android:layout_width="wrap_content"
+                android:layout_height="wrap_content"
+                android:layout_alignParentLeft="true"
+                android:layout_alignParentStart="true"
+                android:layout_below="@+id/nfc_activity_text_placeholder"
+                android:layout_marginTop="8dp"
+                android:adjustViewBounds="true"
+                android:background="@android:color/transparent"
+                android:src="@drawable/yubikey_phone"
+                android:visibility="invisible" />
+
+            <TextView
+                android:id="@+id/nfc_activity_error_text"
+                android:layout_width="wrap_content"
+                android:layout_height="wrap_content"
+                android:layout_centerHorizontal="true"
+                android:layout_centerVertical="true"
+                android:textAppearance="@android:style/TextAppearance.Medium"
+                android:textColor="@color/android_red_dark"
+                tools:text="Error text" />
+
+
+        </RelativeLayout>
+
     </org.sufficientlysecure.keychain.ui.widget.ToolableViewAnimator>
 
 </LinearLayout>