Bug 103068

Summary: Viewport CSS rules should not clamp values like Viewport META
Product: WebKit Reporter: Thiago Marcos P. Santos <tmpsantos>
Component: CSSAssignee: Thiago Marcos P. Santos <tmpsantos>
Status: RESOLVED FIXED    
Severity: Normal CC: ddkilzer, joepeck, kenneth, ojan, simon.fraser, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 95959    
Attachments:
Description Flags
Patch
none
Patch none

Description Thiago Marcos P. Santos 2012-11-22 07:09:28 PST
Currently we are clamping values like width and height following the rules for META tags (1px - 10000px) which are not valid for @viewport.

http://dev.w3.org/csswg/css-device-adapt/#translation-into-atviewport-descriptors
Comment 1 Thiago Marcos P. Santos 2012-11-23 04:58:38 PST
Created attachment 175784 [details]
Patch
Comment 2 Kenneth Rohde Christiansen 2012-11-23 05:04:37 PST
Comment on attachment 175784 [details]
Patch

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

> Source/WebCore/dom/ViewportArguments.cpp:186
> +    } else {
> +        // Clamp values to a valid range, but not for @viewport since is
> +        // not mandated by the specification.
> +        resultWidth = clampLengthValue(resultWidth);
> +        resultHeight = clampLengthValue(resultHeight);
> +        resultZoom = clampScaleValue(resultZoom);
> +        resultMinZoom = clampScaleValue(resultMinZoom);
> +        resultMaxZoom = clampScaleValue(resultMaxZoom);
>      }

I think an if would be nicer, we probably dont want to clam implicit viewport either

if (type != ViewportArguments::CSSDeviceAdaptation && type != ViewportArguments::Implicit) or maybe the other way around. Make it explicit what we are clamping.

> Source/WebCore/dom/ViewportArguments.cpp:240
> +    } else {
> +        // Extend width and height to fill the visual viewport for the resolved initial-scale.
> +        resultWidth = max<float>(resultWidth, initialViewportSize.width() / result.initialScale);
> +        resultHeight = max<float>(resultHeight, initialViewportSize.height() / result.initialScale);
> +    }

I think this should probably only happen for viewport meta, not the old legacy meta tags which dont support scale anyway
Comment 3 Thiago Marcos P. Santos 2012-11-26 07:46:44 PST
Created attachment 176001 [details]
Patch
Comment 4 WebKit Review Bot 2012-11-26 08:23:18 PST
Comment on attachment 176001 [details]
Patch

Clearing flags on attachment: 176001

Committed r135728: <http://trac.webkit.org/changeset/135728>
Comment 5 WebKit Review Bot 2012-11-26 08:23:22 PST
All reviewed patches have been landed.  Closing bug.