Bug 87169

Summary:

Move some CSS regions properties to CSSParser::isValidKeywordPropertyAndValue.

Product:

WebKit

Reporter:

Alexis Menard (darktears) <menard>

Component:

CSS

Assignee:

Alexis Menard (darktears) <menard>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

macpherson, tony, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Patch	none
Patch for landing	none
Patch for landing	none

Alexis Menard (darktears)

Reported 2012-05-22 14:30:44 PDT

Move some CSS regions properties to CSSParser::isValidKeywordPropertyAndValue.

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
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Alexis Menard (darktears)

Comment 1 2012-05-22 14:32:15 PDT

Created attachment 143365 [details] Patch

Tony Chang

Comment 2 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.

Alexis Menard (darktears)

Comment 3 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.

Alexis Menard (darktears)

Comment 4 2012-05-22 14:57:50 PDT

Created attachment 143371 [details] Patch for landing

Alexis Menard (darktears)

Comment 5 2012-05-22 15:01:37 PDT

Created attachment 143372 [details] Patch for landing

WebKit Review Bot

Comment 6 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>

WebKit Review Bot

Comment 7 2012-05-22 16:58:27 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.