GNOME Bugzilla – Bug 581488
copy/paste is broken
Last modified: 2009-10-11 20:14:47 UTC
I often have discussions which I need to in turn forward/discuss with others, so copying text out of my conversation window is very important. I use the "Clean" theme, and when I copy and paste text from Empathy conversation windows, the contacts' names and all of the smileys are lost, which makes the conversation hard to follow, and loses quite a lot of the meaning.
Created attachment 137702 [details] [review] Proposed fix in branch fix-581488. Fix copy to clipboard to show smileys and to show name and time when using themes other than Classic. http://gitorious.org/~gmillaire/empathy/gmillaires-clone/commits/fix-581488 libempathy-gtk/empathy-chat-text-view.c | 47 ++++++++++++++++++++++++++++++- libempathy-gtk/empathy-smiley-manager.c | 2 + libempathy-gtk/empathy-theme-boxes.c | 11 +++++++ 3 files changed, 59 insertions(+), 1 deletions(-)
*** Bug 560584 has been marked as a duplicate of this bug. ***
Created attachment 144308 [details] [review] Fix in branch fix-581488 http://gitorious.org/~gmillaire/empathy/gmillaires-clone/commits/fix-581488 configure.ac | 2 +- libempathy-gtk/Makefile.am | 2 - libempathy-gtk/empathy-account-chooser.c | 71 +- libempathy-gtk/empathy-account-chooser.h | 10 +- libempathy-gtk/empathy-account-widget-irc.c | 39 +- libempathy-gtk/empathy-account-widget-irc.h | 4 +- libempathy-gtk/empathy-account-widget-sip.c | 9 +- libempathy-gtk/empathy-account-widget-sip.h | 4 +- libempathy-gtk/empathy-account-widget.c | 75 +- libempathy-gtk/empathy-account-widget.h | 22 +- libempathy-gtk/empathy-avatar-chooser.c | 10 +- libempathy-gtk/empathy-avatar-image.c | 2 +- libempathy-gtk/empathy-cell-renderer-expander.c | 38 +- libempathy-gtk/empathy-chat-text-view.c | 58 +- libempathy-gtk/empathy-chat.c | 112 +- libempathy-gtk/empathy-chat.h | 2 +- libempathy-gtk/empathy-contact-dialogs.c | 20 +- libempathy-gtk/empathy-contact-list-view.c | 21 +- libempathy-gtk/empathy-contact-widget.c | 14 +- libempathy-gtk/empathy-contact-widget.ui | 12 +- libempathy-gtk/empathy-irc-network-dialog.c | 2 +- libempathy-gtk/empathy-kludge-label.c | 89 - libempathy-gtk/empathy-kludge-label.h | 53 - libempathy-gtk/empathy-log-window.c | 45 +- libempathy-gtk/empathy-log-window.h | 4 +- libempathy-gtk/empathy-new-message-dialog.c | 2 +- libempathy-gtk/empathy-smiley-manager.c | 2 + libempathy-gtk/empathy-status-preset-dialog.c | 4 +- libempathy-gtk/empathy-theme-adium.c | 169 +-- libempathy-gtk/empathy-theme-boxes.c | 11 + libempathy-gtk/empathy-theme-manager.c | 67 +- libempathy-gtk/empathy-theme-manager.h | 1 - libempathy-gtk/empathy-ui-utils.c | 8 +- libempathy-gtk/empathy-ui-utils.h | 4 +- libempathy-gtk/empathy-video-widget.c | 6 +- libempathy/Makefile.am | 3 - libempathy/empathy-account-manager.c | 579 +++-- libempathy/empathy-account-manager.h | 19 +- libempathy/empathy-account-priv.h | 44 - libempathy/empathy-account.c | 575 ----- libempathy/empathy-account.h | 94 - libempathy/empathy-chatroom-manager.c | 21 +- libempathy/empathy-chatroom-manager.h | 11 +- libempathy/empathy-chatroom.c | 20 +- libempathy/empathy-chatroom.h | 11 +- libempathy/empathy-contact-manager.c | 1 - libempathy/empathy-contact.c | 13 +- libempathy/empathy-contact.h | 6 +- libempathy/empathy-dispatch-operation.c | 4 +- libempathy/empathy-dispatch-operation.h | 2 - libempathy/empathy-log-manager.c | 15 +- libempathy/empathy-log-manager.h | 12 +- libempathy/empathy-log-store-empathy.c | 35 +- libempathy/empathy-log-store.c | 12 +- libempathy/empathy-log-store.h | 26 +- libempathy/empathy-message.c | 36 - libempathy/empathy-message.h | 3 - libempathy/empathy-tp-chat.c | 65 +- libempathy/empathy-tp-roomlist.c | 46 +- libempathy/empathy-tp-roomlist.h | 4 +- libempathy/empathy-utils.c | 15 +- libempathy/empathy-utils.h | 1 + megaphone/src/megaphone-applet.c | 30 +- po/LINGUAS | 1 - po/es.po | 469 ++-- po/fr.po | 2103 +++++++----------- po/or.po | 2490 -------------------- po/pa.po | 1060 ++++----- po/sv.po | 2811 ++++++++++++----------- po/ta.po | 561 ++--- python/pyempathy/pyempathy.defs | 137 +- python/pyempathygtk/pyempathygtk.defs | 101 +- src/empathy-accounts-dialog.c | 125 +- src/empathy-accounts-dialog.h | 4 +- src/empathy-call-window-fullscreen.c | 6 +- src/empathy-chat-window.c | 50 +- src/empathy-chat-window.h | 4 +- src/empathy-chatrooms-window.c | 4 +- src/empathy-debug-dialog.c | 4 +- src/empathy-event-manager.c | 11 +- src/empathy-import-dialog.c | 24 +- src/empathy-import-dialog.h | 1 - src/empathy-import-pidgin.c | 1 + src/empathy-main-window.c | 25 +- src/empathy-new-chatroom-dialog.c | 16 +- src/empathy-preferences.c | 161 +- src/empathy-preferences.ui | 44 + src/empathy-sidebar.c | 2 +- src/empathy-status-icon.c | 16 +- src/empathy.c | 24 +- src/ephy-spinner.c | 4 +- tests/check-empathy-chatroom-manager.c | 45 +- tests/check-empathy-chatroom.c | 2 +- tests/check-empathy-helpers.c | 25 +- tests/check-empathy-helpers.h | 6 +- tests/check-helpers.h | 1 + 96 files changed, 4459 insertions(+), 8576 deletions(-)
Created attachment 144309 [details] [review] Fix in branch fix-581488 http://gitorious.org/~gmillaire/empathy/gmillaires-clone/commits/fix-581488 configure.ac | 2 +- libempathy-gtk/Makefile.am | 2 - libempathy-gtk/empathy-account-chooser.c | 71 +- libempathy-gtk/empathy-account-chooser.h | 10 +- libempathy-gtk/empathy-account-widget-irc.c | 39 +- libempathy-gtk/empathy-account-widget-irc.h | 4 +- libempathy-gtk/empathy-account-widget-sip.c | 9 +- libempathy-gtk/empathy-account-widget-sip.h | 4 +- libempathy-gtk/empathy-account-widget.c | 75 +- libempathy-gtk/empathy-account-widget.h | 22 +- libempathy-gtk/empathy-avatar-chooser.c | 10 +- libempathy-gtk/empathy-avatar-image.c | 2 +- libempathy-gtk/empathy-cell-renderer-expander.c | 38 +- libempathy-gtk/empathy-chat-text-view.c | 58 +- libempathy-gtk/empathy-chat.c | 112 +- libempathy-gtk/empathy-chat.h | 2 +- libempathy-gtk/empathy-contact-dialogs.c | 20 +- libempathy-gtk/empathy-contact-list-view.c | 21 +- libempathy-gtk/empathy-contact-widget.c | 14 +- libempathy-gtk/empathy-contact-widget.ui | 12 +- libempathy-gtk/empathy-irc-network-dialog.c | 2 +- libempathy-gtk/empathy-kludge-label.c | 89 - libempathy-gtk/empathy-kludge-label.h | 53 - libempathy-gtk/empathy-log-window.c | 45 +- libempathy-gtk/empathy-log-window.h | 4 +- libempathy-gtk/empathy-new-message-dialog.c | 2 +- libempathy-gtk/empathy-smiley-manager.c | 2 + libempathy-gtk/empathy-status-preset-dialog.c | 4 +- libempathy-gtk/empathy-theme-adium.c | 169 +-- libempathy-gtk/empathy-theme-boxes.c | 11 + libempathy-gtk/empathy-theme-manager.c | 67 +- libempathy-gtk/empathy-theme-manager.h | 1 - libempathy-gtk/empathy-ui-utils.c | 8 +- libempathy-gtk/empathy-ui-utils.h | 4 +- libempathy-gtk/empathy-video-widget.c | 6 +- libempathy/Makefile.am | 3 - libempathy/empathy-account-manager.c | 579 +++-- libempathy/empathy-account-manager.h | 19 +- libempathy/empathy-account-priv.h | 44 - libempathy/empathy-account.c | 575 ----- libempathy/empathy-account.h | 94 - libempathy/empathy-chatroom-manager.c | 21 +- libempathy/empathy-chatroom-manager.h | 11 +- libempathy/empathy-chatroom.c | 20 +- libempathy/empathy-chatroom.h | 11 +- libempathy/empathy-contact-manager.c | 1 - libempathy/empathy-contact.c | 13 +- libempathy/empathy-contact.h | 6 +- libempathy/empathy-dispatch-operation.c | 4 +- libempathy/empathy-dispatch-operation.h | 2 - libempathy/empathy-log-manager.c | 15 +- libempathy/empathy-log-manager.h | 12 +- libempathy/empathy-log-store-empathy.c | 35 +- libempathy/empathy-log-store.c | 12 +- libempathy/empathy-log-store.h | 26 +- libempathy/empathy-message.c | 36 - libempathy/empathy-message.h | 3 - libempathy/empathy-tp-chat.c | 65 +- libempathy/empathy-tp-roomlist.c | 46 +- libempathy/empathy-tp-roomlist.h | 4 +- libempathy/empathy-utils.c | 15 +- libempathy/empathy-utils.h | 1 + megaphone/src/megaphone-applet.c | 30 +- po/LINGUAS | 1 - po/es.po | 469 ++-- po/fr.po | 2103 +++++++----------- po/or.po | 2490 -------------------- po/pa.po | 1060 ++++----- po/sv.po | 2811 ++++++++++++----------- po/ta.po | 561 ++--- python/pyempathy/pyempathy.defs | 137 +- python/pyempathygtk/pyempathygtk.defs | 101 +- src/empathy-accounts-dialog.c | 125 +- src/empathy-accounts-dialog.h | 4 +- src/empathy-call-window-fullscreen.c | 6 +- src/empathy-chat-window.c | 50 +- src/empathy-chat-window.h | 4 +- src/empathy-chatrooms-window.c | 4 +- src/empathy-debug-dialog.c | 4 +- src/empathy-event-manager.c | 11 +- src/empathy-import-dialog.c | 24 +- src/empathy-import-dialog.h | 1 - src/empathy-import-pidgin.c | 1 + src/empathy-main-window.c | 25 +- src/empathy-new-chatroom-dialog.c | 16 +- src/empathy-preferences.c | 161 +- src/empathy-preferences.ui | 44 + src/empathy-sidebar.c | 2 +- src/empathy-status-icon.c | 16 +- src/empathy.c | 24 +- src/ephy-spinner.c | 4 +- tests/check-empathy-chatroom-manager.c | 45 +- tests/check-empathy-chatroom.c | 2 +- tests/check-empathy-helpers.c | 25 +- tests/check-empathy-helpers.h | 6 +- tests/check-helpers.h | 1 + 96 files changed, 4459 insertions(+), 8576 deletions(-)
Sorry for the last fix, that's an error on my part!
Created attachment 144310 [details] [review] Fix in branch fix-581488 http://gitorious.org/~gmillaire/empathy/gmillaires-clone/commits/fix-581488 libempathy-gtk/empathy-chat-text-view.c | 46 ++++++++++++++++++++++++++++++- libempathy-gtk/empathy-smiley-manager.c | 2 + libempathy-gtk/empathy-theme-boxes.c | 11 +++++++ 3 files changed, 58 insertions(+), 1 deletions(-)
Created attachment 144433 [details] [review] Fix a leak in str Fix a leak of str in chat_text_view_copy_clipboard http://gitorious.org/~gmillaire/empathy/gmillaires-clone/commits/fix-581488
Patch merged to master. Thanks!
The string is still leaked! + gtk_clipboard_set_text (clipboard, g_string_free (str, FALSE), str->len); Also, what's that magic number? if (c == 0xFFFC) { Please use a defined macro if any... or at least drop a comment in the code.
Reopening bug
doesn't g_string_free(str, FALSE) free the momory allocated to the string?? 0xFFFC is returned by gtk_text_iter_get_char() if a non-character element, such as an image embedded in the buffer, is found. Can surely add a comment to clarify :)
(In reply to comment #11) > doesn't g_string_free(str, FALSE) free the momory allocated to the string?? Not if you pass FALSE :)
but then i can't use it this wy because with TRUE g_string_free would return NULL... I have to do it in 2 lines?
I see in gtk_text_buffer_get_slice's doc: "Note that 0xFFFC can occur in normal text as well, so it is not a reliable indicator that a pixbuf or widget is in the buffer." For the string, you should just pass str->str, then g_string_free(str, TRUE) in next line.
It's true, I didn't count for the fact that 0xFFFC could be a non-character other than a pixbuf or widget. In my head, if it was not a character, not a pixbuf and not a widget, it was not important to keep in the copy/paste. But if it is important to keep it, I could insert it in the string without problem.
For the 0xFFFC issue, just add a comment. 0xFFFC is the "unknown character" unicode.
Created attachment 144595 [details] [review] Proposed fix for bug Fixed memory leak and added a comment for 0xFFFC http://gitorious.org/~gmillaire/empathy/gmillaires-clone/commits/fix-581488
(In reply to comment #16) > For the 0xFFFC issue, just add a comment. 0xFFFC is the "unknown character" > unicode. No, it's OBJECT REPLACEMENT; it's specifically meant for this usage. I agree with Gabriel's interpretation: it's not useful to copy it to the clipboard.
In my last patch I only added a comment for 0xFFFC. If it's not a widget or a pixbuf, I still don't copy it to the clipboard.
Ok to merge. Thanks for the fixes :D @Daf: Indeed that's the object replacement character, found that too by googling... But in GTK's code there are comments telling it is "unknown character", which is wrong it seems :)
@Gabriel: oh, little typo it seems: "If another non-character, we don't put it in the clipboard" --> "It is a non-character, we don't put it in the clipboard" ?
Created attachment 144789 [details] [review] Fix comment for 0xFFFC Fix comment for 0xFFFC http://gitorious.org/~gmillaire/empathy/gmillaires-clone/commits/fix-581488
Comment on attachment 144789 [details] [review] Fix comment for 0xFFFC Thanks, please commit :)
What do you mean commit?
It is indeed an excess of language: the changes are already committed. He actually meant you can push the changes to the main Empathy repository on git.gnome.org making it an official committed change. :)
Right, it comes from the old days of SVN. Note that bugzilla still have patch status "accepted-commit_now"
Xavier: AFAIK Gabril doesn't have a git access so you should push his patches for him.
I squashed your commits and pushed to master. Thanks!
Xavier: Gabriel: thanks a lot for your work on this bug! Do you think it would be a good candidate for a 2.28 backport? This is a pretty annoying one so it could be good to have it fixed in the stable branch. Oth, I don't know how intrusive/safe is the patch so I let you decide.
I would say it is not really intrusive. It adds a string to widgets, then copy the selected text character by character (instead of copying directly to clipboard), inserting widget and pixbuf text when found...
Cool! Any chance you could cook a branch based on gnome-2.28 backporting the needing patches?
hmmm... how do I do that? :D
You take your branch with your fix, and you git rebase it on gnome-2-28 instead of master.
Actually if you have to use rebase -i and delete all the patches which are not relevant to your fix. Another option is to cherry-pick each commit. If someone knows a better way please let me know. :)
Ok here's the new branch where I squashed the fixes to 581488. Tell me if it's all ok! http://gitorious.org/~gmillaire/empathy/gmillaires-clone/commits/fix-581488-2-28
Your branch doesn't build: empathy-chat-text-view.c: In function ‘chat_text_view_copy_clipboard’: empathy-chat-text-view.c:1217: error: expected ‘;’ before ‘}’ token Are you sure you squashed all the needed commits?
I fixed that in master.
I see. Xavier: could you check if Gabriel's branch looks fine if we add ec9d9d8779bd3e8a18fdd210bc0d317dc120f91e to it?