Comment 90 for bug 21202

Revision history for this message
In , Mozbugz (mozbugz) wrote :

(From update of attachment 407846)
What makes things awkward here is that Mozilla is using low level selection
functions and signals on its own GtkWidget mWidget to set data on the
clipboard, but gtk_clipboard_store() only works on higher level GtkClipboards
which have their own GtkWidget. There is no way to associate Mozilla's
selections on its own GtkWidget with GtkClipboards.

The patch here works around that (on quit) by changing the selection owner
from nsClipboard's mWidget to a GtkClipboard and adds code to convert the
nsITransferable to GtkClipboard format.

I'm not so enthusiastic though about having yet another place where
nsITransferables are converted to GtkSelectionData (and targets).

The conversion in nsClipboard::Store() of this patch does convert from
text/unicode _or_ images, but doesn't convert both text _and_ images (for
which support was added in bug 518249) and doesn't convert to other mime types
such as text/html (and maybe text/uri-list and text/plain, if they get used).

Also, the Clipboard Manager Specification says "Clients which support the
SAVE_TARGETS mechanism should announce this by listing SAVE_TARGETS as a
target for the CLIPBOARD", but with this patch that target only gets added to
the clipboard briefly on quit. I assume the intention is that the
SAVE_TARGETS target informs the clipboard manager that it doesn't need to
preemptively convert the clipboard selection whenever this app changes it.

I think the best solution is to make nsClipboard use GtkClipboards for setting
data. (It already uses GtkClipboard for getting data.)

nsClipboard::SetData() would use gtk_clipboard_set_with_data() and
gtk_clipboard_set_can_store() instead of gtk_selection_owner_set(),
gtk_selection_clear_targets(), and gtk_selection_add_target(s)().
(m*Owner and m*Transferable need to be set after gtk_clipboard_set_with_data()
as the GtkClipboardClearFunc will erase them.)

nsClipboard would no longer need mWidget. invisible_selection_get_cb() and
selection_clear_event_cb() would be replaced with the similar callbacks for
gtk_clipboard_set_with_data().

nsClipboard::SelectionGetEvent() would be similar, except the unused arguments
would be removed.

nsClipboard::SelectionClearEvent() would get a GtkClipboard instead of a
GdkEventSelection. That means the aEvent->selection is no longer available,
but the type of selection can be determined by comparing the GtkClipboard*
with gtk_clipboard_get(GDK_SELECTION_CLIPBOARD or GDK_SELECTION_PRIMARY).

nsClipboard::Store() would simply just call gtk_clipboard_store() and
nsClipboard::SelectionGetEvent() would be used for the conversion.

Little nits:

>+#define APP_QUIT "quit-application"

>+ os->AddObserver(this, "quit-application", PR_FALSE);

>+ if (strcmp(aTopic, APP_QUIT) == 0) {

I'd like each of these topics to be represented consistently.

As "quit-application" is only used twice, I'd suggest just using the string
literal each time.

>+ if (aTransferable == nsnull)

Mozilla style is "!aTransferable".
https://developer.mozilla.org/En/Mozilla_Coding_Style_Guide

>+ // Save global clipboard content to gtk
>+ nsresult Store (void);
>
> private:

Let's make Store() private unless you have other uses in mind.