[Webkit-unassigned] [Bug 53707] Viewport Warning/Error Messages Are Now Inaccurate

[bugzilla-daemon at webkit.org]

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





--- Comment #20 from Kenneth Rohde Christiansen <kenneth at webkit.org>  2011-03-02 13:06:11 PST ---
(In reply to comment #19)
> Before I was just trying to match old behavior, but now that I am forced to
> look deeper at this I think we can make things a little saner.
> 
> These tips are unique in that they need to know the deviceWidth and deviceHeight
> known above WebCore by the WebKit clients. They also need a console to add the
> tip too inside WebCore, but that is a more minor detail.
> 
> Currently the deviceWidth and deviceHeight are not known at parsing time,
> but are known when computeViewportAttributes is called. In Qt by accessing
> environmental variables like getintenv("QTWEBKIT_DEVICE_WIDTH"), and in
> GTK by the width/height of the window rect accessed via
> windowRect(webView->priv->corePage->chrome()->windowRect()).

These are hardcoded in our webkit2 port though.

> I think the ideal solution is to have the deviceWidth and deviceHeight at the
> time that we parse the viewport arguments. I suggest adding an accessor
> on ChromeClient along the lines of:
> 
>   ChromeClient.h:
>   virtual FloatRect viewportDeviceRect() = 0;

Any reason why not just to use a FloatSize?

virtual FloatSize viewportDeviceSize() const = 0; (we want these to be const as well)

Also keep in mind that we are doing a DPI adjustment (with my device currently 1.5, but I guess iPhone 4 uses 2.0 due to the retina display). You will want an accessor for that as well

virtual float viewportDevicePixelRatio() const = 0.

The above is not the best solution, as we currently have one with similar name, which is used to tell the web content what dpi adjustment factor was used (@media all and (-webkit-pixel-ratio))

So it will be better with one like

virtual int viewportDeviceDPI() = 0;

> 
> The advantages of this approach are that:
> 
>   • the tips are created at parse time, like all other viewport warnings/tips/errors
>     so we are safe to report viewport warnings to the console like normal.
>   • we eliminate the possibility that the warning could be printed multiple times
>     because there is nothing that prevents computeViewportAttributes from be
>     called/calculated multiple times.
> 
> 
> While I'm at it, if we still want to keep this tip, I see there is a new keyword for
> "desktop-width". While I haven't investigated this yet, we could also add an
> accessor and a tip for this as well:

The desktop-width is what is used for laying out desktop pages (pages with no viewport).

The iPhone uses 980, IE on Windows Phone 7 uses 1024, and Android had 3 values to choose from (I believe the former plus 800 which I believe is default)

> 
>   ChromeClient.h:
>   virtual float viewportDesktopWidth() = 0;
> 
>   New Tip when width constant matches viewportDesktopWidth():
>   TIP: Viewport width constant matches desktop width, try using the "desktop-width" constant instead for future compatibility.
> 
> 
> Finally, is the following really something we want to encourage?
> 
>   <meta name="viewport" content="width:device-height">

Nope because that is not a valid viewport meta tag :-) I guess you want a = instead of :

Anyway, I'm not sure and it is currently supported, so why not?

> I was just thinking about matching old behavior, but I think we should only
> tip about "device-width" for width, and "device-height" for height.

I do not feel strong about this.

> Do these suggestions sound good to you? I'll start a patch for the above, but
> as you've seen I'm not the best with other ports =).

I think it sounds fine.

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