[webkit-reviews] review denied: [Bug 27792] [GTK] new-window-policy-decision-requested provides no information about the target frame : [Attachment 33715] Updated patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Jul 30 05:04:25 PDT 2009


Gustavo Noronha (kov) <gns at gnome.org> has denied Martin Robinson
<martin.james.robinson at gmail.com>'s request for review:
Bug 27792: [GTK] new-window-policy-decision-requested provides no information
about the target frame
https://bugs.webkit.org/show_bug.cgi?id=27792

Attachment 33715: Updated patch
https://bugs.webkit.org/attachment.cgi?id=33715&action=review

------- Additional Comments from Gustavo Noronha (kov) <gns at gnome.org>
> + * Since: 1.1.12

Should be 1.1.13, now =).

> +void
webkit_web_navigation_action_set_target_frame(WebKitWebNavigationAction*
navigationAction, const gchar* targetFrame)
> +{
> +    g_return_if_fail(WEBKIT_IS_WEB_NAVIGATION_ACTION(navigationAction));

No need for this check, after making it static.

> --- a/WebKitTools/ChangeLog
> +++ b/WebKitTools/ChangeLog
> @@ -1,3 +1,14 @@
> +2009-07-29  Martin Robinson	<martin.james.robinson at gmail.com>
> +
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	   [GTK] new-window-policy-decision-requested provides no information
about the target frame
> +	   https://bugs.webkit.org/show_bug.cgi?id=27792
> +
> +	   * GtkLauncher/main.c:
> +	   (policy_callback):
> +	   (create_browser):
> +

The ChangeLog entry should be in WebKit/gtk/ChangeLog, and list the correct
funtions =).

I'm OK with the approach and with the API additions. I'll say r-, though,
because I see too many nits to be addressed, and because of the ChangeLog
issue, but feel free to r+ the next patch, Jan, or Xan.


More information about the webkit-reviews mailing list