[webkit-reviews] review denied: [Bug 63451] Embedded widgets are not drawn : [Attachment 125816] Fix

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Feb 7 10:54:39 PST 2012


Martin Robinson <mrobinson at webkit.org> has denied  review:
Bug 63451: Embedded widgets are not drawn
https://bugs.webkit.org/show_bug.cgi?id=63451

Attachment 125816: Fix
https://bugs.webkit.org/attachment.cgi?id=125816&action=review

------- Additional Comments from Martin Robinson <mrobinson at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=125816&action=review


Great work! If you attach a proper ChangeLog to this patch and fix the nits
below, we can commit it directly from the bug. See
http://www.webkit.org/coding/contributing.html. I think you can skip writing
tests for this change...just delete that line from the ChangeLog.

Do you mind testing what happens when you remove the contents of the
GtkPluginWidget::paint method entirely? If the real expose event is drawing the
child widgets, perhaps we don't have to make fake expose events at all.

> Source/WebCore/platform/gtk/GtkPluginWidget.cpp:49
> +    GtkWidget *parent;
> +
> +    parent = gtk_widget_get_parent(platformWidget());
> +    gtk_container_remove(GTK_CONTAINER(parent), platformWidget());

This should just be one line:

gtk_container_remove(GTK_CONTAINER(gtk_widget_get_praent(platformWidget()),
platformWidget());

>> Source/WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:486
>> +	    gtk_container_add (GTK_CONTAINER (getViewFromFrame(m_frame)),
gtkWidget);
> 
> Extra space before ( in function call  [whitespace/parens] [4]

Please fix the style issue here.

> Source/WebKit/gtk/webkit/webkitwebview.cpp:664
> +    /* Force child widgets to be drawn as last */

Might want to expand this comment a bit:

// Chaining up to the parent forces child widgets to be drawn.


More information about the webkit-reviews mailing list