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

[bugzilla-daemon at webkit.org]

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


Gustavo Noronha (kov) <gns at gnome.org> changed:

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




--- Comment #28 from Gustavo Noronha (kov) <gns at gnome.org>  2009-07-14 14:17:48 PDT ---
(From update of attachment 32375)
> +//    gtk_target_list_unref(targetList);
>  }

I don't like adding commented code. It seems to me like we need to keep track
of both targetList and context, to make sure they go away when the drag
operation ends. Should they be added to the private struct?

> +namespace WebCore {
> +    class IntPoint;
> +}

Maybe make this class WebCore::IntPoint; to avoid using more lines than
necessary? =)

>  namespace WebKit {
> +

I think there are some unecesary empty line additions in the patch. Some, like
the one bellow are OK, but I think the others should be removed:

>      public:
> +        DragClient(WebKitWebView*);
> +

> +        private:
> +            WebKitWebView* m_webView;
> +
> +            WebCore::IntPoint m_startPos;

Like this one.

> +static void webkit_web_view_drag_end(GtkWidget* widget, GdkDragContext* context)
> +{
> +    DragClipboard::clipboard()->resetClipboard();
> +}

This is where we want to free context and targetList, I believe.

> +    fprintf(stderr, "webkit_web_view_drag_data_get %d\n", info);

Debugging that should go away?

The rest looks fine. I think if you modify the code to not leak targetList and
context we should be good to go with this one =). r- for now.

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