From f335eb1106c42b5f28734eeaaa0d42d70b5e8046 Mon Sep 17 00:00:00 2001
From: Jorge Martin Espinosa <jorgem@element.io>
Date: Mon, 7 Apr 2025 11:56:25 +0200
Subject: [PATCH] Fix the share location URI (#4544)

* Fix the share location URI

Use `ENGLISH` locale to make sure the coordinates are formatted using the right decimal separator.

Also, when no label parameter was added, the URI just centered the map on some coordinates, instead of adding a marker.
---
 .../common/actions/AndroidLocationActions.kt  | 15 +++++++--------
 .../actions/AndroidLocationActionsTest.kt     | 19 ++++++++++++++++++-
 2 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/features/location/impl/src/main/kotlin/io/element/android/features/location/impl/common/actions/AndroidLocationActions.kt b/features/location/impl/src/main/kotlin/io/element/android/features/location/impl/common/actions/AndroidLocationActions.kt
index b059a88dbb..304f8eb1aa 100644
--- a/features/location/impl/src/main/kotlin/io/element/android/features/location/impl/common/actions/AndroidLocationActions.kt
+++ b/features/location/impl/src/main/kotlin/io/element/android/features/location/impl/common/actions/AndroidLocationActions.kt
@@ -11,12 +11,14 @@ import android.content.Context
 import android.content.Intent
 import android.net.Uri
 import androidx.annotation.VisibleForTesting
+import androidx.core.net.toUri
 import com.squareup.anvil.annotations.ContributesBinding
 import io.element.android.features.location.api.Location
 import io.element.android.libraries.androidutils.system.openAppSettingsPage
 import io.element.android.libraries.di.AppScope
 import io.element.android.libraries.di.ApplicationContext
 import timber.log.Timber
+import java.util.Locale
 import javax.inject.Inject
 
 @ContributesBinding(AppScope::class)
@@ -25,7 +27,7 @@ class AndroidLocationActions @Inject constructor(
 ) : LocationActions {
     override fun share(location: Location, label: String?) {
         runCatching {
-            val uri = Uri.parse(buildUrl(location, label))
+            val uri = buildUrl(location, label).toUri()
             val showMapsIntent = Intent(Intent.ACTION_VIEW).setData(uri)
             val chooserIntent = Intent.createChooser(showMapsIntent, null)
             chooserIntent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK)
@@ -42,17 +44,14 @@ class AndroidLocationActions @Inject constructor(
     }
 }
 
+// Ref: https://developer.android.com/guide/components/intents-common#ViewMap
 @VisibleForTesting
 internal fun buildUrl(
     location: Location,
     label: String?,
     urlEncoder: (String) -> String = Uri::encode
 ): String {
-    // Ref: https://developer.android.com/guide/components/intents-common#ViewMap
-    val base = "geo:0,0?q=%.6f,%.6f".format(location.lat, location.lon)
-    return if (label == null) {
-        base
-    } else {
-        "%s (%s)".format(base, urlEncoder(label))
-    }
+    // This is needed so the coordinates are formatted with a dot as decimal separator
+    val locale = Locale.ENGLISH
+    return "geo:0,0?q=%.6f,%.6f (%s)".format(locale, location.lat, location.lon, urlEncoder(label.orEmpty()))
 }
diff --git a/features/location/impl/src/test/kotlin/io/element/android/features/location/impl/common/actions/AndroidLocationActionsTest.kt b/features/location/impl/src/test/kotlin/io/element/android/features/location/impl/common/actions/AndroidLocationActionsTest.kt
index 4a98e22167..5b584a39f8 100644
--- a/features/location/impl/src/test/kotlin/io/element/android/features/location/impl/common/actions/AndroidLocationActionsTest.kt
+++ b/features/location/impl/src/test/kotlin/io/element/android/features/location/impl/common/actions/AndroidLocationActionsTest.kt
@@ -11,6 +11,7 @@ import com.google.common.truth.Truth.assertThat
 import io.element.android.features.location.api.Location
 import org.junit.Test
 import java.net.URLEncoder
+import java.util.Locale
 
 internal class AndroidLocationActionsTest {
     // We use an Android-native encoder in the actual app, switch to an equivalent JVM one for the tests
@@ -25,7 +26,7 @@ internal class AndroidLocationActionsTest {
         )
 
         val actual = buildUrl(location, null, ::urlEncoder)
-        val expected = "geo:0,0?q=1.234568,123.456789"
+        val expected = "geo:0,0?q=1.234568,123.456789 ()"
 
         assertThat(actual).isEqualTo(expected)
     }
@@ -57,4 +58,20 @@ internal class AndroidLocationActionsTest {
 
         assertThat(actual).isEqualTo(expected)
     }
+
+    @Test
+    fun `buildUrl - URL encodes coordinates in locale with comma decimal separator`() {
+        val location = Location(
+            lat = 1.000001,
+            lon = 2.000001,
+            accuracy = 0f
+        )
+        // Set a locale with comma as decimal separator
+        Locale.setDefault(Locale.Category.FORMAT, Locale("pt", "BR"))
+
+        val actual = buildUrl(location, "(weird/stuff here)", ::urlEncoder)
+        val expected = "geo:0,0?q=1.000001,2.000001 (%28weird%2Fstuff+here%29)"
+
+        assertThat(actual).isEqualTo(expected)
+    }
 }