Bug 95962

Summary: Validate CSS Device Adaptation properties and resolve shorthands
Product: WebKit Reporter: Thiago Marcos P. Santos <tmpsantos>
Component: CSSAssignee: Thiago Marcos P. Santos <tmpsantos>
Status: RESOLVED FIXED    
Severity: Normal CC: cmarcelo, donggwan.kim, kenneth, macpherson, menard, 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-09-06 03:01:31 PDT 
Add missing keywords and validate the values attributed to the properties allowed in a viewport rule.
Comment 1 Thiago Marcos P. Santos 2012-09-12 14:49:56 PDT 
Created attachment 163704 [details]
Patch
Comment 2 Kenneth Rohde Christiansen 2012-10-25 06:56:52 PDT 
Comment on attachment 163704 [details]
Patch

This needs some test
Comment 3 Thiago Marcos P. Santos 2012-11-02 08:51:59 PDT 
Created attachment 172069 [details]
Patch
Comment 4 Kenneth Rohde Christiansen 2012-11-02 09:05:41 PDT 
Comment on attachment 172069 [details]
Patch

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

> Source/WebCore/css/CSSParser.cpp:2867
> +    // Properties bellow are validated inside parseViewportProperty, because we

below*

> Source/WebCore/css/CSSParser.cpp:10469
> +        if (id == CSSValueAuto)
> +            validPrimitive = true;
> +        else
> +            validPrimitive = (!id && validUnit(value, FNumber | FPercent | FNonNeg));
> +        break;

why not validPrimitive = (id == CSSValueAuto) || ....

> LayoutTests/css3/device-adapt/viewport-properties-validation.html:102
> +            zoom: 60%;
> +            letter-spacing: 0.5em;
> +        }

Why not testing max-zoom outside etc.
Comment 5 Kenneth Rohde Christiansen 2012-11-02 09:30:37 PDT 
Comment on attachment 172069 [details]
Patch

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

>> LayoutTests/css3/device-adapt/viewport-properties-validation.html:102
>> +        }
> 
> Why not testing max-zoom outside etc.

You are, but a bit up .foo { }, sorry
Comment 6 Alexis Menard (darktears) 2012-11-05 04:22:37 PST 
Comment on attachment 172069 [details]
Patch

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

>> Source/WebCore/css/CSSParser.cpp:10469
>> +        break;
> 
> why not validPrimitive = (id == CSSValueAuto) || ....

but the way he wrote it follows the style of the file e.g. line 2140 for example.
Comment 7 WebKit Review Bot 2012-11-05 04:48:12 PST 
Comment on attachment 172069 [details]
Patch

Clearing flags on attachment: 172069

Committed r133458: <http://trac.webkit.org/changeset/133458>
Comment 8 WebKit Review Bot 2012-11-05 04:48:16 PST 
All reviewed patches have been landed.  Closing bug.