Bug 317255

Comment 1 Silenio Quarti 2011-04-07 11:19:05 EDT

Arun, please investigate this bug.

Comment 2 Arun Thondapu 2011-05-09 07:17:12 EDT

Created attachment 195055 [details]
Proposed patch

Please review this patch.
I have tested it using the 'ClipboardExample' as well as by launching Eclipse with the 'Eclipse Application' run configuration.
Pasting of copied text works now even after Eclipse exits.

Please let me know if this fix can get into 3.7 RC1.

Thanks!

Comment 3 Silenio Quarti 2011-05-09 10:55:36 EDT

Yes, we should do this for RC1, but first please address these issues:

- Patch is missing the changes in OS.java. Please include them.

- It is better to include the auto generated changes as well (in os.c, os_stats.c and os_stats.h). It is easier for someone to review.

- The functions gtk_clipboard_set_can_store() and gtk_clipboard_store() were only added in GTK 2.6. Need to add a version check. Something like "OS.GTK_VERSION >= OS.VERSION(2, 6, 0)". You should still call gtk_clipboard_clear() for old versions.

Comment 4 Arun Thondapu 2011-05-09 13:11:32 EDT

Oops sorry I missed those files in the patch by mistake.
I'll upload a revised patch which includes the version check too.

(In reply to comment #3)
> Yes, we should do this for RC1, but first please address these issues:
> 
> - Patch is missing the changes in OS.java. Please include them.
> 
> - It is better to include the auto generated changes as well (in os.c,
> os_stats.c and os_stats.h). It is easier for someone to review.
> 
> - The functions gtk_clipboard_set_can_store() and gtk_clipboard_store() were
> only added in GTK 2.6. Need to add a version check. Something like
> "OS.GTK_VERSION >= OS.VERSION(2, 6, 0)". You should still call
> gtk_clipboard_clear() for old versions.

Comment 5 Arun Thondapu 2011-05-10 04:32:52 EDT

Created attachment 195182 [details]
Revised patch

This is the revised patch which contains all the relevant changes.
Please review the same and suggest if I have missed anything else.

Thanks!

Comment 6 Remy Suen 2011-05-10 05:52:48 EDT

(In reply to comment #5)
> Created attachment 195182 [details]
> Revised patch
> 
> This is the revised patch which contains all the relevant changes.
> Please review the same and suggest if I have missed anything else.

gtk_clipboard_set_can_store is also a GTK 2.6 method (see Silenio's comment 3).
http://developer.gnome.org/gtk/2.6/gtk-Clipboards.html#gtk-clipboard-set-can-store

Comment 7 Silenio Quarti 2011-05-10 11:05:50 EDT

Arun,

- Please add the version check around gtk_clipboard_set_can_store() as well. 
- Try to keep the same formatting. An if/else statement should always have the curly brackets block:

if (OS.GTK_VERSION >= OS.VERSION(2, 6, 0)) {
    OS.gtk_clipboard_store(Clipboard.GTKCLIPBOARD);
} else {
    OS.gtk_clipboard_clear(Clipboard.GTKCLIPBOARD);
}

An if statement by itself may not have the curly bracket block, but the block should be in the same line. For example:

if (clipboardOwner != 0) OS.gtk_widget_destroy (clipboardOwner);

Comment 8 Silenio Quarti 2011-05-10 16:33:16 EDT

Created attachment 195273 [details]
final patch

Here is the patch my last requests. I will release the changes so that we can get it in tomorrow's I-build.

Comment 9 Silenio Quarti 2011-05-10 16:39:16 EDT

Fixed > 20110510

Thanks Arun!