[Webkit-unassigned] [Bug 53600] [GTK] Add DRT support for modal dialogs

[bugzilla-daemon at webkit.org]

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


Martin Robinson <mrobinson at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #128211|review?                     |review-
               Flag|                            |




--- Comment #24 from Martin Robinson <mrobinson at webkit.org>  2012-02-22 08:36:37 PST ---
(From update of attachment 128211)
View in context: https://bugs.webkit.org/attachment.cgi?id=128211&action=review

One very important thing about this signal is that the client must be able to differentiate between modal dialogs and normal child WebViews. Imagine an application that makes a modal WebView an overlay over an existing one, instead of a new window. 

Is web-view-ready called for modal dialogs? If it is and especially if it's called before run-web-view-as-modal, it's going to be really tricky to pack the modal dialog WebView properly.  There is no create-modal-web-view, like in the Mac port, to know to pack the widget differentily.

r- for now, until these issues are worked out or the documentation explains it. Thank you for updating your patch. I think this can definitely go into stable with just a little polish.

> Source/WebKit/gtk/webkit/webkitwebview.cpp:1690
> +     * made transient for its parent, set as a modal window, and %TRUE should be
> +     * returned in the signal handler. After that, a loop will be created internally
> +     * and run until the @web_view is closed (i.e. its chrome is destroyed) and the
> +     * #WebKitWebView::close-web-view signal is emitted.
> +     *
> +     * Return value: %TRUE to stop handlers from being invoked for the event or
> +     * %FALSE to propagate the event furter
> +     *
> +     * Since: 1.7.90
> +     */

The documentation should make it clear that this is the appropriate time to show the window and not during create-web-view. Another important piece of information is whether or not web-view-ready fires here. You should probably change the Since: line to say "1.8.0," because I'm not sure at what point this patch will be merged into the stable branch.

> Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:1294
>                       "signal::create-web-view", webViewCreate, 0,
> +                     "signal::web-view-run-as-modal", webViewRunModalDialog, 0,
>                       "signal::close-web-view", webViewClose, 0,

The signal name is not really consistent with create-web-view and close-web-view and also doesn't scan as an English phrase. I much prefer run-web-view-as-modal.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.