Comment 42 for bug 69613

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

(From update of attachment 410768)
I like this much better, thank you.

(In reply to comment #37)
> Store() has been removed because gtk_clipboard_set_can_store() is enough.

That would be enough when Gecko is embedded in GTK apps using gtk_main
(because that calls _gtk_clipboard_store_all), but I don't understand how it
would work with a XUL app, which uses lower level event functions.

> GtkClipboardClearFunc is not used because we don't need that (do we?).

GtkClipboardClearFunc provides notification that something else owns the
selection. nsClipboard::SelectionClearEvent called EmptyClipboard which
released the nsITransferable and notified the owner with:

            mSelectionOwner->LosingOwnership(mSelectionTransferable);

This comment makes me suspect that this is important (though comments that say why are more helpful):

>- // XXX make sure to set up the selection_clear event

Nits:

>+ bool ImagesAdded = PR_FALSE;

I'd like avoid mixing types here. Either of the following would be OK:

  PRBool ImagesAdded = PR_FALSE;
  bool ImagesAdded = false;

>+ gint nTargetsNum;

Just |nTargets| or |numTargets|

>+ if(gtk_clipboard_set_with_data(gtkClipboard, gtkTargets, nTargetsNum,

Mozilla style is a space after "if".

>- nsCOMPtr<nsITransferable> trans = GetTransferable(whichClipboard);
>+ nsCOMPtr<nsITransferable> trans = GetTransferable(whichClipboard);

Unnecessary extra whitespace.