Bug 87169 - Move some CSS regions properties to CSSParser::isValidKeywordPropertyAndValue.
Summary: Move some CSS regions properties to CSSParser::isValidKeywordPropertyAndValue.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Alexis Menard (darktears)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-05-22 14:30 PDT by Alexis Menard (darktears)
Modified: 2012-05-22 16:58 PDT (History)
3 users (show)

See Also:


Attachments
Patch (6.22 KB, patch)
2012-05-22 14:32 PDT, Alexis Menard (darktears)
no flags Details | Formatted Diff | Diff
Patch for landing (6.36 KB, patch)
2012-05-22 14:57 PDT, Alexis Menard (darktears)
no flags Details | Formatted Diff | Diff
Patch for landing (6.45 KB, patch)
2012-05-22 15:01 PDT, Alexis Menard (darktears)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexis Menard (darktears) 2012-05-22 14:30:44 PDT
Move some CSS regions properties to CSSParser::isValidKeywordPropertyAndValue.
Comment 1 Alexis Menard (darktears) 2012-05-22 14:32:15 PDT
Created attachment 143365 [details]
Patch
Comment 2 Tony Chang 2012-05-22 14:37:54 PDT
Comment on attachment 143365 [details]
Patch

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

OK

> Source/WebCore/css/CSSParser.cpp:588
> +    case CSSPropertyWebkitColumnBreakBefore:
> +    case CSSPropertyWebkitRegionBreakAfter:
> +    case CSSPropertyWebkitRegionBreakBefore:
> +        if (valueID == CSSValueAuto || valueID == CSSValueAlways || valueID == CSSValueAvoid || valueID == CSSValueLeft || valueID == CSSValueRight)
> +            return ((propertyId == CSSPropertyWebkitRegionBreakAfter) || (propertyId == CSSPropertyWebkitRegionBreakBefore)) ? parserContext.isCSSRegionsEnabled : true;

Should we split the region properties into a separate part of the switch statement?  It would avoid the extra check for the non-region properties.
Comment 3 Alexis Menard (darktears) 2012-05-22 14:48:27 PDT
(In reply to comment #2)
> (From update of attachment 143365 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=143365&action=review
> 
> OK
> 
> > Source/WebCore/css/CSSParser.cpp:588
> > +    case CSSPropertyWebkitColumnBreakBefore:
> > +    case CSSPropertyWebkitRegionBreakAfter:
> > +    case CSSPropertyWebkitRegionBreakBefore:
> > +        if (valueID == CSSValueAuto || valueID == CSSValueAlways || valueID == CSSValueAvoid || valueID == CSSValueLeft || valueID == CSSValueRight)
> > +            return ((propertyId == CSSPropertyWebkitRegionBreakAfter) || (propertyId == CSSPropertyWebkitRegionBreakBefore)) ? parserContext.isCSSRegionsEnabled : true;
> 
> Should we split the region properties into a separate part of the switch statement?  It would avoid the extra check for the non-region properties.

Ok make sense.
Comment 4 Alexis Menard (darktears) 2012-05-22 14:57:50 PDT
Created attachment 143371 [details]
Patch for landing
Comment 5 Alexis Menard (darktears) 2012-05-22 15:01:37 PDT
Created attachment 143372 [details]
Patch for landing
Comment 6 WebKit Review Bot 2012-05-22 16:58:22 PDT
Comment on attachment 143372 [details]
Patch for landing

Clearing flags on attachment: 143372

Committed r118082: <http://trac.webkit.org/changeset/118082>
Comment 7 WebKit Review Bot 2012-05-22 16:58:27 PDT
All reviewed patches have been landed.  Closing bug.