Fix collapsible markdown images

[tdwio]

Summary

Fix collapsing markdown images

Ticket Link

N/A

Screenshots

• Before

Screen.Recording.2024-04-29.at.09.55.43.mov

• After

Screen.Recording.2024-04-29.at.09.55.53.mov

Release Note

Fix collapsible markdown images

[mattermost-build]

Hello @tdwio,

Thanks for your pull request! A Core Committer will review your pull request soon. For code contributions, you can learn more about the review process here.

[M-ZubairAhmed]

@tdwio do we have a ticket for this?

[mattermost-build]

E2E tests not automatically triggered, because the PR is not in a mergeable state. Please update the branch with the base branch and resolve outstanding conflicts.

[tdwio]

@tdwio do we have a ticket for this?

I have not found any related issue. However, the original feature came from this pull-request : mattermost/mattermost-webapp#7352, so I assumed this is a bug

[hmhealey]

Great find! I'm surprised that the tests for this didn't catch the issue, but perhaps there's a case we missed here.

Playing around with this myself without your changes, I've noticed that the image isn't collapsible right when it's posted, but becomes collapsible after I refresh. Do you see the same thing?

It seems like there's an underlying bug here where imageMetadata is empty when a post is first made which can cause some other issues like messing up the scroll position of the post list. Would you be interested in looking into that at all? If not, that's totally fine though since I can just file a separate ticket to investigate that.

[tdwio]

Playing around with this myself without your changes, I've noticed that the image isn't collapsible right when it's posted, but becomes collapsible after I refresh. Do you see the same thing?

I see the same thing. Also, if you edit a post and add a markdown image it will be collapsible

It seems like there's an underlying bug here where imageMetadata is empty when a post is first made which can cause some other issues like messing up the scroll position of the post list. Would you be interested in looking into that at all? If not, that's totally fine though since I can just file a separate ticket to investigate that.

I will look into that

[tdwio]

@hmhealey

I have found that imagesMetadata object have keys with normal image URLs (example : https://media1.giphy.com/media/VqztgK1DlT5JzPBPuR/200.gif) whereas attribs.src from messageHtmlToComponent refers to a proxied image URL (example: https://test.proxy/api/v4/image?url=[...]. Like you said, this caused imageMetadata to be undefined causing gifs/images to not be collapsible. Sometimes those URLs also have encoded ampersand &.

I have removed my original edits and pushed modifications to messageHtmlToComponent and necessary tests 👍

[hmhealey]

Great find. I'll try to take a look at this tomorrow since I want to see if I can find why the metadata still seems to be different on first post vs afterwards, but making sure that the clientside code can handle that sounds like a good fix. We've had issues with proxied URLs not being consistently returned by the server before, so it'll be nice to know that the web app can handle that too

[mattermost-build]

This PR has been automatically labelled "stale" because it hasn't had recent activity.
A core team member will check in on the status of the PR to help with questions.
Thank you for your contribution!