[Webkit-unassigned] [Bug 19130] [GTK] ChromeClient::createWindow and friends need to be implemented
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Jun 20 12:34:39 PDT 2008
https://bugs.webkit.org/show_bug.cgi?id=19130
christian at imendio.com changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #21464|review? |review-
Flag| |
------- Comment #15 from christian at imendio.com 2008-06-20 12:34 PDT -------
(From update of attachment 21464)
>+ 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(),
>+ NULL);
>+
>+ g_object_set(G_OBJECT(webWindowFeatures),
>+ "y", intrect.y(),
>+ NULL);
>+
>+ g_object_set(G_OBJECT(webWindowFeatures),
>+ "width", intrect.width(),
>+ NULL);
>+
>+ g_object_set(G_OBJECT(webWindowFeatures),
>+ "height", intrect.height(),
>+ NULL);
>+ }
That should be one g_object_set instead of four.
>+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));
>+ gtk_widget_show_all(toplevel);
>+}
You should never gtk_widget_show_all on code you did not write. Plus you cannot
know if there is a toplevel, so you need to check that first. A gtk_widget_show
might be okay.
>+static void webkit_web_view_request_init(WebKitWebViewRequest* request)
>+{
>+ request->priv = WEBKIT_WEB_VIEW_REQUEST_GET_PRIVATE(request);
>+
>+ WebKitWebViewRequestPrivate* priv = request->priv;
>+ priv->name = g_strdup("");
>+ priv->webWindowFeatures = webkit_web_window_features_new();
>+}
Why is name initially "" as opposed to NULL?
>+ g_object_class_install_property(objectClass, PROP_NAME,
>+ g_param_spec_string("name",
>+ "New window name",
>+ "The name of the new window",
>+ "",
>+ WEBKIT_PARAM_READWRITE));
I'm not sure about name. It's the name of a "frame" or "window" in HTML terms,
so it might be better to name it accordingly.
>+WEBKIT_API GType
>+webkit_web_view_request_get_type(void);
>+
>+WEBKIT_API WebKitNetworkRequest*
>+webkit_web_view_request_get_network_request(WebKitWebViewRequest* request);
>+
>+WEBKIT_API const gchar*
>+webkit_web_view_request_get_name(WebKitWebViewRequest* request);
>+
>+WEBKIT_API void
>+webkit_web_view_request_set_name(WebKitWebViewRequest* request, const gchar* name);
>+
>+WEBKIT_API WebKitWebWindowFeatures*
>+webkit_web_view_request_get_window_features(WebKitWebViewRequest* request);
>+
>+WEBKIT_API void
>+webkit_web_view_request_set_window_features(WebKitWebViewRequest* request, WebKitWebWindowFeatures* windowFeatures);
WebViewRequest might be confusing, looking at NetworkRequest. However I'm not
quite clear yet as for how this class fits in the interface. It might be less
confusing in a general context.
>+enum {
>+ PROP_0,
>+
>+ PROP_X,
>+ PROP_Y,
>+ PROP_WIDTH,
>+ PROP_HEIGHT,
>+ PROP_TOOLBAR_VISIBLE,
>+ PROP_STATUSBAR_VISIBLE,
>+ PROP_SCROLLBAR_VISIBLE,
>+ PROP_MENUBAR_VISIBLE,
>+ PROP_LOCATIONBAR_VISIBLE,
>+ PROP_RESIZABLE,
>+ PROP_FULLSCREEN,
>+ PROP_DIALOG
If I remember correctly one of these became meaningless at some point, in the
sense that it causes no error in javascript but is never honored. I think it
probably was "resizable" but I cannot find the bug number right now.
>+ g_object_class_install_property(gobject_class,
>+ PROP_X,
>+ g_param_spec_int(
>+ "x",
>+ "x",
>+ "The starting x position of the window on the screen.",
>+ -1,
>+ G_MAXINT,
>+ -1,
>+ flags));
Why is the lower limit -1? I don't think this is in the ECMA spec.
>+ g_object_class_install_property(gobject_class,
>+ PROP_FULLSCREEN,
>+ g_param_spec_boolean(
>+ "fullscreen",
>+ "Full-screen",
>+ "Controls whether window will be displayed fullscreen.",
>+ FALSE,
>+ flags));
All the properties have non-matching name and nick. The spelling must be the
same, merely the case is different, for example "fullscreen" and "Fullscreen"
in this case.
>+ g_object_class_install_property(gobject_class,
>+ PROP_DIALOG,
>+ g_param_spec_boolean(
>+ "dialog",
>+ "Dialog",
>+ "Controls whether window should be a dialog, or a top level window.",
>+ FALSE,
>+ flags));
What is the exact use case of this? Is this actually something that websites
make use of/ is in a spec/ has any other relevance?
>+ g_object_set(G_OBJECT(webWindowFeatures),
>+ "toolbar_visible", features.toolBarVisible,
>+ "statusbar_visible", features.statusBarVisible,
>+ "scrollbar_visible", features.scrollbarsVisible,
>+ "menubar_visible", features.menuBarVisible,
>+ "locationbar_visible", features.locationBarVisible,
>+ "resizable", features.resizable,
>+ "fullscreen", features.fullscreen,
>+ "dialog", features.dialog,
>+ NULL);
Please use hyphens for property and signal names, like "toolbar-visible"
instead of "toolbar_visible".
--
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