Bug 95962 - Validate CSS Device Adaptation properties and resolve shorthands
Summary: Validate CSS Device Adaptation properties and resolve shorthands
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Thiago Marcos P. Santos
URL:
Keywords:
Depends on:
Blocks: 95959
  Show dependency treegraph
 
Reported: 2012-09-06 03:01 PDT by Thiago Marcos P. Santos
Modified: 2012-11-05 04:48 PST (History)
6 users (show)

See Also:


Attachments
Patch (9.12 KB, patch)
2012-09-12 14:49 PDT, Thiago Marcos P. Santos
no flags Details | Formatted Diff | Diff
Patch (17.70 KB, patch)
2012-11-02 08:51 PDT, Thiago Marcos P. Santos
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.