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

[bugzilla-daemon at webkit.org]

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


xan.lopez at gmail.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |xan.lopez at gmail.com




------- Comment #8 from xan.lopez at gmail.com  2009-03-07 13:07 PDT -------
(In reply to comment #6)
> Created an attachment (id=28124)
 --> (https://bugs.webkit.org/attachment.cgi?id=28124&action=view) [review]
> The clipboard, drag image and other implementations
> 
> Implements drag image manipulation functions, WebCore's clipboard and others.
> (My god, so unwilling for detailed descriptions :)
> 

-DragImageRef createDragImageFromImage(Image*)
+DragImageRef createDragImageFromImage(Image* image)
 {
-    return 0;
+    return image->getGdkPixbuf();
 }

Mmm, is this supposed to return a new reference or not? The name seems to imply
'yes', the function used 'no', although later checking the GdkPixbuf it seems
it does. Maybe it should be 'createGdkPixbuf'? Maybe I should comment in the
other bug... :)

+    } else if (isElementImage) {
+        gtk_target_list_add_image_targets(targetList,
WEBKIT_WEB_VIEW_TARGET_INFO_IMAGE, false);
+    }

No braces here.

+    Element* targetElement =
focusedFrame->document()->elementFromPoint(m_startPos.x(), m_startPos.y());
+    bool isElementLink = false;
+    bool isElementImage = targetElement->renderer()->isImage();

Can elementFromPoint() ever return NULL?

+    ((GdkEventButton*)event)->window =
gtk_widget_get_window(GTK_WIDGET(m_webView));
+    ((GdkEventButton*)event)->time = GDK_CURRENT_TIME;

Better convert this to C++ castings, or better, create a GdkEventButton*
variable.

+    GdkDragContext* context = gtk_drag_begin(GTK_WIDGET(m_webView),
+                                   targetList,
+                                   dragAction,
+                                   1,
+                                   event);

Indentation is broken.

+        gchar* 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, (guchar*)data, strlen(data));

More broken indentation. Also in general I fixnd the whole g{char, int, ...}
stuff pretty useless, and the glib/gtk+ maintainers agree it was a mistake. I
think we should just use char.

+                        8, (guchar*)data, strlen(data));
+

C casting.

Same comments about indentation and C castings and gchar apply in the other
switch branches. Also I think you could declare 'char* data' before the switch
and avoid the braces for new scope inside the branches.


-- 
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.