search

telegramdesktop / tdesktop

Telegram Desktop messaging app
https://desktop.telegram.org/
Other
26.09k stars 5.18k forks source link

Copy-pasting from Firefox downloads image again #10564

Closed arisudesu closed 1 year ago

arisudesu commented 3 years ago

When copying images from firefox to telegram, the latter does not insert the image data as a photo, but instead downloads the same image by url. This wastes twice the amount of traffic on re-downloading the picture. In addition, this creates a problem when copying pictures from sites protected by authorization, because telegram does not have the same cookies - instead, it downloads the 403 page as a file and tries to send it.

Steps to reproduce (example 1)

  1. Take, for example, large photo from wikipedia (~80 MB): https://upload.wikimedia.org/wikipedia/commons/e/ec/Mona_Lisa%2C_by_Leonardo_da_Vinci%2C_from_C2RMF_retouched.jpg
  2. Right-click it and select "Copy image"
  3. Paste it into telegram input field
  4. Observe how long it is pasted

Steps to reproduce (example 2)

(You'll need account on pixiv.net - it's free)

  1. Navigate to a pixiv image, e.g.: https://www.pixiv.net/en/artworks/79037838
  2. Click on the artwork to enlarge it
  3. Right-click it and select "Copy image"
  4. Paste it into telegram input field
  5. Image is not copy-pasted, instead a 146 bytes file is prompted to send, which contains "403 Forbidden" message

Expected behaviour

Image is copy-pasted without redownload.

Actual behaviour

Telegram forces it to download again.

Configuration

Operating system: Windows 10 20H2

Version of Telegram Desktop: 2.6.1

Installation source (Linux Only) - the official website / GitHub releases / flatpak / snap / distribution package: -

Used theme: -

ilya-fedin commented 3 years ago

This is not downloading, tdesktop doesn't have code to download a picture from URL when pasting...

arisudesu commented 3 years ago

Then please explain what's going on? Why is the file containing the html error copied to telegram, while the downloaded image is displayed in the browser? This is evidence that the telegram does not use the already downloaded picture.

Another observation is that this does not happen in chrome, edge.

RememberTheAir commented 3 years ago

Both the examples you described don't seem to suggest that the picture is downloaded again by tdesktop

Example 1: pasting the image takes some time because the file is very large. If after pasting the image you click on "cancel", you'll see that tdesktop pasted the file uri it was sending in the input box, that is stored in some firefox cache directory (in my case it's file:///C:/Users/REDACTED/AppData/Local/Temp/Mona_Lisa,_by_Leonardo_da_Vinci,_from_C2RMF_retouched.jpg. The image is not downloaded by tdesktop again

Example 2: no idea why it sends that file instead of the picture, but apparently copy-pasting that image doesn't work in every software that supports this action (eg. try to paste it in an email)

ilya-fedin commented 3 years ago

What "file instead of the picture" are you both talking about? :thinking:

RememberTheAir commented 3 years ago

If you enlarge the image of the Pixiv link in the OP and copy-paste it in tdesktop, it will be sent as this file: 79037838_p0

If you open it with a text editor, it contains this:

<html>
<head><title>403 Forbidden</title></head>
<body>
<center><h1>403 Forbidden</h1></center>
<hr><center>nginx</center>
</body>
</html>

It is sent like this:

image

ilya-fedin commented 3 years ago

If you enlarge the image of the Pixiv link in the OP and copy-paste it in tdesktop

Oh, I don't want to create an account :(

arisudesu commented 3 years ago

I've recorded what's happening step by step with request logging on local server: https://a.pomf.cat/oldqgq.mp4.

Image is downloaded twice, first time in browser, second time by telegram (?) when pasted.

ilya-fedin commented 3 years ago

But tdesktop doesn't have any code for downloading any stuff on pasting. Maybe Qt does this?

reallyuniquename commented 3 years ago

That HTTP request comes from Firefox. I assume this has something to do with Firefox (not) caching 80MB image since I can't reproduce this with smaller files. Frankly, I don't know. Good catch though.

Also I do not recommend testing this with Pixiv, they prevent direct image access even for authorized users.

Aokromes commented 3 years ago

so, no telegram bug?

reallyuniquename commented 3 years ago

Probably not.

But it would be better to investigate this from Telegram side because some other applications manage to paste images from Firefox without this issue.

arisudesu commented 3 years ago

That HTTP request comes from Firefox. I assume this has something to do with Firefox (not) caching 80MB image since I can't reproduce this with smaller files. Frankly, I don't know. Good catch though.

It happens with small images too, except that you don't notice the delay.

reallyuniquename commented 3 years ago

It happens with small images too

For me it doesn't. Try something small like 256x256px and under 25KB. There is no additional request and no file:// thingy as @RememberTheAir described, i.e. image data is being copied by value and not by reference.

arisudesu commented 3 years ago

It happens for me: https://a.pomf.cat/rjiwza.mp4. Github avatar is 11274 bytes long.

I don't clearly understand what is happening between telegram and firefox.

reallyuniquename commented 3 years ago

@arisudesu You are right, it does happen with Github avatar.

Could you try serving the same image through local python HTTP server? For me it doesn't log any additional requests. Maybe it's not just about image size. Still, looks like a caching issue on Firefox side?

ilya-fedin commented 3 years ago

maybe the file firefox creates in temp directory is not a regular file, but some kind of special file, so that when some app tries to access it, Firefox gets it from internal cache or (depending on cache settings) asks it from the server again? This could be some new Firefox feature and can describe why such an issue appeared only now.

ilya-fedin commented 3 years ago

It happens for me: https://a.pomf.cat/rjiwza.mp4.

ah, the file is created after pasting. So it could be that Firefox sets some native handle in clipboard message, so that Windows itself asks Firefox to download file on pasting.

ilya-fedin commented 3 years ago

At that point it's worth for you @arisudesu and @reallyuniquename to compare your Firefox versions

arisudesu commented 3 years ago

Mine is 87.0, was 86.0 at the moment of opening the issue.

Meanwhile I've debugged Telegram and found what causes the download: it is weird behavior of QMimeData::text() which is used here: https://github.com/telegramdesktop/tdesktop/blob/a506e9b9eb734dc66e232f5a72af1da174ca21e2/Telegram/SourceFiles/history/history_widget.cpp#L364 It's Qt downloads file to obtain its file:// uri for text inserted on cancel. I've disabled call to QMimeData::text() here with the code:

return confirmSendingFiles(data, std::nullopt, !data->hasImage() ? data->text() : QString());

But image is still downloaded, so I had to disable one block here: https://github.com/telegramdesktop/tdesktop/blob/a506e9b9eb734dc66e232f5a72af1da174ca21e2/Telegram/SourceFiles/history/history_widget.cpp#L4470-L4483 with check on hasUrls():

if (hasImage && data->hasUrls()) {
    // original code
}

The image isn't downloaded anymore. No file uri is inserted on cancel too, though does it make sense? I don't know.

ilya-fedin commented 3 years ago

though does it make sense?

it's a bug as well (#8265) UPD: the code looks like it's done on a reason

arisudesu commented 3 years ago

Great. Now you can fix both problems in one go.

Although I have already set up a working environment and a fix, it is unlikely I'll be able to make a patch, because Telegram code is huge and I can't be sure if I don't break something else by accident. I leave this to core developers.

stale[bot] commented 3 years ago

Hey there!

This issue was inactive for a long time and will be automatically closed in 30 days if there isn't any further activity. We therefore assume that the user has lost interest or resolved the problem on their own.

Don't worry though; if this is an error, let us know with a comment and we'll be happy to reopen the issue.

Thanks!

aprosvetova commented 3 years ago

Unfortunately, the issue is still present on the latest stable version.

stale[bot] commented 2 years ago

Hey there!

This issue was inactive for a long time and will be automatically closed in 30 days if there isn't any further activity. We therefore assume that the user has lost interest or resolved the problem on their own.

Don't worry though; if this is an error, let us know with a comment and we'll be happy to reopen the issue.

Thanks!

arisudesu commented 2 years ago

Unfortunately, the issue is still present on the latest stable version.

This bot is kinda useless tbh.

Aokromes commented 2 years ago

no it's not, it helps us to close already fixed non-closed issues.

ilya-fedin commented 2 years ago

Yeah, stale bot is really useful. People open a lot of issues everyday and stalebot helps rotate them closing issues no one interested in. People like create duplicates and there's no manpower to check all the issues, the time when auto-closer bot was broken and there were no stalebot yet shown that issues reach 1000 numbers very quickly without rotation. An imperfect solution for imperfect world.

github-actions[bot] commented 2 years ago

Hey there!

This issue was inactive for a long time and will be automatically closed in 30 days if there isn't any further activity. We therefore assume that the user has lost interest or resolved the problem on their own.

Don't worry though; if this is an error, let us know with a comment and we'll be happy to reopen the issue.

Thanks!

seiya-git commented 2 years ago

Unfortunately, the issue is still present on the latest stable version.

ilya-fedin commented 2 years ago

This issue is now a part of #25126

seiya-git commented 1 year ago

Unfortunately, the issue is still present on the latest stable version.

ilya-fedin commented 1 year ago

No one helped with reporting it to Qt sadly and it seems it has too little priority for @john-preston to look at it

john-preston commented 1 year ago

Ok, it looks like I fixed this. We'll check in the next version.