[Webkit-unassigned] [Bug 19130] [GTK] ChromeClient::createWindow and friends need to be implemented
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Jun 24 09:42:54 PDT 2008
https://bugs.webkit.org/show_bug.cgi?id=19130
christian at imendio.com changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #21902|review? |review-
Flag| |
------- Comment #18 from christian at imendio.com 2008-06-24 09:42 PDT -------
(From update of attachment 21902)
>-void ChromeClient::setWindowRect(const FloatRect& r)
>+void ChromeClient::setWindowRect(const FloatRect& rect)
> {
>- notImplemented();
>+ if (!m_webView)
>+ return;
>+
>+ GtkWidget* window = gtk_widget_get_toplevel(GTK_WIDGET(m_webView));
>+ if (window) {
>+ IntRect intrect = IntRect(rect);
>+ WebKitWebWindowFeatures* webWindowFeatures = webkit_web_view_get_window_features(m_webView);
>+
>+ g_object_set(G_OBJECT(webWindowFeatures),
>+ "x", intrect.x(),
>+ "y", intrect.y(),
>+ "width", intrect.width(),
>+ "height", intrect.height(),
>+ NULL);
>+ }
What is the purpose of that "if (window) {" check? I have actually missed that
in the last patch. It doesn't have any effect, from what I can see.
>+ if (!webView)
> return 0;
>- } else {
>- /* TODO: FrameLoadRequest is not used */
>- WebKitWebView* webView = WEBKIT_WEB_VIEW_GET_CLASS(m_webView)->create_web_view(m_webView);
>- if (!webView)
>- return 0;
>-
>- WebKitWebViewPrivate* privateData = WEBKIT_WEB_VIEW_GET_PRIVATE(webView);
>- return privateData->corePage;
>- }
>+
>+ WebKitWebViewPrivate* privateData = WEBKIT_WEB_VIEW_GET_PRIVATE(webView);
>+ return privateData->corePage;
Make that "return core(webView);" please.
>@@ -67,6 +69,7 @@ extern "C" {
> struct _WebKitWebViewPrivate {
> WebCore::Page* corePage;
> WebKitWebSettings* webSettings;
>+ WebKitWebWindowFeatures* webWindowFeatures;
>
> WebKitWebFrame* mainFrame;
> WebCore::String applicationNameForUserAgent;
>@@ -110,6 +113,9 @@ extern "C" {
> WebKitWebHistoryItem*
> webkit_web_history_item_new_with_core_item(WebCore::HistoryItem*);
>
>+ void
>+ webkit_web_view_show (WebKitWebView* webView);
>+
> // FIXME: Move these to webkitwebframe.h once their API has been discussed.
>
> WEBKIT_API GSList*
What is the idea here? Please decide if you want to keep this private for now,
by moving it below the FIXME comment, or move it to webkitwebview.h. :)
>@@ -123,6 +129,12 @@ extern "C" {
>
> WEBKIT_API gchar*
> webkit_web_view_get_selected_text (WebKitWebView* web_view);
>+
>+ WEBKIT_API WebKitWebWindowFeatures*
>+ webkit_web_window_features_new_from_core_features (const WebCore::WindowFeatures& features);
This is also misplaced. We will *never* expose WebCore interfaces. So please
move this above the comment and remove the WEBKIT_API macro.
>+ * WebKitWebView::create-web-view:
>+ * @web_view: the object on which the signal is emitted
>+ * @frame: the #WebKitWebFrame
>+ * @window_features: a #WebKitWebWindowFeaturest
>+ * @network_request: a #WebKitNetworkRequest
>+ * @return: a newly allocated #WebKitWebView or %NULL
>+ *
>+ * Emitted when the creation of a new window is requested.
>+ * If this signal is handled the signal handler should return the
>+ * newly created #WebKitWebView. The new #WebKitWebView should not be
>+ * displayed to the user until the "show-web-view" signal is emitted.
>+ *
>+ * The signal handlers should not try to deal with the reference
>+ * count for the new #WebKitWebView. The widget to which the
>+ * widget is added will handle that.
>+ *
>+ * @web_view_request contains information about the new #WebKitWebView,
>+ * but note that you can freely ignore any particular request like the
>+ * size of the new window or what bars should be displayed in the new
>+ * window.
>+ */
>+ webkit_web_view_signals[CREATE_WEB_VIEW] = g_signal_new("create-web-view",
>+ G_TYPE_FROM_CLASS(webViewClass),
>+ (GSignalFlags)(G_SIGNAL_RUN_LAST | G_SIGNAL_ACTION),
>+ G_STRUCT_OFFSET (WebKitWebViewClass, create_web_view),
>+ webkit_create_web_view_request_handled,
>+ NULL,
>+ webkit_marshal_OBJECT__OBJECT_OBJECT_OBJECT,
>+ G_TYPE_OBJECT, 3,
>+ WEBKIT_TYPE_WEB_FRAME,
>+ WEBKIT_TYPE_WEB_WINDOW_FEATURES,
>+ WEBKIT_TYPE_NETWORK_REQUEST);
According to the documentation the return type needs to be a
WEBKIT_TYPE_WEB_VIEW (or a subclass). So that type should also be specified in
code.
>+ * WebKitWebView::show-web-view:
>+ * @web_view: the object on which the signal is emitted
>+ * @return: %TRUE to stop other handlers from being invoked for the event, %FALSE to propagate the event further
>+ *
>+ * Emitted after "create-web-view" when the new #WebKitWebView should
>+ * be displayed to the user.
>+ *
>+ * If there isn't any handler for this signal or if they return %FALSE,
>+ * then the default action is to call gtk_widget_show_all() on the
>+ * toplevel window containing @web_view.
The documentation needs to follow the implementation. ;)
>+void webkit_web_view_show(WebKitWebView* webView)
>+{
>+ g_return_if_fail(WEBKIT_IS_WEB_VIEW(webView));
>+
>+ gboolean isHandled = FALSE;
>+ g_signal_emit(webView, webkit_web_view_signals[SHOW_WEB_VIEW], 0, &isHandled);
>+ if (isHandled)
>+ return;
>+
>+ GtkWidget* toplevel = gtk_widget_get_toplevel(GTK_WIDGET(webView));
>+ if (toplevel)
>+ gtk_widget_show(toplevel);
>+ gtk_widget_show(GTK_WIDGET(webView));
I'm not sure how useful that behaviour is but I could go with it if someone
likes it.
Note that "if (toplevel)" is wrong and must be "GTK_WIDGET_TOPLEVEL(toplevel)"
(see the documentation of gtk_widget_get_toplevel in the Gtk+ API reference)
>+ g_object_class_install_property(gobject_class,
>+ PROP_TOOLBAR_VISIBLE,
>+ g_param_spec_boolean(
>+ "toolbar-visible",
>+ "Toolbar Visible",
>+ "Controls whether the tool bar should be visible for the window.",
>+ TRUE,
>+ flags));
>+ [snip]
Thanks for fixing the property names. Now this is might be nit picking but I
would really like to see the one word style in blurbs, too, for the sake of
consistency and avoiding confusion about API.
I like this version much better than the previous one. I will try to find some
time to try this out in midori.
--
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.
More information about the webkit-unassigned
mailing list