|Summary:||[GTK] Embedded GtkWidgets are not drawn|
|Product:||WebKit||Reporter:||Dan Vrátil <dvratil>|
|Severity:||Normal||CC:||cgarcia, danilo.cesar, gns, mrobinson, phatina, pnormand, webkit.review.bot, xan.lopez|
|Version:||528+ (Nightly build)|
Description Dan Vrátil 2011-06-27 06:54:20 PDT
Created attachment 98716 [details] A simple test case Embedded widgets that are created in 'create-plugin-widget' signal handler are not drawn in WebKitGTK+. In the webview there is an empty space allocated with proper dimensions, but the widget is not displayed and it does not respond to any events (clicking, moving mouse over it...). When you connect to widget's 'notify' signal, the widget reports it's being correctly resized and 'visibile' is set to TRUE, so WebKit does know about it, it just has some problems displaying/drawing it.
Comment 1 Peter Hatina 2011-07-07 00:19:34 PDT
The issue seems to be caused by not placing embedded widget into any container. If the test case gets updated in this way, everything works - instead of returning only a simple label from the "create_plugin_widget", return a top-level window containing the widget and this window will be shown.
Comment 2 Danilo Cesar Lemes de Paula 2011-09-21 15:53:24 PDT
(In reply to comment #1) > The issue seems to be caused by not placing embedded widget into any container. If the test case gets updated in this way, everything works - instead of returning only a simple label from the "create_plugin_widget", return a top-level window containing the widget and this window will be shown. I tried that approach, however I've only got a new top-level window, and not a embedded widget as expected.
Comment 3 Danilo Cesar Lemes de Paula 2011-09-23 10:09:48 PDT
Source/WebCore/platform/gtk/GtkPluginWidget.cpp::void GtkPluginWidget::paint(GraphicsContext* context, const IntRect& rect) At least in my tests, if (!context->gdkExposeEvent()) is executed, and returning, to the widget will never get painted... I'm investigating, but if you guys find any other information, that would be cool.
Comment 4 Gustavo Noronha (kov) 2011-09-23 10:20:08 PDT
Created attachment 108488 [details] WIP patch This is a simple patch I started writing to make the code gtk3-compliant (i.e. not use gdkExposeEvent) and test this issue. Haven't gotten the widget to show up yet, though.
Comment 5 Dan Vrátil 2012-02-07 04:39:27 PST
Created attachment 125816 [details] Fix Proposed patch that fixes this issue by adding the plugin widget to WebView container and forcing these widgets to be drawn as the very last. Additionally, when GtkPluginWidget is destroyed it also removes the child widget from the WebView container. Otherwise the widgets remain displayed even when a new page is loaded in the webview. This functionality is quite important for the Evolution WebKit port. Is it possible to estimate when this could get to a stable release?
Comment 6 WebKit Review Bot 2012-02-07 10:47:56 PST
Attachment 125816 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/platform/gtk/GtkPluginWidge..." exit_code: 1 Source/WebCore/platform/gtk/GtkPluginWidget.cpp:46: Declaration has space between type name and * in GtkWidget *parent [whitespace/declaration]  Source/WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:486: Extra space before ( in function call [whitespace/parens]  Total errors found: 2 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 7 Martin Robinson 2012-02-07 10:54:40 PST
Comment on attachment 125816 [details] Fix 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]  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.
Comment 8 Martin Robinson 2012-02-07 10:55:13 PST
Oh, by the way, if we commit this patch I'll merge into the stable branch for the next stable release.
Comment 10 WebKit Review Bot 2012-02-08 01:54:24 PST
Attachment 126030 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebKit/gtk/ChangeLog:6: Line contains tab character. [whitespace/tab]  Source/WebKit/gtk/ChangeLog:7: Line contains tab character. [whitespace/tab]  Source/WebCore/ChangeLog:6: Line contains tab character. [whitespace/tab]  Source/WebCore/ChangeLog:7: Line contains tab character. [whitespace/tab]  Total errors found: 4 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 11 Dan Vrátil 2012-02-08 01:54:58 PST
Hi, indeed the paint() method in GtkPluginWidget is not needed anymore. Hopefully it's all OK now.
Comment 12 Dan Vrátil 2012-02-08 01:59:08 PST
Created attachment 126032 [details] Fixed patch Sorry, another try.
Comment 13 Philippe Normand 2012-02-08 02:09:28 PST
Comment on attachment 126032 [details] Fixed patch Attachment 126032 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/11449322
Comment 15 Martin Robinson 2012-02-08 07:35:49 PST
Comment on attachment 126053 [details] Fixed patch View in context: https://bugs.webkit.org/attachment.cgi?id=126053&action=review > Source/WebKit/gtk/webkit/webkitwebview.cpp:672 > + /* Chaining up to the parent forces child widgets to be drawn. */ > + GTK_WIDGET_CLASS(webkit_web_view_parent_class)->draw(widget, cr); > + You removed the ::paint method for GTK+ 2.x, but you did not chain up in webkit_web_view_expose. We use C++ style comments usually. :) No worries though. I will fix those two minor issues and land this patch. Thank you for taking the time to investigate this issue!