[Webkit-unassigned] [Bug 26718] [Gtk] Add support for javascript windows for DRT

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Jun 25 22:32:12 PDT 2009


https://bugs.webkit.org/show_bug.cgi?id=26718


jmalonzo at gmail.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |xan.lopez at gmail.com




------- Comment #3 from jmalonzo at gmail.com  2009-06-25 22:32 PDT -------
> >  
> >  void ChromeClient::closeWindowSoon()
> >  {
> > -    notImplemented();
> > +    webkit_web_view_set_group_name(m_webView, "");
> 
> What's this for?



> 
> > +    webkit_web_view_stop_loading(m_webView);
> > +
> > +    g_signal_emit_by_name(m_webView, "close-web-view");
> >  }
> 
> In some cases it's good to have a way to ask the user if he wants to cancel the
> closing action (ie, return TRUE to prevent closing). For example, the case were
> forms are modified and not submitted when closing a window can be handled in
> two ways I guess: 'close-web-view', call view_has_modified_forms, if it has
> show a dialog asking, return what the user wants to close-request. Another
> option, I guess, is to just have higher level APIs for this
> ('close-with-modified-forms' signal).
>
> Not sure what makes most sense here, but just commenting :)
>

I'll add a check if it's handled to give clients a way to do whatever before
closing. Thanks :)

> 
> > +    /**
> > +    * WebKitWebSettings:javascript-can-open-windows
> > +    *
> > +    * Whether JavaScript can open windows automatically.
> > +    *
> > +    * Since 1.1.11
> > +    */
> > +    g_object_class_install_property(gobject_class,
> > +                                    PROP_JAVASCRIPT_CAN_OPEN_WINDOWS,
> > +                                    g_param_spec_boolean("javascript-can-open-windows",
> > +                                                         _("JavaScript can open windows automatically"),
> > +                                                         _("Whether JavaScript can open windows automatically"),
> > +                                                         FALSE,
> 
> Mmm, are we sure FALSE is the right default?

It's defaulted to FALSE in WebCore::Settings that's why it's also FALSE here.
Maybe we should set it to TRUE and let applications decide if they don't really
want it.

> 
> >      /**
> > +     * WebKitWebView::close-web-view:
> > +     * @web_view: the object on which the signal is emitted
> > +     *
> > +     * Emitted when closing a window is requested.
> 
> So, I guess you mean view here and not window? Is this signal emitted when the
> widget is destroyed?

Should be view. This gets emitted when window.close() is called.

> 
> > @@ -132,8 +136,10 @@ static gchar* dumpFramesAsText(WebKitWebFrame* frame)
> >  
> >      if (gLayoutTestController->dumpChildFramesAsText()) {
> >          GSList* children = webkit_web_frame_get_children(frame);
> > -        for (GSList* child = children; child; child = g_slist_next(child))
> > -           appendString(result, dumpFramesAsText((WebKitWebFrame*)children->data));
> > +        for (unsigned i = 0; i < g_slist_length(children); ++i) {
> > +            WebKitWebFrame* childFrame = static_cast<WebKitWebFrame*>(g_slist_nth_data(children, i));
> > +            appendString(result, dumpFramesAsText(childFrame));
> > +        }
> 
> Mmm, why this?

This is a fix to get all the unskipped tests in this patch work. This seemed to
fix it, but I think I know the reason why. I'll update.

> > +
> > +    // Also check if we still have opened webViews and free them.
> > +    if (gLayoutTestController->closeRemainingWindowsWhenComplete() || webViewList) {
> 
> Shouldn't that be an && instead of ||? Or am I missunderstaning something.

No. The webViewList check is just so we cleanup properly. Some tests may have
forgotten to call closeRemainingWindowsWhenComplete so this just makes sure we
free those WebViews.

> 
> > +        while (webViewList) {
> > +            g_object_unref(WEBKIT_WEB_VIEW(webViewList->data));
> > +            webViewList = g_slist_next(webViewList);
> > +        }
> > +        g_slist_free(webViewList);
> > +        webViewList = 0;
> > +    }
> > +
> 
> > +static WebKitWebView* webViewCreate(WebKitWebView*, WebKitWebFrame*);
> > +
> > +static WebKitWebView* createWebView()
> > +{
> > +    WebKitWebView* view = WEBKIT_WEB_VIEW(webkit_web_view_new());
> > +    webkit_web_view_set_group_name(view, "org.webkit.gtk.DumpRenderTree");
> > +
> > +    g_signal_connect(G_OBJECT(view), "load-started", G_CALLBACK(webViewLoadStarted), 0);
> > +    g_signal_connect(G_OBJECT(view), "load-finished", G_CALLBACK(webViewLoadFinished), 0);
> > +    g_signal_connect(G_OBJECT(view), "window-object-cleared", G_CALLBACK(webViewWindowObjectCleared), 0);
> > +    g_signal_connect(G_OBJECT(view), "console-message", G_CALLBACK(webViewConsoleMessage), 0);
> > +    g_signal_connect(G_OBJECT(view), "script-alert", G_CALLBACK(webViewScriptAlert), 0);
> > +    g_signal_connect(G_OBJECT(view), "script-prompt", G_CALLBACK(webViewScriptPrompt), 0);
> > +    g_signal_connect(G_OBJECT(view), "script-confirm", G_CALLBACK(webViewScriptConfirm), 0);
> > +    g_signal_connect(G_OBJECT(view), "title-changed", G_CALLBACK(webViewTitleChanged), 0);
> > +    g_signal_connect(G_OBJECT(view), "navigation-policy-decision-requested", G_CALLBACK(webViewNavigationPolicyDecisionRequested), 0);
> > +    g_signal_connect(G_OBJECT(view), "status-bar-text-changed", G_CALLBACK(webViewStatusBarTextChanged), 0);
> > +    g_signal_connect(G_OBJECT(view), "create-web-view", G_CALLBACK(webViewCreate), 0);
> > +    g_signal_connect(G_OBJECT(view), "close-web-view", G_CALLBACK(webViewClose), 0);
> 
> While you are at it, g_signal_connect does not take a GObject as first
> parameter, so no need to cast. And you could use g_object_connect.

Ok, thanks for that. I'll post an update.
> 


-- 
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