[Webkit-unassigned] [Bug 45443] [GTK] Support for viewport meta tag

[bugzilla-daemon at webkit.org]

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


Martin Robinson <mrobinson at webkit.org> changed:

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




--- Comment #42 from Martin Robinson <mrobinson at webkit.org>  2010-10-26 18:43:16 PST ---
(From update of attachment 71716)
View in context: https://bugs.webkit.org/attachment.cgi?id=71716&action=review

> WebKit/gtk/webkit/webkitviewportattributes.cpp:63
> +    /* viewport properties */

I think the comments in this structure can be removed. They don't add much extra information.

> WebKit/gtk/webkit/webkitviewportattributes.cpp:159
> +gfloat webkit_viewport_attributes_get_initial_scale_factor(WebKitViewportAttributes *viewportAttributes)

Asterisk should be next to the type name.

> WebKit/gtk/webkit/webkitviewportattributes.cpp:175
> +gfloat webkit_viewport_attributes_get_minimum_scale_factor(WebKitViewportAttributes *viewportAttributes)

Ditto.

> WebKit/gtk/webkit/webkitviewportattributes.cpp:191
> +gfloat webkit_viewport_attributes_get_maximum_scale_factor(WebKitViewportAttributes *viewportAttributes)

Ditto.

> WebKit/gtk/webkit/webkitviewportattributes.cpp:207
> +gfloat webkit_viewport_attributes_get_device_pixel_ratio(WebKitViewportAttributes *viewportAttributes)

Ditto.

> WebKit/gtk/webkit/webkitviewportattributes.cpp:223
> +gboolean webkit_viewport_attributes_get_is_user_scalable(WebKitViewportAttributes *viewportAttributes)

Asterisk should be next to the type.

> WebKit/gtk/webkit/webkitviewportattributes.cpp:239
> +gboolean webkit_viewport_attributes_get_is_valid(WebKitViewportAttributes *viewportAttributes)

Ditto.

> WebKit/gtk/webkit/webkitviewportattributes.cpp:253
> +/**
> + * webkit_viewport_attributes_recompute:
> + * @viewportAttributes: a #WebKitViewportAttributes
> + *
> + * Recompute the optimal viewport attributes when the size of the visible viewport is changed. 
> + *
> + * Since: 1.3.5
> + */
> +void webkit_viewport_attributes_recompute(WebKitViewportAttributes *viewportAttributes)

Ditto. I'm not sure this is clear as to why you'd want to call webkit_viewport_attributes_recompute. If the UA is supposed to call this when the size of the device viewport has changed, I think the comment should say that explicitly.

> WebKit/gtk/webkit/webkitviewportattributes.h:78
> +webkit_viewport_attributes_get_device_pixel_ratio   (WebKitViewportAttributes *viewportAttributes);
> +
> +WEBKIT_API gboolean 
> +webkit_viewport_attributes_get_is_user_scalable     (WebKitViewportAttributes *viewportAttributes);
> +
> +WEBKIT_API gboolean

Ditto.

> WebKit/gtk/webkit/webkitwebview.cpp:3158
> +    priv->viewportAttributes = adoptPlatformRef(webkit_viewport_attributes_new(priv->corePage, 980, gdk_screen_get_width(screen), gdk_screen_get_height(screen), webViewGetDPI(webView)));

Why is this initialized to 980 here? There should at least be a comment explaining it.

> WebKit/gtk/webkit/webkitwebview.cpp:4533
> +void webkit_web_view_init_viewport_attributes(WebKitWebView* webView, gint desktopWidth, gint deviceWidth, gint deviceHeight, gint deviceDPI)

As I mentioned on IRC, it's my belief that we should wait for the DumpRenderTreeSupport code to land and have these private methods there.

> WebKitTools/DumpRenderTree/gtk/LayoutTestControllerGtk.cpp:837
> +    webkit_web_view_init_viewport_attributes(webView, 980, 320, 480, 160);
> +    WebKitViewportAttributes* viewportAttributes = webkit_web_view_get_viewport_attributes(webView);
> +    webkit_viewport_attributes_compute_internal(viewportAttributes, availableWidth, availableHeight);
> +    fprintf(stdout, "viewport size %dx%d scale %f with limits [%f, %f]\n",

Shouldn't webkit_web_view_init_viewport_attributes call webkit_viewport_attributes_compute_internal itself? Is there a situation where you'd want to call webkit_web_view_init_viewport_attributes without immediately calling webkit_viewport_attributes_compute_internal? Is it possible to move webkit_viewport_attributes_compute_internal into the body of webkit_viewport_attribute_recompute and have webkit_web_view_init_viewport_attributes call webkit_viewport_attribute_recompute?

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