Reply preview placeholders don't always populate

ara4n commented 1 year ago

Steps to reproduce

Open a room
See a reply, with a reply preview placeholder embedded in it.
The placeholder never resolves with a preview of the replied message, despite the user having permission (and keys) to view the reply.
Scroll up the timeline to force the app to load the reply
Observe the placeholder now resolves.

Outcome

What did you expect?

Reliable reply previews

What happened instead?

App doesn't seem to proactively go and fetch reply previews via /context or bundled aggregations or whatever if the timeline doesn't already track the replied msg's event.

Your phone model

No response

Operating system version

No response

Application version

279

Homeserver

No response

Will you send logs?

No

ara4n commented 1 year ago

just caught this one in the act and rageshaked it: https://github.com/vector-im/element-x-ios-rageshakes/issues/1044

ara4n commented 10 months ago

this still happens once every few days for me:

giomfo commented 7 months ago

I don't reproduce this issue in v1.6.2 (559). I opened several rooms, and succeeded to observe several replies with a reply preview placeholder embedded in them. The placeholder was always resolves with a preview of the replied message.

pixlwave commented 6 months ago

Closing this as stale, seems to work as expected even when the reply is years old (so not yet downloaded through a pagination)

ara4n commented 3 months ago

Still happening on build 667. The reply is stuck in state "pending":


EventTimelineItem {
    sender: "@andybalaam:matrix.org",
    sender_profile: Ready(
        Profile {
            display_name: Some(
                "andybalaam",
            ),
            display_name_ambiguous: false,
            avatar_url: Some(
                "mxc://matrix.org/HcOKfHoyUseJyNvJCZbySygK",
            ),
        },
    ),
    timestamp: 2024-07-31T13:33:38.769,
    content: Message(
        Message {
            in_reply_to: Some(
                InReplyToDetails {
                    event_id: "$Gy7wlbILga6N7xOMp9QNgBThOMxIkqG-wBbpaFVmtFQ",
                    event: Pending,
                },
            ),
            thread_root: None,
            edited: false,
            ..
        },
    ),
    kind: Remote(
        RemoteEventTimelineItem {
            event_id: "$Ki2KRc60Uada13bifSqlmyWUWWytN2oxjvTagZYo0Yk",
            transaction_id: None,
            reactions: {},
            read_receipts: {},
            is_own: false,
            is_highlighted: false,
            encryption_info: Some(
                EncryptionInfo {
                    sender: "@andybalaam:matrix.org",
                    sender_device: Some(
                        "MKBDKTQJQJ",
                    ),
                    algorithm_info: MegolmV1AesSha2 {
                        curve25519_key: "Ss6eaJGA82Uc9TQn1BbcLqpm6jtjGGwqMBadf/8Y+WY",
                        sender_claimed_keys: {
                            "ed25519": "mPXKwg8Yz0WIDedksSIbcSdYSyQMB1HI08skehk16tc",
                        },
                    },
                    verification_state: Unverified(
                        None(
                            InsecureSource,
                        ),
                    ),
                },
            ),
            origin: Pagination,
            ..
        },
    ),
}

{
  "sender" : "@andybalaam:matrix.org",
  "content" : {
    "body" : "> <@andybalaam:matrix.org> Hi Element X, we in the Crypto team have one additional piece of work for \"EX production-ready\" - it's the one that was marked as \"not ready\" when we had a conversation a couple of days ago.\n> \n> We now have a preferred option and I wanted to ask you whether it sounds feasible.\n> \n> The purpose is to prevent people who have explicitly verified someone accidentally sending messages. We need to cover 2 cases:\n> \n> 1. A verified person becomes unverified\n> 2. A verified person has a new unsigned device\n> \n> in both cases, when I try to send a message, we want sending to fail, and then an error message to be displayed.\n> \n> So we expect to change the message sending code (in Rust) to throw an error in these cases, and we expect the UI to display the error and offer options.\n> \n> The options in case 1 would be \"withdraw verification\" (mark the person as unverified and send the message) or \"cancel sending\" (discard the message). Note that on EX there is no \"Re-verify\", which is what we would really like.\n> \n> The options in case 2 would be \"send anyway\" (send the message to the unverified device and remember that we don't need to ask again for that device) or \"cancel sending\" (discard the message).\n> \n> I'd like to ask:\n> \n> * in terms of the Rust side: sending messages throwing this kind of permanent error, and the send queue dealing with it\n> * in terms of the UI sides: displaying these errors on send and taking the actions\n> \n> Does this sound do-able?\n\nI made an issue to cover this item: https:\/\/github.com\/element-hq\/element-meta\/issues\/2488",
    "format" : "org.matrix.custom.html",
    "formatted_body" : "<mx-reply><blockquote><a href=\"https:\/\/matrix.to\/#\/!kCCQTCfnABLKGGvQjo:matrix.org\/$Gy7wlbILga6N7xOMp9QNgBThOMxIkqG-wBbpaFVmtFQ?via=matrix.org&via=element.io&via=one.ems.host\">In reply to<\/a> <a href=\"https:\/\/matrix.to\/#\/@andybalaam:matrix.org\">@andybalaam:matrix.org<\/a><br><p>Hi Element X, we in the Crypto team have one additional piece of work for \"EX production-ready\" - it's the one that was marked as \"not ready\" when we had a conversation a couple of days ago.<\/p>\n<p>We now have a preferred option and I wanted to ask you whether it sounds feasible.<\/p>\n<p>The purpose is to prevent people who have explicitly verified someone accidentally sending messages. We need to cover 2 cases:<\/p>\n<ol>\n<li>A verified person becomes unverified<\/li>\n<li>A verified person has a new unsigned device<\/li>\n<\/ol>\n<p>in both cases, when I try to send a message, we want sending to fail, and then an error message to be displayed.<\/p>\n<p>So we expect to change the message sending code (in Rust) to throw an error in these cases, and we expect the UI to display the error and offer options.<\/p>\n<p>The options in case 1 would be \"withdraw verification\" (mark the person as unverified and send the message) or \"cancel sending\" (discard the message). Note that on EX there is no \"Re-verify\", which is what we would really like.<\/p>\n<p>The options in case 2 would be \"send anyway\" (send the message to the unverified device and remember that we don't need to ask again for that device) or \"cancel sending\" (discard the message).<\/p>\n<p>I'd like to ask:<\/p>\n<ul>\n<li>in terms of the Rust side: sending messages throwing this kind of permanent error, and the send queue dealing with it<\/li>\n<li>in terms of the UI sides: displaying these errors on send and taking the actions<\/li>\n<\/ul>\n<p>Does this sound do-able?<\/p>\n<\/blockquote><\/mx-reply>I made an issue to cover this item: https:\/\/github.com\/element-hq\/element-meta\/issues\/2488",
    "m.mentions" : {

    },
    "m.relates_to" : {
      "m.in_reply_to" : {
        "event_id" : "$Gy7wlbILga6N7xOMp9QNgBThOMxIkqG-wBbpaFVmtFQ"
      },
      "is_falling_back" : false
    },
    "msgtype" : "m.text"
  },
  "origin_server_ts" : 1722432818769,
  "room_id" : "!kCCQTCfnABLKGGvQjo:matrix.org",
  "event_id" : "$Ki2KRc60Uada13bifSqlmyWUWWytN2oxjvTagZYo0Yk",
  "type" : "m.room.message",
  "unsigned" : {
    "age" : 344382435,
    "membership" : "join"
  }
}

element-hq / element-x-ios