Bug 173421 - REGRESSION(r218325): [GTK] Runtime critical warnings running layout tests that show a popup menu
Summary: REGRESSION(r218325): [GTK] Runtime critical warnings running layout tests tha...
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
Keywords: Gtk, Regression
Depends on:
Reported: 2017-06-15 08:51 PDT by Carlos Garcia Campos
Modified: 2017-06-15 23:03 PDT (History)
7 users (show)

See Also:

Patch (1.68 KB, patch)
2017-06-15 08:54 PDT, Carlos Garcia Campos
clopez: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Garcia Campos 2017-06-15 08:51:56 PDT
(WebKitTestRunner:15339): GLib-GObject-WARNING **: invalid cast from 'WebKitWebViewBase' to 'WebKitWebView'

(WebKitTestRunner:15339): GLib-GObject-CRITICAL **: g_signal_emit_valist: assertion 'signal_id > 0' failed

In r218325 I forgot to create a WebKitPopupMenu or WebPopupMenuProxyGtk depending on whether the view is a WebKitWebView or not.
Comment 1 Carlos Garcia Campos 2017-06-15 08:53:07 PDT
This only affect WTR.
Comment 2 Carlos Garcia Campos 2017-06-15 08:54:22 PDT
Created attachment 312986 [details]
Comment 3 Build Bot 2017-06-15 08:55:59 PDT
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Comment 4 Adrian Perez 2017-06-15 09:19:54 PDT
Comment on attachment 312986 [details]

Informally reviewing: r+

Was this somehow not caught by the tests? From the ChangeLog description
it sounds that the tests still pass despite the critical warnings.
Comment 5 Michael Catanzaro 2017-06-15 11:23:43 PDT
Comment on attachment 312986 [details]

View in context: https://bugs.webkit.org/attachment.cgi?id=312986&action=review

Any warning ought to cause tests to fail :/ but it probably doesn't work that way currently.

> Source/WebKit2/UIProcess/API/gtk/PageClientImpl.cpp:211
> +    if (WEBKIT_IS_WEB_VIEW(m_viewWidget))
> +        return WebKitPopupMenu::create(m_viewWidget, page);
> +    return WebPopupMenuProxyGtk::create(m_viewWidget, page);

It merits a comment to explain the difference, don't you think?
Comment 6 Carlos Garcia Campos 2017-06-15 23:03:29 PDT
Committed r218382: <http://trac.webkit.org/changeset/218382>