Merge pull request #2781 from vector-im/feature/bma/widget_url_robustness

Fix a bunch of bugs and feature parity for Widget

CHANGES.md

@@ -8,6 +8,7 @@ Improvements 🙌:
  -
 
 Bugfix 🐛:
+ - Bug in WidgetContent.computeURL() (#2767)
  - Duplicate thumbs | Mobile reactions for 👍 and 👎 are not the same as web (#2776)
  - Join room by alias other federation error (#2778)
 

matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/widgets/WidgetService.kt

@@ -97,10 +97,11 @@ interface WidgetService {
     ): LiveData<List<Widget>>
 
     /**
-     * Creates a new widget in a room. It makes sure you have the rights to handle this.
+     * Creates and send a new widget in a room. It makes sure you have the rights to handle this.
      *
-     * @param roomId: the room where you want to deactivate the widget.
+     * @param roomId the room where you want to create the widget.
-     * @param widgetId: the widget to deactivate.
+     * @param widgetId the widget to create.
+     * @param content the content of the widget
      * @param callback the matrix callback to listen for result.
      * @return Cancelable
      */

matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/util/UrlExtensions.kt

@@ -35,3 +35,10 @@ fun StringBuilder.appendParamToUrl(param: String, value: String): StringBuilder
 
     return this
 }
+
+fun StringBuilder.appendParamsToUrl(params: Map<String, String>): StringBuilder {
+    params.forEach { (param, value) ->
+        appendParamToUrl(param, value)
+    }
+    return this
+}

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/widgets/DefaultWidgetURLFormatter.kt

@@ -20,11 +20,12 @@ import org.matrix.android.sdk.api.MatrixConfiguration
 import org.matrix.android.sdk.api.session.integrationmanager.IntegrationManagerConfig
 import org.matrix.android.sdk.api.session.integrationmanager.IntegrationManagerService
 import org.matrix.android.sdk.api.session.widgets.WidgetURLFormatter
+import org.matrix.android.sdk.api.util.appendParamToUrl
+import org.matrix.android.sdk.api.util.appendParamsToUrl
 import org.matrix.android.sdk.internal.session.SessionLifecycleObserver
 import org.matrix.android.sdk.internal.session.SessionScope
 import org.matrix.android.sdk.internal.session.integrationmanager.IntegrationManager
 import org.matrix.android.sdk.internal.session.widgets.token.GetScalarTokenTask
-import java.net.URLEncoder
 import javax.inject.Inject
 
 @SessionScope
@@ -90,25 +91,4 @@ internal class DefaultWidgetURLFormatter @Inject constructor(private val integra
         }
         return false
     }
-
-    private fun StringBuilder.appendParamsToUrl(params: Map<String, String>): StringBuilder {
-        params.forEach { (param, value) ->
-            appendParamToUrl(param, value)
-        }
-        return this
-    }
-
-    private fun StringBuilder.appendParamToUrl(param: String, value: String): StringBuilder {
-        if (contains("?")) {
-            append("&")
-        } else {
-            append("?")
-        }
-
-        append(param)
-        append("=")
-        append(URLEncoder.encode(value, "utf-8"))
-
-        return this
-    }
 }

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/widgets/helper/WidgetFactory.kt

@@ -65,23 +65,31 @@ internal class WidgetFactory @Inject constructor(private val userDataSource: Use
         )
     }
 
+    // Ref: https://github.com/matrix-org/matrix-widget-api/blob/master/src/templating/url-template.ts#L29-L33
     private fun WidgetContent.computeURL(roomId: String?, widgetId: String): String? {
         var computedUrl = url ?: return null
         val myUser = userDataSource.getUser(userId)
-        computedUrl = computedUrl
-                .replace("\$matrix_user_id", userId)
-                .replace("\$matrix_display_name", myUser?.displayName ?: userId)
-                .replace("\$matrix_avatar_url", myUser?.avatarUrl ?: "")
-                .replace("\$matrix_widget_id", widgetId)
 
-        if (roomId != null) {
+        val keyValue = data.mapKeys { "\$${it.key}" }.toMutableMap()
-            computedUrl = computedUrl.replace("\$matrix_room_id", roomId)
+
-        }
+        keyValue[WIDGET_PATTERN_MATRIX_USER_ID] = userId
-        for ((key, value) in data) {
+        keyValue[WIDGET_PATTERN_MATRIX_DISPLAY_NAME] = myUser?.getBestName() ?: userId
-            if (value is String) {
+        keyValue[WIDGET_PATTERN_MATRIX_AVATAR_URL] = myUser?.avatarUrl ?: ""
-                computedUrl = computedUrl.replace("$$key", URLEncoder.encode(value, "utf-8"))
+        keyValue[WIDGET_PATTERN_MATRIX_WIDGET_ID] = widgetId
-            }
+        keyValue[WIDGET_PATTERN_MATRIX_ROOM_ID] = roomId ?: ""
+
+        for ((key, value) in keyValue) {
+            computedUrl = computedUrl.replace(key, URLEncoder.encode(value.toString(), "utf-8"))
         }
         return computedUrl
     }
+
+    companion object {
+        // Value to be replaced in URLS
+        const val WIDGET_PATTERN_MATRIX_USER_ID = "\$matrix_user_id"
+        const val WIDGET_PATTERN_MATRIX_DISPLAY_NAME = "\$matrix_display_name"
+        const val WIDGET_PATTERN_MATRIX_AVATAR_URL = "\$matrix_avatar_url"
+        const val WIDGET_PATTERN_MATRIX_WIDGET_ID = "\$matrix_widget_id"
+        const val WIDGET_PATTERN_MATRIX_ROOM_ID = "\$matrix_room_id"
+    }
 }

vector/src/main/java/im/vector/app/features/call/conference/JitsiCallViewModel.kt

@@ -16,7 +16,6 @@
 
 package im.vector.app.features.call.conference
 
-import android.net.Uri
 import com.airbnb.mvrx.Fail
 import com.airbnb.mvrx.MvRxViewModelFactory
 import com.airbnb.mvrx.Success
@@ -64,14 +63,12 @@ class JitsiCallViewModel @AssistedInject constructor(
                 .subscribe {
                     val jitsiWidget = it.firstOrNull()
                     if (jitsiWidget != null) {
-                        val uri = Uri.parse(jitsiWidget.computedUrl)
-                        val confId = uri.getQueryParameter("confId")
                         val ppt = jitsiWidget.computedUrl?.let { url -> JitsiWidgetProperties(url, stringProvider) }
                         setState {
                             copy(
                                     widget = Success(jitsiWidget),
                                     jitsiUrl = "https://${ppt?.domain}",
-                                    confId = confId ?: "",
+                                    confId = ppt?.confId ?: "",
                                     subject = roomName ?: ""
                             )
                         }

vector/src/main/java/im/vector/app/features/call/conference/JitsiWidgetProperties.kt

@@ -19,9 +19,11 @@ package im.vector.app.features.call.conference
 import android.net.Uri
 import im.vector.app.R
 import im.vector.app.core.resources.StringProvider
+import java.net.URLDecoder
 
 class JitsiWidgetProperties(private val uriString: String, val stringProvider: StringProvider) {
     val domain: String by lazy { configs["conferenceDomain"] ?: stringProvider.getString(R.string.preferred_jitsi_domain) }
+    val confId: String? by lazy { configs["conferenceId"] }
     val displayName: String? by lazy { configs["displayName"] }
     val avatarUrl: String? by lazy { configs["avatarUrl"] }
 
@@ -30,8 +32,9 @@ class JitsiWidgetProperties(private val uriString: String, val stringProvider: S
     private val configs: Map<String, String?> by lazy {
         configString?.split("&")
                 ?.map { it.split("=") }
-                ?.map { (key, value) -> key to value }
+                ?.filter { it.size == 2 }
+                ?.map { (key, value) -> key to URLDecoder.decode(value, "UTF-8") }
                 ?.toMap()
-                ?: mapOf()
+                .orEmpty()
     }
 }

vector/src/main/java/im/vector/app/features/call/conference/VectorJitsiActivity.kt

@@ -37,6 +37,7 @@ import org.jitsi.meet.sdk.JitsiMeetConferenceOptions
 import org.jitsi.meet.sdk.JitsiMeetView
 import org.jitsi.meet.sdk.JitsiMeetViewListener
 import org.matrix.android.sdk.api.extensions.tryOrNull
+import timber.log.Timber
 import java.net.URL
 import javax.inject.Inject
 
@@ -154,13 +155,19 @@ class VectorJitsiActivity : VectorBaseActivity<ActivityJitsiBinding>(), JitsiMee
     }
 
     override fun onConferenceTerminated(p0: MutableMap<String, Any>?) {
+        Timber.v("JitsiMeetViewListener.onConferenceTerminated()")
+        // Do not finish if there is an error
+        if (p0?.get("error") == null) {
             finish()
         }
+    }
 
     override fun onConferenceJoined(p0: MutableMap<String, Any>?) {
+        Timber.v("JitsiMeetViewListener.onConferenceJoined()")
     }
 
     override fun onConferenceWillJoin(p0: MutableMap<String, Any>?) {
+        Timber.v("JitsiMeetViewListener.onConferenceWillJoin()")
     }
 
     companion object {

vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailViewModel.kt

@@ -90,6 +90,7 @@ import org.matrix.android.sdk.api.session.room.timeline.TimelineEvent
 import org.matrix.android.sdk.api.session.room.timeline.getTextEditableContent
 import org.matrix.android.sdk.api.session.widgets.model.Widget
 import org.matrix.android.sdk.api.session.widgets.model.WidgetType
+import org.matrix.android.sdk.api.util.appendParamToUrl
 import org.matrix.android.sdk.api.util.toOptional
 import org.matrix.android.sdk.internal.crypto.model.event.EncryptedEventContent
 import org.matrix.android.sdk.internal.crypto.model.event.WithHeldCode
@@ -400,15 +401,19 @@ class RoomDetailViewModel @AssistedInject constructor(
 
             // We use the default element wrapper for this widget
             // https://github.com/vector-im/element-web/blob/develop/docs/jitsi-dev.md
-            val url = "https://app.element.io/jitsi.html" +
+            // https://github.com/matrix-org/matrix-react-sdk/blob/develop/src/utils/WidgetUtils.ts#L469
-                    "?confId=$confId" +
+            val url = buildString {
-                    "#conferenceDomain=\$domain" +
+                append("https://app.element.io/jitsi.html")
-                    "&conferenceId=\$conferenceId" +
+                appendParamToUrl("confId", confId)
-                    "&isAudioOnly=${!action.withVideo}" +
+                append("#conferenceDomain=\$domain")
-                    "&displayName=\$matrix_display_name" +
+                append("&conferenceId=\$conferenceId")
-                    "&avatarUrl=\$matrix_avatar_url" +
+                append("&isAudioOnly=\$isAudioOnly")
-                    "&userId=\$matrix_user_id"
+                append("&displayName=\$matrix_display_name")
-
+                append("&avatarUrl=\$matrix_avatar_url")
+                append("&userId=\$matrix_user_id")
+                append("&roomId=\$matrix_room_id")
+                append("&theme=\$theme")
+            }
             val widgetEventContent = mapOf(
                     "url" to url,
                     "type" to WidgetType.Jitsi.legacy,

vector/src/main/java/im/vector/app/features/widgets/WidgetArgsBuilder.kt

@@ -16,11 +16,16 @@
 
 package im.vector.app.features.widgets
 
+import android.content.Context
 import im.vector.app.core.di.ActiveSessionHolder
+import im.vector.app.features.themes.ThemeUtils
 import org.matrix.android.sdk.api.session.widgets.model.Widget
 import javax.inject.Inject
 
-class WidgetArgsBuilder @Inject constructor(private val sessionHolder: ActiveSessionHolder) {
+class WidgetArgsBuilder @Inject constructor(
+        private val sessionHolder: ActiveSessionHolder,
+        private val context: Context
+) {
 
     @Suppress("UNCHECKED_CAST")
     fun buildIntegrationManagerArgs(roomId: String, integId: String?, screen: String?): WidgetArgs {
@@ -38,7 +43,8 @@ class WidgetArgsBuilder @Inject constructor(private val sessionHolder: ActiveSes
                 urlParams = mapOf(
                         "screen" to normalizedScreen,
                         "integ_id" to integId,
-                        "room_id" to roomId
+                        "room_id" to roomId,
+                        "theme" to getTheme()
                 ).filterNotNull()
         )
     }
@@ -54,7 +60,8 @@ class WidgetArgsBuilder @Inject constructor(private val sessionHolder: ActiveSes
                 widgetId = widgetId,
                 urlParams = mapOf(
                         "widgetId" to widgetId,
-                        "room_id" to roomId
+                        "room_id" to roomId,
+                        "theme" to getTheme()
                 ).filterNotNull()
         )
     }
@@ -66,7 +73,10 @@ class WidgetArgsBuilder @Inject constructor(private val sessionHolder: ActiveSes
                 baseUrl = baseUrl,
                 kind = WidgetKind.ROOM,
                 roomId = roomId,
-                widgetId = widgetId
+                widgetId = widgetId,
+                urlParams = mapOf(
+                        "theme" to getTheme()
+                ).filterNotNull()
         )
     }
 
@@ -74,4 +84,12 @@ class WidgetArgsBuilder @Inject constructor(private val sessionHolder: ActiveSes
     private fun Map<String, String?>.filterNotNull(): Map<String, String> {
         return filterValues { it != null } as Map<String, String>
     }
+
+    private fun getTheme(): String {
+        return if (ThemeUtils.isLightTheme(context)) {
+            "light"
+        } else {
+            "dark"
+        }
+    }
 }

vector/src/main/res/layout/item_timeline_event_base_state.xml

@@ -44,12 +44,13 @@
             android:id="@+id/messageVerificationRequestStub"
             style="@style/TimelineContentStubBaseParams"
             android:layout="@layout/item_timeline_event_verification_stub"
-            tools:visibility="gone" />
+            tools:visibility="visible" />
 
         <ViewStub
             android:id="@+id/messageVerificationDoneStub"
             style="@style/TimelineContentStubBaseParams"
             android:layout="@layout/item_timeline_event_status_tile_stub"
+            tools:layout_marginTop="180dp"
             tools:visibility="visible" />
 
     </FrameLayout>
@@ -59,8 +60,8 @@
         android:id="@+id/messageE2EDecoration"
         android:layout_width="16dp"
         android:layout_height="16dp"
-        android:layout_marginTop="4dp"
         android:layout_alignTop="@id/viewStubContainer"
+        android:layout_marginTop="4dp"
         android:layout_toStartOf="@id/viewStubContainer"
         android:visibility="gone"
         tools:src="@drawable/ic_shield_warning"
@@ -70,11 +71,11 @@
         android:id="@+id/messageFailToSendIndicator"
         android:layout_width="14dp"
         android:layout_height="14dp"
+        android:layout_alignTop="@+id/viewStubContainer"
         android:layout_marginStart="2dp"
+        android:layout_toEndOf="@+id/viewStubContainer"
         android:src="@drawable/ic_warning_badge"
         android:visibility="gone"
-        android:layout_toEndOf="@+id/viewStubContainer"
-        android:layout_alignTop="@+id/viewStubContainer"
         tools:visibility="visible" />