[Webkit-unassigned] [Bug 23642] [GTK] Drag and drop support

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon May 25 18:40:53 PDT 2009


https://bugs.webkit.org/show_bug.cgi?id=23642


gns at gnome.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #29786|review?                     |review-
               Flag|                            |




------- Comment #15 from gns at gnome.org  2009-05-25 18:40 PDT -------
(From update of attachment 29786)
Do I understand it correctly that you no rely on that new clipboard
implementation on this patch?

> +        m_dragClipboard->setImage(pixbuf);;

You are leaking pixbuf here, unless m_dragClipboard somehow adopts the initial
reference.

> +    GdkEvent* event = gdk_event_new(GDK_BUTTON_PRESS);
> +    reinterpret_cast<GdkEventButton*>(event)->window = gtk_widget_get_window(GTK_WIDGET(m_webView));
> +    reinterpret_cast<GdkEventButton*>(event)->time = GDK_CURRENT_TIME;
> +
> +    GdkDragContext* context = gtk_drag_begin(GTK_WIDGET(m_webView),
> +                                             targetList, dragAction, 1, event);

I think you are leaking this event.

> +    case WEBKIT_WEB_VIEW_TARGET_INFO_HTML:
> +        data = g_strdup((clipboard->html().isEmpty() || clipboard->html().isNull())
> +                        ? clipboard->text().utf8().data() : clipboard->html().utf8().data());
> +        gtk_selection_data_set(selection_data, selection_data->target, 8,
> +                               reinterpret_cast<guchar*>(data), strlen(data));
> +        g_free(data);

No reason to strdup and free here, just use the pointer.

> +    case WEBKIT_WEB_VIEW_TARGET_INFO_TEXT:
> +        data = g_strdup(clipboard->text().utf8().data());
> +        gtk_selection_data_set(selection_data, selection_data->target, 8,
> +                               reinterpret_cast<guchar*>(data), strlen(data));
> +        g_free(data);

Same here.

Looking at Oliver comments, I think you should clarify the usage of
DragClipboard here. My understanding is that the clipboard you are using is
separate from the one that is exposed to javascript, but I may be wrong. I'll
say r- for now, since I think you still need to update the other patch before
we move forward here.


-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.



More information about the webkit-unassigned mailing list