412196 - [1.8 only] patch to allow pasting image from clipboard into composer under linux/unix

Status: RESOLVED WONTFIX

(Core :: Widget: Gtk, enhancement)

Description

[Tobias Koenig]

User-Agent:       w3m/0.5.1+cvs-1.968
Build Identifier: 

Hej,

attached is a patch which adds the functionality to Thunderbird 2.0 to paste
an image from the clipboard under Linux/Unix (whenever gtk2 widget is used...).

I hope this patch will make it upstream :)

Ciao,
Tobias


Reproducible: Always

Steps to Reproduce:
1.
2.
3.

Comment 1

[Tobias Koenig]

Attachment: Paste clipboard image into composer under Linux/Unix

Comment 2

[Magnus Melin [:mkmelin]]

I don't think you need the #ifdef MOZILLA_1_8_BRANCH since the patch isn't very likely to get accepted for tb2, only for tb3.

To get the patch included, set the appropriate review/superreview flag of the patch.
See http://developer.mozilla.org/en/docs/Getting_your_patch_in_the_tree

Marking NEW as we got a patch.

Comment 3

[Magnus Melin [:mkmelin]]

Comment on attachment 296847 [details] [diff] [review]
Paste clipboard image into composer under Linux/Unix

I'm not a reviewer for this code, try [e-mail] perhaps?

Comment 4

[Reed Loden [:reed]]

Comment on attachment 296847 [details] [diff] [review]
Paste clipboard image into composer under Linux/Unix

Please make your patch against the latest trunk CVS.

Comment 5

[Tobias Koenig]

Hmm, it seems this issue is fixed in latest trunk already (at least what I can read from the sources) :| Can you paste images from clipboard under Linux? (compiling trunk here again will take some hours).

Comment 6

[Michael Ventnor]

(In reply to comment #5)
> Hmm, it seems this issue is fixed in latest trunk already (at least what I can
> read from the sources) :| Can you paste images from clipboard under Linux?
> (compiling trunk here again will take some hours).
> 

AFAICT you can. This was fixed as part of bug 21747.

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 296847 [details] [diff] [review]
Paste clipboard image into composer under Linux/Unix

guess we don't need this then.

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I suppose Linux distributions might want to take this on branch, in which case we could take it on branch too. But we don't need it on trunk anymore.

Comment 9

[Tobias Koenig]

Ok, so do I have to open the bug again and trigger a review for branch?

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Blizzard, caillon, who decides what to take on branch for Linux these days?

Comment 11

[Christopher Blizzard (:blizzard)]

I trust the Linux brain trust at the distributions - caillon, asac, others.  With your review, of course.

Comment 12

[Christopher Aillon (sabbatical, not receiving bugmail)]

For 1.8.0.x that's asac and me.
For 1.8.1.x that's still dveditz and co.

This patch would need to get reviewed/approved for the 1.8.1 branch first I think before we'd take it on the 1.8.0 branch so we don't "regress" moving forward.  But I'm not opposed to having it, pending discussion with asac if the rest falls into place, though I'd rather see the patch support more than just jpg (not sure if that's possible back then)

Comment 13

[Robert O'Callahan (:roc) (email my personal email if necessary)]

-        tv.tv_sec = 0;
+        tv.tv_sec = 5;
         tv.tv_usec = kClipboardTimeout;

You're adding 5 seconds here? wouldn't it make more sense to change kClipboardTimeout?

+                        if (gdk_pixbuf_get_has_alpha(new_pixbuf) == TRUE)
+                            format = imgIEncoder::INPUT_FORMAT_RGBA;

Why bother since we can't create a transparent image anyway?

Comment 14

[Tobias Koenig]

(In reply to comment #13)
> -        tv.tv_sec = 0;
> +        tv.tv_sec = 5;
>          tv.tv_usec = kClipboardTimeout;
> 
> You're adding 5 seconds here? wouldn't it make more sense to change
> kClipboardTimeout?
Well, because increasing the microseconds is not enough, you need a timeout
of several seconds for large images.

> +                        if (gdk_pixbuf_get_has_alpha(new_pixbuf) == TRUE)
> +                            format = imgIEncoder::INPUT_FORMAT_RGBA;
> 
> Why bother since we can't create a transparent image anyway?
We can't? So why does ImageEncoder provide that format flag?

Ciao,
Tobias

Comment 15

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Maybe the encoder is capable of creating transparent images, but you're a) creating a JPEG, which doesn't support transparency and b) passing "transparency=none".

> Well, because increasing the microseconds is not enough, you need a timeout
> of several seconds for large images.

OK, but you're changing kClipboardTimeout to mean "the clipboard timeout, minus five seconds", which is not very intuitive. Either you need two constants, say kClipboardTimeoutSeconds and kClipboardTimeoutMicroseconds, or you need to change kClipboardTimeout to 5000000 microseconds and do some arithmetic to split it up into the tv fields.

Comment 16

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 296847 [details] [diff] [review]
Paste clipboard image into composer under Linux/Unix

Comments need to be addressed

Comment 17

[Vincent (caméléon)]

Tobias, do you still work on this -annoying- bug? Seems most of the job is done, so it would be a pity if we stop when 90% of the job is done :-(

Comment 18

[Magnus Melin [:mkmelin]]

-> WONTFIX as this works on recent versions, and the 1.8 branch is dead.

Comment 19

[Vincent (caméléon)]

sorry, it doesn't works for me on Ubuntu 11.10 and TB8 (also tryed with earlybird). Are you sure that this bug is fixed?

Comment 20

[Magnus Melin [:mkmelin]]

Yup- just tried it, works just fine - ubuntu (still 11.04) and tb 3.1.13, also latest trunk.
Make sure you're using HTML compose mode though...

Comment 21

[Vincent (caméléon)]

Attachment: exemple of odt document which doesn't work in Thunderbird 8

exemple of an image of a car in a odt document which doesn't work in Thunderbird 8.
I also try in safe mode, the result is the same: the "past" command in the "Edit" menu is disabled (gray). I am sure I compose mail in html. I also check that the past command works in other application (like Gimp).

Could you do the test to with this particular file? Any other person can give its feedback?

Comment 22

[Magnus Melin [:mkmelin]]

Yeah copying from that file doesn't work. Maybe the source have to be in compatible format (jpg/gif/png)? Try finding a pic on the web, in firefox go to the image and use "Copy Image" from the context menu, then paste in thunderbird. That's what i tested.

Comment 23

[Vincent (caméléon)]

Ok, I understand now that my problem should have its own bug report. I will try to find an existing one or create a new one if needed.