[HOLD for payment 2023-02-20] [$2000] Android - Attachment- User seen frame when attaching picture #14448

kbecciv · 2023-01-20T22:47:38Z

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

Action Performed:

Launch the app
Log i with expensifail account
Go to any chat
Tap on plus button
Select Add Attachment
Select Take a photo
Take a photo and send it

Expected Result:

Picture should be displayed in full size

Actual Result:

User seen frame when attaching picture

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

Android / native

Version Number: 1.2.57.2

Reproducible in staging?: Yes

Reproducible in production?: Yes

If this was caught during regression testing, add the test name, ID and link from TestRail:

Email or phone of affected tester (no customers):

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation

Screen_Recording_20230120_154219_New.Expensify.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~01ddb836bc6b1f8022
Upwork Job ID: 1618698186309328896
Last Price Increase: 2023-02-08

dylanexpensify · 2023-01-26T19:32:53Z

reviewing now

dylanexpensify · 2023-01-26T19:50:41Z

Using browserstack I believe I could create (hard to take photo though on the app)

melvin-bot · 2023-01-26T19:51:38Z

Job added to Upwork: https://www.upwork.com/jobs/~01ddb836bc6b1f8022

melvin-bot · 2023-01-26T19:51:41Z

Current assignee @dylanexpensify is eligible for the External assigner, not assigning anyone new.

melvin-bot · 2023-01-26T19:51:48Z

Triggered auto assignment to Contributor-plus team member for initial proposal review - @thesahindia (External)

melvin-bot · 2023-01-26T19:51:53Z

Triggered auto assignment to @mountiny (External), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

alexxxwork · 2023-01-29T04:15:42Z

RCA:

the issue comes from wrong image size calculation in

App/src/components/ThumbnailImage.js

Line 55 in 142fbba

calculateThumbnailImageSize(width, height) {

Vertical images doesn't get rotated on Android phones - they are still horizontal images with exif orientation flag set. So RNImage.getSize returns width > height for vertical images and doesn't know anything about rotation

Proposal:

We could:

Use react-native-exif to get image orientation from thumbnail
It could be a backend problem - to rotate images with rotation flag in exif and make them 'ordinary' - this is the most right way to do this imho.

eh2077 · 2023-01-29T04:56:42Z

Proposal

RCA

This issue comes from react-native-fast-image library.
In android platform we can read rotation information of image through ExifInterface otherwise the image width and height maybe exchanged. We can fix this by improving the patch patches/react-native-fast-image+8.6.3.patch by

changing the input type of BitmapSizeDecoder from InputSteam to File
adding RequestOptions to let Glide skipMemoryCache

FastImageViewWithUrl.java

requestManager
                .as(Size.class)
                .apply(new RequestOptions()
                        .skipMemoryCache(true)
                        .diskCacheStrategy(DiskCacheStrategy.DATA))
                .load(imageSource == null ? null : imageSource.getSourceForLoad())
                .into(new SimpleTarget<Size>() {
                    @Override
                    public void onResourceReady(@NonNull Size resource, @Nullable Transition<? super Size> transition) {
                        WritableMap resourceData = new WritableNativeMap();
                        resourceData.putInt("width", resource.width);
                        resourceData.putInt("height", resource.height);
                        eventEmitter.receiveEvent(viewId,
                            "onFastImageLoad",
                            resourceData
                        );
                    }
                });

BitmapSizeDecoder.java

public Resource<BitmapFactory.Options> decode(@NonNull File source, int width, int height, @NonNull Options options) throws IOException {
        BitmapFactory.Options bitmapOptions = new BitmapFactory.Options();
        bitmapOptions.inJustDecodeBounds = true;
        BitmapFactory.decodeFile(source.getAbsolutePath(), bitmapOptions);
        int orientation = Integer.parseInt(new ExifInterface(source.getAbsolutePath()).getAttribute(ExifInterface.TAG_ORIENTATION));
        if (orientation == ExifInterface.ORIENTATION_ROTATE_90 || orientation == ExifInterface.ORIENTATION_ROTATE_270) {
            int outHeight = bitmapOptions.outHeight;
            int outWidth = bitmapOptions.outWidth;
            bitmapOptions.outHeight = outWidth;
            bitmapOptions.outWidth = outHeight;
        }
        return new SimpleResource(bitmapOptions);
    }

If you want to verify the solution, you can just apply the following diff to patches/react-native-fast-image+8.6.3.patch and rebuild the Android App

Click me

diff --git a/patches/react-native-fast-image+8.6.3.patch b/patches/react-native-fast-image+8.6.3.patch
index fc7e59c17c..b07bbc1d74 100644
--- a/patches/react-native-fast-image+8.6.3.patch
+++ b/patches/react-native-fast-image+8.6.3.patch
@@ -1,12 +1,13 @@
 diff --git a/node_modules/react-native-fast-image/android/src/main/java/com/dylanvann/fastimage/BitmapSizeDecoder.java b/node_modules/react-native-fast-image/android/src/main/java/com/dylanvann/fastimage/BitmapSizeDecoder.java
 new file mode 100644
-index 0000000..03ad017
+index 0000000..5614e4f
 --- /dev/null
 +++ b/node_modules/react-native-fast-image/android/src/main/java/com/dylanvann/fastimage/BitmapSizeDecoder.java
-@@ -0,0 +1,31 @@
+@@ -0,0 +1,39 @@
 +package com.dylanvann.fastimage;
 +
 +import android.graphics.BitmapFactory;
++import android.media.ExifInterface;
 +
 +import androidx.annotation.NonNull;
 +import androidx.annotation.Nullable;
@@ -16,22 +17,29 @@ index 0000000..03ad017
 +import com.bumptech.glide.load.engine.Resource;
 +import com.bumptech.glide.load.resource.SimpleResource;
 +
++import java.io.File;
 +import java.io.IOException;
-+import java.io.InputStream;
 +
-+public class BitmapSizeDecoder implements ResourceDecoder<InputStream, BitmapFactory.Options> {
++public class BitmapSizeDecoder implements ResourceDecoder<File, BitmapFactory.Options> {
 +
 +    @Override
-+    public boolean handles(@NonNull InputStream source, @NonNull Options options) throws IOException {
++    public boolean handles(@NonNull File source, @NonNull Options options) throws IOException {
 +        return true;
 +    }
 +
 +    @Nullable
 +    @Override
-+    public Resource<BitmapFactory.Options> decode(@NonNull InputStream source, int width, int height, @NonNull Options options) throws IOException {
++    public Resource<BitmapFactory.Options> decode(@NonNull File source, int width, int height, @NonNull Options options) throws IOException {
 +        BitmapFactory.Options bitmapOptions = new BitmapFactory.Options();
 +        bitmapOptions.inJustDecodeBounds = true;
-+        BitmapFactory.decodeStream(source, null, bitmapOptions);
++        BitmapFactory.decodeFile(source.getAbsolutePath(), bitmapOptions);
++        int orientation = Integer.parseInt(new ExifInterface(source.getAbsolutePath()).getAttribute(ExifInterface.TAG_ORIENTATION));
++        if (orientation == ExifInterface.ORIENTATION_ROTATE_90 || orientation == ExifInterface.ORIENTATION_ROTATE_270) {
++            int outHeight = bitmapOptions.outHeight;
++            int outWidth = bitmapOptions.outWidth;
++            bitmapOptions.outHeight = outWidth;
++            bitmapOptions.outWidth = outHeight;
++        }
 +        return new SimpleResource(bitmapOptions);
 +    }
 +}
@@ -67,7 +75,7 @@ index 0000000..7d208d1
 +}
 \ No newline at end of file
 diff --git a/node_modules/react-native-fast-image/android/src/main/java/com/dylanvann/fastimage/FastImageOkHttpProgressGlideModule.java b/node_modules/react-native-fast-image/android/src/main/java/com/dylanvann/fastimage/FastImageOkHttpProgressGlideModule.java
-index 811292a..f60b87c 100644
+index 811292a..dcfb413 100644
 --- a/node_modules/react-native-fast-image/android/src/main/java/com/dylanvann/fastimage/FastImageOkHttpProgressGlideModule.java
 +++ b/node_modules/react-native-fast-image/android/src/main/java/com/dylanvann/fastimage/FastImageOkHttpProgressGlideModule.java
 @@ -2,6 +2,7 @@ package com.dylanvann.fastimage;
@@ -78,12 +86,20 @@ index 811292a..f60b87c 100644
  
  import com.bumptech.glide.Glide;
  import com.bumptech.glide.Registry;
-@@ -47,6 +48,9 @@ public class FastImageOkHttpProgressGlideModule extends LibraryGlideModule {
+@@ -11,6 +12,7 @@ import com.bumptech.glide.load.model.GlideUrl;
+ import com.bumptech.glide.module.LibraryGlideModule;
+ import com.facebook.react.modules.network.OkHttpClientProvider;
+ 
++import java.io.File;
+ import java.io.IOException;
+ import java.io.InputStream;
+ import java.util.HashMap;
+@@ -47,6 +49,9 @@ public class FastImageOkHttpProgressGlideModule extends LibraryGlideModule {
                  .build();
          OkHttpUrlLoader.Factory factory = new OkHttpUrlLoader.Factory(client);
          registry.replace(GlideUrl.class, InputStream.class, factory);
 +        // Decoder + Transcoder pair for InputStream -> Size
-+        registry.prepend(InputStream.class, BitmapFactory.Options.class, new BitmapSizeDecoder());
++        registry.prepend(File.class, BitmapFactory.Options.class, new BitmapSizeDecoder());
 +        registry.register(BitmapFactory.Options.class, Size.class, new BitmapSizeTranscoder());
      }
  
@@ -115,7 +131,7 @@ index dbeb813..bf8f21c 100644
          return false;
      }
 diff --git a/node_modules/react-native-fast-image/android/src/main/java/com/dylanvann/fastimage/FastImageViewWithUrl.java b/node_modules/react-native-fast-image/android/src/main/java/com/dylanvann/fastimage/FastImageViewWithUrl.java
-index 34fcf89..1339f5c 100644
+index 34fcf89..8f6e741 100644
 --- a/node_modules/react-native-fast-image/android/src/main/java/com/dylanvann/fastimage/FastImageViewWithUrl.java
 +++ b/node_modules/react-native-fast-image/android/src/main/java/com/dylanvann/fastimage/FastImageViewWithUrl.java
 @@ -2,6 +2,7 @@ package com.dylanvann.fastimage;
@@ -126,7 +142,7 @@ index 34fcf89..1339f5c 100644
  import android.annotation.SuppressLint;
  import android.content.Context;
  import android.graphics.drawable.Drawable;
-@@ -9,16 +10,24 @@ import android.graphics.drawable.Drawable;
+@@ -9,16 +10,26 @@ import android.graphics.drawable.Drawable;
  import androidx.annotation.Nullable;
  import androidx.appcompat.widget.AppCompatImageView;
  
@@ -134,10 +150,12 @@ index 34fcf89..1339f5c 100644
  import com.bumptech.glide.RequestBuilder;
  import com.bumptech.glide.RequestManager;
 +import com.bumptech.glide.load.DataSource;
++import com.bumptech.glide.load.engine.DiskCacheStrategy;
 +import com.bumptech.glide.load.engine.GlideException;
  import com.bumptech.glide.load.model.GlideUrl;
  import com.bumptech.glide.request.Request;
 +import com.bumptech.glide.request.RequestListener;
++import com.bumptech.glide.request.RequestOptions;
 +import com.bumptech.glide.request.target.SimpleTarget;
 +import com.bumptech.glide.request.target.Target;
 +import com.bumptech.glide.request.transition.Transition;
@@ -151,7 +169,7 @@ index 34fcf89..1339f5c 100644
  import java.util.ArrayList;
  import java.util.Collections;
  import java.util.List;
-@@ -124,9 +133,34 @@ class FastImageViewWithUrl extends AppCompatImageView {
+@@ -124,9 +135,34 @@ class FastImageViewWithUrl extends AppCompatImageView {
              RCTEventEmitter eventEmitter = context.getJSModule(RCTEventEmitter.class);
              int viewId = this.getId();
  
@@ -189,7 +207,7 @@ index 34fcf89..1339f5c 100644
          }
  
          if (requestManager != null) {
-@@ -148,6 +182,25 @@ class FastImageViewWithUrl extends AppCompatImageView {
+@@ -148,6 +184,28 @@ class FastImageViewWithUrl extends AppCompatImageView {
                  builder.listener(new FastImageRequestListener(key));
  
              builder.into(this);
@@ -199,7 +217,10 @@ index 34fcf89..1339f5c 100644
 +            int viewId = this.getId();
 +            requestManager
 +                .as(Size.class)
-+                .load(imageSource == null ? null : imageSource.getSourceForLoad())
++                .apply(new RequestOptions()
++                        .skipMemoryCache(true)
++                        .diskCacheStrategy(DiskCacheStrategy.DATA))
++                .load(imageSource == null ? null : imageSource.getUri())
 +                .into(new SimpleTarget<Size>() {
 +                    @Override
 +                    public void onResourceReady(@NonNull Size resource, @Nullable Transition<? super Size> transition) {

alexxxwork · 2023-01-29T14:17:57Z

changing the input type of BitmapSizeDecoder from InputSteam to File

What's the point of using File instead of input stream? ExifInterface works with InputStream as well: https://android.googlesource.com/platform/frameworks/base/+/master/media/java/android/media/ExifInterface.java#1590

eh2077 · 2023-01-30T02:23:29Z

changing the input type of BitmapSizeDecoder from InputSteam to File

What's the point of using File instead of input stream? ExifInterface works with InputStream as well: https://android.googlesource.com/platform/frameworks/base/+/master/media/java/android/media/ExifInterface.java#1590

Hey @alexxxwork , thanks for looking into it.

It's mainly because the File constructor API has wider sdk version support than the newer InputStream constructor. You can check the API level and the minSdkVersion property defined in build.gradle file of related projects. You can also get hints from an IDE, like Android Studio.

mountiny · 2023-01-30T09:11:18Z

Not overdue, waiting to review proposals cc @thesahindia

thesahindia · 2023-01-30T09:23:44Z

I am not familiar with the code here, so I think we should assign a new C+.

0xmiros · 2023-01-30T10:01:16Z

I can review proposals today

melvin-bot · 2023-01-30T10:05:40Z

📣 @0xmiroslav You have been assigned to this job by @mountiny!
Please apply to this job in Upwork and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

ShekovNikita · 2023-01-30T14:29:54Z

If XML is used, then one or two lines in the View is sufficient:
scaleType="select the desired type"
adjustViewBounds="true"

dylanexpensify · 2023-02-22T08:07:21Z

working on this today!

dylanexpensify · 2023-02-22T09:56:10Z

@alexxxwork test steps look great, thank you!

dylanexpensify · 2023-02-22T14:54:56Z

posted rt proposal. @mountiny mind reviewing? https://expensify.slack.com/archives/C01SKUP7QR0/p1677069041526679

dylanexpensify · 2023-02-23T09:41:29Z

Alright, we're good here on RT steps

dylanexpensify · 2023-02-23T09:44:34Z

@0xmiroslav can you apply to the job please?
@alexxxwork sent offer!

alexxxwork · 2023-02-23T09:49:44Z

@alexxxwork sent offer!

@dylanexpensify thank you. Does this issue eligible 50% bonus for merging whithin 3 business days?

Merged PR within 3 business days of assignment - 50% bonus

dylanexpensify · 2023-02-23T09:50:27Z

Yes it will!

dylanexpensify · 2023-02-23T09:50:56Z

I'll add the bonus during the payout 😄

dylanexpensify · 2023-02-23T09:53:12Z

Payment sent, contract closed @alexxxwork - thanks for your help!

0xmiros · 2023-02-23T10:10:56Z

I think proposals between them can be considered not similar and @alexxxwork provided more simple and complete solution. That being said, I suggest @alexxxwork to raise PR once I confirm and give 👍 after testing various cases on my side. So distribution will be something like this: 1k for each, and 1k more to @alexxxwork if meets bonus timeline @eh2077 do you think it's fair?

@dylanexpensify 👆

dylanexpensify · 2023-02-23T10:13:10Z

@0xmiroslav ah, I missed that comment. Can you confirm if we agreed internally on that course of action?

0xmiros · 2023-02-23T10:13:53Z

I think @mountiny agreed here

dylanexpensify · 2023-02-23T10:16:09Z

Ok, so it seems $2K (inclusive of bonus) to @alexxxwork then? And $1K to @eh2077?

0xmiros · 2023-02-23T10:16:40Z

yes, @mountiny can you please confirm?

mountiny · 2023-02-23T10:34:50Z

Yep, agreed, lets do that!

alexxxwork · 2023-02-23T11:01:19Z

Ok, so it seems $2K (inclusive of bonus) to @alexxxwork then? And $1K to @eh2077?

@dylanexpensify oh, sorry. Didn't get it that this agreement already included time bonus. Should we rollback upwork transaction now?

dylanexpensify · 2023-02-23T11:11:52Z

@alexxxwork no worries, this is on me! I'm going to request a refund from you of $1K!

Thanks for confirming @mountiny !

dylanexpensify · 2023-02-23T11:24:18Z

@alexxxwork sent refund request!
@0xmiroslav sent offer!
@eh2077 can you please apply?

alexxxwork · 2023-02-23T11:38:57Z

@alexxxwork sent refund request!
@0xmiroslav sent offer!
@eh2077 can you please apply?

@dylanexpensify accepted 🙌

eh2077 · 2023-02-23T12:30:28Z

@alexxxwork sent refund request!

@0xmiroslav sent offer!

@eh2077 can you please apply?

Hi @dylanexpensify, I have applied it, thanks!

dylanexpensify · 2023-02-23T13:28:19Z

@eh2077 offer sent!
@0xmiroslav payment sent, contract ended!

dylanexpensify · 2023-02-23T13:30:55Z

all payments now done! thanks everyone!

eh2077 · 2023-02-23T13:31:12Z

@eh2077 offer sent!

@0xmiroslav payment sent, contract ended!

@dylanexpensify accepted the offer, thanks!

dylanexpensify · 2023-02-23T14:03:55Z

payment already sent @eh2077 ! 🙌

kbecciv added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jan 20, 2023

melvin-bot bot assigned dylanexpensify Jan 20, 2023

melvin-bot bot locked and limited conversation to collaborators Jan 20, 2023

dylanexpensify added the External Added to denote the issue can be worked on by a contributor label Jan 26, 2023

melvin-bot bot unlocked this conversation Jan 26, 2023

melvin-bot bot changed the title ~~Android - Attachment- User seen frame when attaching picture~~ [$1000] Android - Attachment- User seen frame when attaching picture Jan 26, 2023

melvin-bot bot assigned thesahindia Jan 26, 2023

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jan 26, 2023

melvin-bot bot assigned mountiny Jan 26, 2023

melvin-bot bot added the Overdue label Jan 29, 2023

melvin-bot bot removed the Overdue label Jan 30, 2023

mountiny assigned 0xmiros and unassigned thesahindia Jan 30, 2023

melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Jan 30, 2023

melvin-bot bot added Daily KSv2 Overdue and removed Weekly KSv2 labels Feb 19, 2023

melvin-bot bot removed the Overdue label Feb 22, 2023

dylanexpensify closed this as completed Feb 23, 2023

situchan mentioned this issue Apr 6, 2023

[HOLD for payment 2023-04-24] [$2000] Connect Bank Account - Plaid triggered again after several clicks on selector page in offline mode #15861

Closed

6 tasks

[HOLD for payment 2023-02-20] [$2000] Android - Attachment- User seen frame when attaching picture #14448

[HOLD for payment 2023-02-20] [$2000] Android - Attachment- User seen frame when attaching picture #14448

Comments

kbecciv commented Jan 20, 2023 • edited by melvin-bot bot Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

dylanexpensify commented Jan 26, 2023

dylanexpensify commented Jan 26, 2023

melvin-bot bot commented Jan 26, 2023

melvin-bot bot commented Jan 26, 2023

melvin-bot bot commented Jan 26, 2023

melvin-bot bot commented Jan 26, 2023

alexxxwork commented Jan 29, 2023 • edited Loading

RCA:

Proposal:

eh2077 commented Jan 29, 2023 • edited Loading

alexxxwork commented Jan 29, 2023 • edited Loading

eh2077 commented Jan 30, 2023

mountiny commented Jan 30, 2023

thesahindia commented Jan 30, 2023

0xmiros commented Jan 30, 2023

melvin-bot bot commented Jan 30, 2023

ShekovNikita commented Jan 30, 2023

dylanexpensify commented Feb 22, 2023

dylanexpensify commented Feb 22, 2023

dylanexpensify commented Feb 22, 2023

dylanexpensify commented Feb 23, 2023

dylanexpensify commented Feb 23, 2023

alexxxwork commented Feb 23, 2023

dylanexpensify commented Feb 23, 2023

dylanexpensify commented Feb 23, 2023

dylanexpensify commented Feb 23, 2023

0xmiros commented Feb 23, 2023

dylanexpensify commented Feb 23, 2023

0xmiros commented Feb 23, 2023

dylanexpensify commented Feb 23, 2023

0xmiros commented Feb 23, 2023

mountiny commented Feb 23, 2023

alexxxwork commented Feb 23, 2023

dylanexpensify commented Feb 23, 2023

dylanexpensify commented Feb 23, 2023

alexxxwork commented Feb 23, 2023

eh2077 commented Feb 23, 2023

dylanexpensify commented Feb 23, 2023

dylanexpensify commented Feb 23, 2023

eh2077 commented Feb 23, 2023

dylanexpensify commented Feb 23, 2023

kbecciv commented Jan 20, 2023 •

edited by melvin-bot bot

Loading

alexxxwork commented Jan 29, 2023 •

edited

Loading

eh2077 commented Jan 29, 2023 •

edited

Loading

alexxxwork commented Jan 29, 2023 •

edited

Loading