element-hq / element-integration-manager

Element Integration Manager related issues
6 stars 1 forks source link

The title of a Jitsi widget is set to `$roomName` #102

Closed luixxiul closed 1 year ago

luixxiul commented 1 year ago

Steps to reproduce

  1. Create a room
  2. Add a Jitsi widget
  3. Enable the widget
  4. Hover on the widget
  5. Check the title which appears on the widget

Outcome

What did you expect?

What happened instead?

Screenshot from 2023-06-17 13-44-57

Operating system

Debian

Browser information

Firefox

URL for webapp

develop.element.io

Application version

No response

Homeserver

No response

Will you send logs?

No

Johennes commented 1 year ago

I've managed to reproduce this on a room that has a non-empty room name.

Johennes commented 1 year ago

@robintown are you sure this is a regression? It looks like we never injected roomName in https://github.com/matrix-org/matrix-react-sdk/blob/develop/src/stores/widgets/StopGapWidget.ts#L214? Or I might be looking in the wrong place. 🤔

t3chguy commented 1 year ago

My guess is the generated URL had the $roomName added, causing this to be effectively a regression. Likely a change in the integration manager caused this.

dhenneke commented 1 year ago

The URL is generated in https://github.com/matrix-org/matrix-react-sdk/blob/ab982689012bec6b916c9e3d9094665d6df87693/src/utils/WidgetUtils.ts#L534. There is no $roomName parameter in the widget API, but it is provided from the content.data.roomName field of the im.vector.modular.widgets state event for Jitsi.

This would be added by Element when using the "Call" button in the not-element-call version: https://github.com/matrix-org/matrix-react-sdk/blob/ab982689012bec6b916c9e3d9094665d6df87693/src/utils/WidgetUtils.ts#L504-L511

But when I add it via the integration manager, the field is missing:

{
  "content": {
    "data": {
      "conferenceId": "XXXX",
      "domain": "meet.element.io",
      "isAudioOnly": false
    },
    "name": "Jitsi Meet",
    "type": "jitsi",
    "url": "https://scalar.vector.im/api/widgets/jitsi.html",
    "creatorUserId": "XXXX",
    "id": "XXXX",
    "roomId": "XXXX",
    "eventId": "$ItMoJUHBi_iM3KLap3vSk9L80T9ICeHJkDsWOlE6PNg"
  },
  "origin_server_ts": 1687767163326,
  "sender": "XXXX",
  "state_key": "XXXX",
  "type": "im.vector.modular.widgets",
  "event_id": "$ItMoJUHBi_iM3KLap3vSk9L80T9ICeHJkDsWOlE6PNg",
  "room_id": "XXXX"
}

(it also won't ever be updated if the room name changes, but that is a different discussion)

justinbot commented 1 year ago

We can work around this in the short term by having the integration manager provide the room name.

However I think it makes more sense for the room name to be templated in by the client, considering it already handles $matrix_room_id, $matrix_display_name, $matrix_avatar_url, and so on.

Johennes commented 1 year ago

However I think it makes more sense for the room name to be templated in by the client, considering it already handles $matrix_room_id, $matrix_display_name, $matrix_avatar_url, and so on.

That does sound sensible to me but I think we should get it standardized as part of the API first? The only reference to this I've found was in https://docs.google.com/document/d/1uPF7XWY_dXTKVKV7jZQ2KmsI19wn9-kFRgQ1tFQP7wQ/edit#heading=h.9rn9lt6ctkgi and that doesn't specify a room name parameter, only the opaque data block.

Johennes commented 1 year ago

(Thanks for fixing it on the integration manager side in the meantime and so quickly!)