status-im / status-desktop

Status Desktop client made in Nim & QML
https://status.app
Mozilla Public License 2.0
304 stars 79 forks source link

Chat: the incorrect GIF address doesn't look good #8560

Open elina2015 opened 2 years ago

elina2015 commented 2 years ago

Bug Report

Description

Steps to reproduce

  1. Go to the test chat and send some GIF
  2. Edit the GIF message, so it becomes an incorrect GIF address (for example https://media.tenor.com/2kbyBJ60OZ0AAAACd/thank-you.gif)
  3. Save the edited GIF message

Expected behavior

The correct default broken GIF image is shown

Actual behavior

The incorrect grey form is shown

Additional Information

Status desktop version: https://github.com/status-im/status-desktop/releases/tag/0.8.0-rc.9 Operating System: macOS Monterey

NOTES: 43 bytes gif is received. We must do a validation on status_go side

jrainville commented 1 year ago

If the gif is not valid, we should just display the link as a link and not show the LinkMessageView

mprakhov commented 1 year ago

Ok, so the problem is that the corrupted gif link is correct, we didn't receive an error from the server, but received a 43 bytes gif, which is empty Tried to validate such a gif via QMovie->isValid() and QImage -> format - everything recognized ok

The only solution I see right now is to load the gif in memory, and check if it is bigger than 43 bytes (this is for the tenor vendor, need to check for another). If less or equal - gif is empty and should not be displayed

jrainville commented 1 year ago

As discussed during the meeting, we'll try the ad hoc fix, since the issue is pretty minor and we don't want to spend too much time on a fix for that.

You can open another issue for a later milestone to move the fix to status-go and do a HEAD on the file to check filesize like you said. We just need to make sure to only do that when the gif setting is ON

jrainville commented 1 year ago

@mprakhov17 you can do the fancy status-go solution for this one. Reminder: check the setting first if gif is enabled. We do not want to download or HEAD any outside file unless the user actually accepted it.

iurimatias commented 1 year ago

Screen Shot 2023-04-20 at 12 29 45 PM

this is how it looks like

alexandraB99 commented 12 months ago

@mprakhov what would be the state/solution of this?

mprakhov commented 12 months ago

Unfortunately, I do not remember the whole context, as it was a lot of time ago, it require digging into the problem again

alexandraB99 commented 12 months ago

@mprakhov from your comment I understand that since there is no indication whether the GIF exists or not we could check the file size, and from @jrainville's comment, this needs to be done in go, if so I would mark this issue as messenger-team.

mprakhov commented 12 months ago

I don't have any objections to assign it on messenger team

iurimatias commented 9 months ago

moved to 2.29 due to lack of space in this milestone

iurimatias commented 3 months ago

the current behaviour now is that actually the old gif remains regardless of any edits