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

[bugzilla-daemon at webkit.org]

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





--- Comment #7 from Kenneth Rohde Christiansen <kenneth at webkit.org>  2010-09-27 08:41:58 PST ---
(From update of attachment 68913)
Some comments for now... will look more later

View in context: https://bugs.webkit.org/attachment.cgi?id=68913&action=review

> ChangeLog:8
> +
> +        [GTK] Support for viewport meta tag
> +        https://bugs.webkit.org/show_bug.cgi?id=45443
> +
> +        * GNUmakefile.am: Added webkitviewportconfig.h webkitviewportconfig.cpp 

Would be good with a bit more elaborated ChangeLog

> WebKit/gtk/WebCoreSupport/ChromeClientGtk.cpp:606
> +void ChromeClient::didReceiveViewportArguments(Frame* frame, const WebCore::ViewportArguments& arguments) const

This is going to change soon in another patch, so you might want to hold off for a bit. There is going to be a dispatchViewportDataChanged or similar.

> WebKit/gtk/WebCoreSupport/ChromeClientGtk.cpp:610
> +    WebCore::FloatRect rect = core(webview)->chrome()->pageRect();

Remember that the device size has to be given as in portrait mode. or better said, device width = width in portrait, and device height = width in landscape (this is without any ui elements)

> WebKit/gtk/webkit/webkitviewportconfig.cpp:40
> +struct _WebKitViewportConfigPrivate {
> +    gint width;
> +    gint height;
> +    gfloat targetDensityDPI;
> +    gfloat initialScaleFactor;
> +    gfloat minimumScaleFactor;
> +    gfloat maximumScaleFactor;
> +    gboolean isUserScalable;
> +    gboolean isValid;
> +    gint deviceDPI;
> +    gint deviceWidth;
> +    gint deviceHeight;
> +    gint desktopWidth;
> +};

You might want a way to extend this struct without causing ABI breakage

> WebKit/gtk/webkit/webkitviewportconfig.cpp:56
> +    priv->targetDensityDPI = 0.0;

uh... that doesnt seem right. Maybe you want to use -1.0 for invalid?

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