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.

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
+    // This is needed so the coordinates are formatted with a dot as decimal separator
-    val base = "geo:0,0?q=%.6f,%.6f".format(location.lat, location.lon)
+    val locale = Locale.ENGLISH
-    return if (label == null) {
+    return "geo:0,0?q=%.6f,%.6f (%s)".format(locale, location.lat, location.lon, urlEncoder(label.orEmpty()))
-        base
-    } else {
-        "%s (%s)".format(base, urlEncoder(label))
-    }
 }

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)
+    }
 }