-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[Issue/20295] Location metadata in uploaded pictures #20955
[Issue/20295] Location metadata in uploaded pictures #20955
Conversation
Generated by 🚫 Danger |
📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.
|
📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome work @pantstamp 🏅
I've tested the implementation on a Pixel 8 / Android 14 and the exif data were saved as expected. The code also looks good 🎉
Before | After |
---|---|
I also checked the failed Sonar report and I don't see any blocking issue. The code duplication refers to a small 3 line if statement (which I don't see value on extracting) and the security issue refers to the addition of the ACCESS_MEDIA_LOCATION
permission which is expected.
…ictures # Conflicts: # build.gradle
Quality Gate failedFailed conditions |
Found 1 violations: The PR caused some dependency changes (expand to see details)
-+--- org.wordpress:fluxc:{strictly 2.83.0} -> 2.83.0
-| +--- org.wordpress:wellsql:2.0.0
-| | +--- androidx.annotation:annotation:1.2.0 -> 1.6.0 (*)
-| | \--- org.wordpress.wellsql:wellsql-annotations:2.0.0
-| +--- org.wordpress.fluxc:fluxc-annotations:2.83.0
-| +--- org.greenrobot:eventbus:3.3.1
-| | \--- org.greenrobot:eventbus-java:3.3.1
-| +--- com.squareup.okhttp3:okhttp:4.9.0 -> 4.12.0 (*)
-| +--- com.android.volley:volley:1.1.1 -> 1.2.1
-| +--- androidx.paging:paging-runtime:2.1.2
-| | +--- androidx.paging:paging-common:2.1.2
-| | | +--- androidx.annotation:annotation:1.0.0 -> 1.6.0 (*)
-| | | \--- androidx.arch.core:core-common:2.0.0 -> 2.2.0 (*)
-| | +--- androidx.arch.core:core-runtime:2.0.0 -> 2.2.0 (*)
-| | +--- androidx.lifecycle:lifecycle-runtime:2.0.0 -> 2.6.2 (*)
-| | +--- androidx.lifecycle:lifecycle-livedata:2.0.0 -> 2.6.2 (*)
-| | \--- androidx.recyclerview:recyclerview:1.0.0 -> 1.3.0 (*)
-| +--- com.goterl:lazysodium-android:5.0.2
-| +--- net.java.dev.jna:jna:5.5.0
-| +--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.6.20 -> 1.9.10 (*)
-| +--- org.jetbrains.kotlin:kotlin-android-extensions-runtime:1.6.20 -> 1.9.22 (*)
-| +--- androidx.appcompat:appcompat:1.0.2 -> 1.6.1 (*)
-| +--- androidx.recyclerview:recyclerview:1.0.0 -> 1.3.0 (*)
-| +--- androidx.exifinterface:exifinterface:1.0.0 -> 1.3.6 (*)
-| +--- androidx.security:security-crypto:1.0.0
-| | +--- androidx.annotation:annotation:1.1.0 -> 1.6.0 (*)
-| | \--- com.google.crypto.tink:tink-android:1.5.0
-| +--- com.squareup.okhttp3:okhttp-urlconnection:4.9.0 -> 4.9.2 (*)
-| +--- com.google.code.gson:gson:2.8.5 -> 2.10.1
-| +--- org.apache.commons:commons-text:1.10.0
-| | \--- org.apache.commons:commons-lang3:3.12.0
-| +--- androidx.room:room-runtime:2.4.2 -> 2.5.0
-| | +--- androidx.annotation:annotation-experimental:1.1.0 -> 1.3.1 (*)
-| | +--- androidx.arch.core:core-runtime:2.0.1 -> 2.2.0 (*)
-| | +--- androidx.room:room-common:2.5.0
-| | | +--- androidx.annotation:annotation:1.3.0 -> 1.6.0 (*)
-| | | \--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.20 -> 1.9.10 (*)
-| | +--- androidx.sqlite:sqlite:2.3.0
-| | | +--- androidx.annotation:annotation:1.0.0 -> 1.6.0 (*)
-| | | \--- org.jetbrains.kotlin:kotlin-stdlib:1.7.20 -> 1.9.22 (*)
-| | \--- androidx.sqlite:sqlite-framework:2.3.0
-| | +--- androidx.annotation:annotation:1.2.0 -> 1.6.0 (*)
-| | +--- androidx.sqlite:sqlite:2.3.0 (*)
-| | \--- org.jetbrains.kotlin:kotlin-stdlib:1.7.20 -> 1.9.22 (*)
-| +--- androidx.room:room-ktx:2.4.2 -> 2.5.0
-| | +--- androidx.room:room-common:2.5.0 (*)
-| | +--- androidx.room:room-runtime:2.5.0 (*)
-| | +--- org.jetbrains.kotlin:kotlin-stdlib:1.7.20 -> 1.9.22 (*)
-| | \--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.6.4 -> 1.7.3 (*)
-| +--- com.google.dagger:dagger:2.42 -> 2.50
-| | \--- javax.inject:javax.inject:1
-| +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.3.9 -> 1.7.3 (*)
-| \--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.3.9 -> 1.7.3 (*)
++--- org.wordpress:fluxc:{strictly trunk-f111f9d00446395f16550bca872c6098c23529e9} -> trunk-f111f9d00446395f16550bca872c6098c23529e9
+| +--- org.wordpress:wellsql:2.0.0
+| | +--- androidx.annotation:annotation:1.2.0 -> 1.6.0 (*)
+| | \--- org.wordpress.wellsql:wellsql-annotations:2.0.0
+| +--- org.wordpress.fluxc:fluxc-annotations:trunk-f111f9d00446395f16550bca872c6098c23529e9
+| +--- org.greenrobot:eventbus:3.3.1
+| | \--- org.greenrobot:eventbus-java:3.3.1
+| +--- com.squareup.okhttp3:okhttp:4.9.0 -> 4.12.0 (*)
+| +--- com.android.volley:volley:1.1.1 -> 1.2.1
+| +--- androidx.paging:paging-runtime:2.1.2
+| | +--- androidx.paging:paging-common:2.1.2
+| | | +--- androidx.annotation:annotation:1.0.0 -> 1.6.0 (*)
+| | | \--- androidx.arch.core:core-common:2.0.0 -> 2.2.0 (*)
+| | +--- androidx.arch.core:core-runtime:2.0.0 -> 2.2.0 (*)
+| | +--- androidx.lifecycle:lifecycle-runtime:2.0.0 -> 2.6.2 (*)
+| | +--- androidx.lifecycle:lifecycle-livedata:2.0.0 -> 2.6.2 (*)
+| | \--- androidx.recyclerview:recyclerview:1.0.0 -> 1.3.0 (*)
+| +--- com.goterl:lazysodium-android:5.0.2
+| +--- net.java.dev.jna:jna:5.5.0
+| +--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.6.20 -> 1.9.10 (*)
+| +--- org.jetbrains.kotlin:kotlin-android-extensions-runtime:1.6.20 -> 1.9.22 (*)
+| +--- androidx.appcompat:appcompat:1.0.2 -> 1.6.1 (*)
+| +--- androidx.recyclerview:recyclerview:1.0.0 -> 1.3.0 (*)
+| +--- androidx.exifinterface:exifinterface:1.0.0 -> 1.3.6 (*)
+| +--- androidx.security:security-crypto:1.0.0
+| | +--- androidx.annotation:annotation:1.1.0 -> 1.6.0 (*)
+| | \--- com.google.crypto.tink:tink-android:1.5.0
+| +--- com.squareup.okhttp3:okhttp-urlconnection:4.9.0 -> 4.9.2 (*)
+| +--- com.google.code.gson:gson:2.8.5 -> 2.10.1
+| +--- org.apache.commons:commons-text:1.10.0
+| | \--- org.apache.commons:commons-lang3:3.12.0
+| +--- androidx.room:room-runtime:2.4.2 -> 2.5.0
+| | +--- androidx.annotation:annotation-experimental:1.1.0 -> 1.3.1 (*)
+| | +--- androidx.arch.core:core-runtime:2.0.1 -> 2.2.0 (*)
+| | +--- androidx.room:room-common:2.5.0
+| | | +--- androidx.annotation:annotation:1.3.0 -> 1.6.0 (*)
+| | | \--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.20 -> 1.9.10 (*)
+| | +--- androidx.sqlite:sqlite:2.3.0
+| | | +--- androidx.annotation:annotation:1.0.0 -> 1.6.0 (*)
+| | | \--- org.jetbrains.kotlin:kotlin-stdlib:1.7.20 -> 1.9.22 (*)
+| | \--- androidx.sqlite:sqlite-framework:2.3.0
+| | +--- androidx.annotation:annotation:1.2.0 -> 1.6.0 (*)
+| | +--- androidx.sqlite:sqlite:2.3.0 (*)
+| | \--- org.jetbrains.kotlin:kotlin-stdlib:1.7.20 -> 1.9.22 (*)
+| +--- androidx.room:room-ktx:2.4.2 -> 2.5.0
+| | +--- androidx.room:room-common:2.5.0 (*)
+| | +--- androidx.room:room-runtime:2.5.0 (*)
+| | +--- org.jetbrains.kotlin:kotlin-stdlib:1.7.20 -> 1.9.22 (*)
+| | \--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.6.4 -> 1.7.3 (*)
+| +--- com.google.dagger:dagger:2.42 -> 2.50
+| | \--- javax.inject:javax.inject:1
+| +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.3.9 -> 1.7.3 (*)
+| \--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.3.9 -> 1.7.3 (*)
\--- org.wordpress:login:1.15.0
- \--- org.wordpress:fluxc:trunk-ed60798b4d96ec19863c74b0f525e2e20f4525db -> 2.83.0 (*)
+ \--- org.wordpress:fluxc:trunk-ed60798b4d96ec19863c74b0f525e2e20f4525db -> trunk-f111f9d00446395f16550bca872c6098c23529e9 (*)
Please review and act accordingly
|
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
Fixes #20295
This is the companion PR for wordpress-mobile/WordPress-FluxC-Android#3030
Merge Instructions
This PR aims to resolve the issues described above. After investigating, I found that there were two issues:
When the "Optimize Images" option was enabled, we didn't copy the ExifInterface data (which includes location metadata) to the new optimized file.
When the "Optimize Images" option was disabled, we could not access the location data in the latest Android API versions because we were not requesting the ACCESS_MEDIA_LOCATION permission.
This PR requests the necessary permissions and uses the new ExifUtils functions to copy the ExifInterface data to the optimized media files.
To Test:
First check the existing problem:
Test Solution
16. Install the app from this PR.
17. Repeat steps 4-13
ATTENTION: If users chooses to "Take a photo" from our app and not choose a photo from the device, no location data are saved in the uploaded image. This functionality in not in the scope of this PR.
Regression Notes
Potential unintended areas of impact
What I did to test those areas of impact (or what existing automated tests I relied on)