Fix Crash on Previewing Existing Image

* Fix #5856 - Image Preview Crash When loading an existing image, we performed the image load before we had created all elements of the UI. This caused issues. We fix this by splitting UI creation and initialisation logic * Fix #5856 - Fix API Errors createUI has `@SuppressLint("NewApi")` due to camera permissioning, but it masked a bug in calling setBackground with lower API levels * Added Unit Tests Very basic right now, but there's a lot of value in ensuring that a screen can be created with no errors.
ankidroid · Mar 24, 2020 · a9c689b · a9c689b
1 parent a2f8c09
commit a9c689b
Show file tree

Hide file tree

Showing 4 changed files with 144 additions and 29 deletions.
diff --git a/...oid/src/main/java/com/ichi2/anki/multimediacard/activity/MultimediaEditFieldActivity.java b/...oid/src/main/java/com/ichi2/anki/multimediacard/activity/MultimediaEditFieldActivity.java
@@ -25,6 +25,7 @@
 import android.os.Bundle;
 
 import androidx.annotation.NonNull;
+import androidx.annotation.VisibleForTesting;
 import androidx.core.app.ActivityCompat;
 import androidx.appcompat.widget.Toolbar;
 import android.view.Menu;
@@ -506,4 +507,9 @@ private static void onRequiredPermissionDenied(ChangeUIRequest request, Multimed
             }
         }
     }
+
+    @VisibleForTesting
+    IFieldController getFieldController() {
+        return mFieldController;
+    }
 }
diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/multimediacard/fields/BasicImageFieldController.java b/AnkiDroid/src/main/java/com/ichi2/anki/multimediacard/fields/BasicImageFieldController.java
@@ -89,39 +89,12 @@ private int getMaxImageSize() {
     @SuppressLint( {"UnsupportedChromeOsCameraSystemFeature", "NewApi"})
     @Override
     public void createUI(Context context, LinearLayout layout) {
-        mImagePreview = new ImageView(mActivity);
-
-        DisplayMetrics metrics = getDisplayMetrics();
-
-        int height = metrics.heightPixels;
-        int width = metrics.widthPixels;
-
         LinearLayout.LayoutParams p = new LayoutParams(ViewGroup.LayoutParams.MATCH_PARENT,
                 android.view.ViewGroup.LayoutParams.WRAP_CONTENT);
-        setPreviewImage(mField.getImagePath(), getMaxImageSize());
-        mImagePreview.setScaleType(ImageView.ScaleType.CENTER_INSIDE);
-        mImagePreview.setAdjustViewBounds(true);
 
-        mImagePreview.setMaxHeight((int) Math.round(height * 0.4));
-        mImagePreview.setMaxWidth((int) Math.round(width * 0.6));
+        drawUIComponents(context);
 
-        mImageFileSize = new EditText(context);
-        mImageFileSize.setMaxWidth((int) Math.round(width * 0.6));
-        mImageFileSize.setEnabled(false);
-        mImageFileSize.setGravity(Gravity.CENTER_HORIZONTAL);
-        mImageFileSize.setBackground(null);
-        mImageFileSize.setVisibility(View.GONE);
-
-        //#5513 - Image compression failed, but we'll confuse most users if we tell them that. Instead, just imply that
-        //there's an action that they can take.
-        mImageFileSizeWarning = new EditText(context);
-        mImageFileSizeWarning.setMaxWidth((int) Math.round(width * 0.6));
-        mImageFileSizeWarning.setEnabled(false);
-        mImageFileSizeWarning.setTextColor(Color.parseColor("#FF4500")); //Orange-Red
-        mImageFileSizeWarning.setGravity(Gravity.CENTER_HORIZONTAL);
-        mImageFileSizeWarning.setVisibility(View.GONE);
-        mImageFileSizeWarning.setBackground(null);
-        mImageFileSizeWarning.setText(R.string.multimedia_editor_image_compression_failed);
+        setPreviewImage(mField.getImagePath(), getMaxImageSize());
 
         Button mBtnGallery = new Button(mActivity);
         mBtnGallery.setText(gtxt(R.string.multimedia_editor_image_field_editing_galery));
@@ -186,6 +159,46 @@ public void createUI(Context context, LinearLayout layout) {
     }
 
 
+    @SuppressLint("NewApi") //Conditionally called anything which requires API 16+.
+    private void drawUIComponents(Context context) {
+        mImagePreview = new ImageView(mActivity);
+
+        DisplayMetrics metrics = getDisplayMetrics();
+
+        int height = metrics.heightPixels;
+        int width = metrics.widthPixels;
+
+
+        mImagePreview.setScaleType(ImageView.ScaleType.CENTER_INSIDE);
+        mImagePreview.setAdjustViewBounds(true);
+
+        mImagePreview.setMaxHeight((int) Math.round(height * 0.4));
+        mImagePreview.setMaxWidth((int) Math.round(width * 0.6));
+
+        mImageFileSize = new EditText(context);
+        mImageFileSize.setMaxWidth((int) Math.round(width * 0.6));
+        mImageFileSize.setEnabled(false);
+        mImageFileSize.setGravity(Gravity.CENTER_HORIZONTAL);
+        if (CompatHelper.getSdkVersion() >= Build.VERSION_CODES.JELLY_BEAN) {
+            mImageFileSize.setBackground(null);
+        }
+        mImageFileSize.setVisibility(View.GONE);
+
+        //#5513 - Image compression failed, but we'll confuse most users if we tell them that. Instead, just imply that
+        //there's an action that they can take.
+        mImageFileSizeWarning = new EditText(context);
+        mImageFileSizeWarning.setMaxWidth((int) Math.round(width * 0.6));
+        mImageFileSizeWarning.setEnabled(false);
+        mImageFileSizeWarning.setTextColor(Color.parseColor("#FF4500")); //Orange-Red
+        mImageFileSizeWarning.setGravity(Gravity.CENTER_HORIZONTAL);
+        mImageFileSizeWarning.setVisibility(View.GONE);
+        if (CompatHelper.getSdkVersion() >= Build.VERSION_CODES.JELLY_BEAN) {
+            mImageFileSize.setBackground(null);
+        }
+        mImageFileSizeWarning.setText(R.string.multimedia_editor_image_compression_failed);
+    }
+
+
     private String gtxt(int id) {
         return mActivity.getText(id).toString();
     }

diff --git a/...test/java/com/ichi2/anki/multimediacard/activity/MultimediaEditFieldActivityTestBase.java b/...test/java/com/ichi2/anki/multimediacard/activity/MultimediaEditFieldActivityTestBase.java
@@ -0,0 +1,53 @@
+package com.ichi2.anki.multimediacard.activity;
+
+import android.Manifest;
+import android.app.Application;
+import android.content.Intent;
+
+import com.ichi2.anki.RobolectricTest;
+import com.ichi2.anki.multimediacard.IMultimediaEditableNote;
+import com.ichi2.anki.multimediacard.fields.IField;
+import com.ichi2.anki.multimediacard.fields.IFieldController;
+import com.ichi2.anki.multimediacard.impl.MultimediaEditableNote;
+
+import org.robolectric.Robolectric;
+import org.robolectric.Shadows;
+import org.robolectric.android.controller.ActivityController;
+import org.robolectric.shadows.ShadowApplication;
+
+import androidx.test.core.app.ApplicationProvider;
+
+public abstract class MultimediaEditFieldActivityTestBase extends RobolectricTest {
+
+    protected void grantCameraPermission() {
+        Application application = ApplicationProvider.getApplicationContext();
+        ShadowApplication app = Shadows.shadowOf(application);
+        app.grantPermissions(Manifest.permission.CAMERA);
+    }
+
+    protected IFieldController getControllerForField(IField field, IMultimediaEditableNote note, int fieldIndex) {
+        Intent intent = new Intent(Intent.ACTION_VIEW);
+        intent.putExtra(MultimediaEditFieldActivity.EXTRA_FIELD_INDEX, fieldIndex);
+        intent.putExtra(MultimediaEditFieldActivity.EXTRA_FIELD, field);
+        intent.putExtra(MultimediaEditFieldActivity.EXTRA_WHOLE_NOTE, note);
+        return getControllerForIntent(intent);
+    }
+
+    protected IMultimediaEditableNote getEmptyNote() {
+        MultimediaEditableNote note = new MultimediaEditableNote();
+        note.setNumFields(1);
+        return note;
+    }
+
+    protected IFieldController getControllerForIntent(Intent intent) {
+
+
+        ActivityController multimediaController = Robolectric.buildActivity(MultimediaEditFieldActivity.class, intent)
+                .create().start().resume().visible();
+        MultimediaEditFieldActivity testCardTemplatePreviewer = (MultimediaEditFieldActivity) multimediaController.get();
+
+
+
+        return testCardTemplatePreviewer.getFieldController();
+    }
+}
diff --git a/...oid/src/test/java/com/ichi2/anki/multimediacard/fields/BasicImageFieldControllerTest.java b/...oid/src/test/java/com/ichi2/anki/multimediacard/fields/BasicImageFieldControllerTest.java
@@ -0,0 +1,43 @@
+package com.ichi2.anki.multimediacard.fields;
+
+import com.ichi2.anki.multimediacard.activity.MultimediaEditFieldActivityTestBase;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.robolectric.RobolectricTestRunner;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.instanceOf;
+
+
+@RunWith(RobolectricTestRunner.class)
+public class BasicImageFieldControllerTest extends MultimediaEditFieldActivityTestBase {
+
+    @Test
+    public void constructionWithoutDataGivesNoError() {
+        grantCameraPermission();
+
+        IFieldController controller = getControllerForField(emptyImageField(), getEmptyNote(), 0);
+
+        assertThat(controller, instanceOf(BasicImageFieldController.class));
+    }
+
+    @Test
+    public void constructionWithDataSucceeds() {
+        grantCameraPermission();
+
+        IFieldController controller = getControllerForField(imageFieldWithData(), getEmptyNote(), 0);
+
+        assertThat(controller, instanceOf(BasicImageFieldController.class));
+    }
+
+    private static IField emptyImageField() {
+        return new ImageField();
+    }
+
+    private static IField imageFieldWithData() {
+        IField field = emptyImageField();
+        field.setImagePath("test");
+        return field;
+    }
+}