[webkit-reviews] review denied: [Bug 45443] [GTK] Support for viewport meta tag : [Attachment 73192] Proposed Patch10
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Nov 9 04:33:01 PST 2010
Gustavo Noronha (kov) <gns at gnome.org> has denied Joone Hur <joone at kldp.org>'s
request for review:
Bug 45443: [GTK] Support for viewport meta tag
https://bugs.webkit.org/show_bug.cgi?id=45443
Attachment 73192: Proposed Patch10
https://bugs.webkit.org/attachment.cgi?id=73192&action=review
------- Additional Comments from Gustavo Noronha (kov) <gns at gnome.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=73192&action=review
> WebKit/gtk/webkit/webkitviewportattributes.cpp:301
> + */
> +void webkit_viewport_attributes_recompute(WebKitWebView* webView)
> +{
> + g_return_if_fail(WEBKIT_IS_WEB_VIEW(webView));
> +
> + GdkScreen* screen = gtk_widget_get_screen(GTK_WIDGET(webView));
> + FloatRect rect = webView->priv->corePage->chrome()->pageRect();
> + ViewportArguments arguments =
webView->priv->corePage->mainFrame()->document()->viewportArguments();
> + // The DesktopWidth has 980, because this value works well for most web
pages designed for desktop user agents.
> + ViewportAttributes attrs = computeViewportAttributes(arguments, 980,
gdk_screen_get_width(screen), gdk_screen_get_height(screen),
webkit_web_view_get_dpi(webView), IntSize(static_cast<gint>(rect.width()),
static_cast<gint>(rect.height())));
> + PlatformRefPtr<WebKitViewportAttributes>
viewportAttributes(adoptPlatformRef(webkit_viewport_attributes_new(attrs,
arguments.userScalable)));
> +
> + g_signal_emit_by_name(webView, "viewport-attributes-changed",
viewportAttributes.get());
I still don't understand your choice of only firing the signal after you have
computed the values. Can you describe to me how you would use this API in a
browser?
> WebKit/gtk/webkit/webkitviewportattributes.h:58
> +WEBKIT_API void
> +webkit_viewport_attributes_recompute (WebKitWebView*
webView);
This should take a WebKitViewPort as the argument, not a web view.
> WebKit/gtk/webkit/webkitwebview.cpp:1394
> -static gdouble webViewGetDPI(WebKitWebView* webView)
> +gdouble webkit_web_view_get_dpi(WebKitWebView* webView)
I suggest keeping the name, using GTK+ style for non-API stuff is actually
discouraged (despite it being everywhere =().
More information about the webkit-reviews
mailing list