Title: [205868] trunk/Source/WebCore
- Revision
- 205868
- Author
- [email protected]
- Date
- 2016-09-13 12:01:32 -0700 (Tue, 13 Sep 2016)
Log Message
Remove CSS keyword properties from CSSParser::parseValue(CSSPropertyID, bool)
https://bugs.webkit.org/show_bug.cgi?id=161918
Reviewed by Simon Fraser.
CSSParser::parseValue(CSSPropertyID, bool) calls ASSERT_NOT_REACHED() when processing a CSS property
that is known to accept only keyword values as a means to guide a person to add such a CSS property
to the switch block in WebCore::isValidKeywordPropertyAndValue(). In theory this sounds good, but
in practice it does not work out and the list of such properties is stale. We should remove the
case statements for such properties and the maintenance burden they required, which was manual and
error prone. We should think about a better way to enforce that all CSS properties are parsed/validated.
The approach of calling ASSERT_NOT_REACHED is not beneficial to catching coding mistakes because
CSSParser::parseValue() has a default case statement to parse/validate SVG CSS properties and hence
does not allow the C++ compiler to validate that the switch block covers all CSSPropertyIDs.
* css/parser/CSSParser.cpp:
(WebCore::CSSParser::parseValue):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (205867 => 205868)
--- trunk/Source/WebCore/ChangeLog 2016-09-13 19:01:04 UTC (rev 205867)
+++ trunk/Source/WebCore/ChangeLog 2016-09-13 19:01:32 UTC (rev 205868)
@@ -1,5 +1,26 @@
2016-09-13 Daniel Bates <[email protected]>
+ Remove CSS keyword properties from CSSParser::parseValue(CSSPropertyID, bool)
+ https://bugs.webkit.org/show_bug.cgi?id=161918
+
+ Reviewed by Simon Fraser.
+
+ CSSParser::parseValue(CSSPropertyID, bool) calls ASSERT_NOT_REACHED() when processing a CSS property
+ that is known to accept only keyword values as a means to guide a person to add such a CSS property
+ to the switch block in WebCore::isValidKeywordPropertyAndValue(). In theory this sounds good, but
+ in practice it does not work out and the list of such properties is stale. We should remove the
+ case statements for such properties and the maintenance burden they required, which was manual and
+ error prone. We should think about a better way to enforce that all CSS properties are parsed/validated.
+
+ The approach of calling ASSERT_NOT_REACHED is not beneficial to catching coding mistakes because
+ CSSParser::parseValue() has a default case statement to parse/validate SVG CSS properties and hence
+ does not allow the C++ compiler to validate that the switch block covers all CSSPropertyIDs.
+
+ * css/parser/CSSParser.cpp:
+ (WebCore::CSSParser::parseValue):
+
+2016-09-13 Daniel Bates <[email protected]>
+
Organize CSS keyword properties in WebCore::isKeywordPropertyID()
https://bugs.webkit.org/show_bug.cgi?id=161917
Modified: trunk/Source/WebCore/css/parser/CSSParser.cpp (205867 => 205868)
--- trunk/Source/WebCore/css/parser/CSSParser.cpp 2016-09-13 19:01:04 UTC (rev 205867)
+++ trunk/Source/WebCore/css/parser/CSSParser.cpp 2016-09-13 19:01:32 UTC (rev 205868)
@@ -3170,124 +3170,6 @@
ASSERT(RuntimeEnabledFeatures::sharedFeatures().isCSSGridLayoutEnabled());
return parseItemPositionOverflowPosition(propId, important);
#endif
- case CSSPropertyBorderBottomStyle:
- case CSSPropertyBorderCollapse:
- case CSSPropertyBorderLeftStyle:
- case CSSPropertyBorderRightStyle:
- case CSSPropertyBorderTopStyle:
- case CSSPropertyBoxSizing:
- case CSSPropertyBreakAfter:
- case CSSPropertyBreakBefore:
- case CSSPropertyBreakInside:
- case CSSPropertyCaptionSide:
- case CSSPropertyClear:
- case CSSPropertyDirection:
- case CSSPropertyDisplay:
- case CSSPropertyEmptyCells:
- case CSSPropertyFloat:
- case CSSPropertyFontStyle:
- case CSSPropertyImageRendering:
- case CSSPropertyListStylePosition:
- case CSSPropertyListStyleType:
- case CSSPropertyObjectFit:
- case CSSPropertyOutlineStyle:
- case CSSPropertyOverflowWrap:
- case CSSPropertyOverflowX:
- case CSSPropertyOverflowY:
- case CSSPropertyPageBreakAfter:
- case CSSPropertyPageBreakBefore:
- case CSSPropertyPageBreakInside:
- case CSSPropertyPointerEvents:
- case CSSPropertyPosition:
- case CSSPropertyResize:
- case CSSPropertySpeak:
- case CSSPropertyTableLayout:
- case CSSPropertyTextLineThroughMode:
- case CSSPropertyTextLineThroughStyle:
- case CSSPropertyTextOverflow:
- case CSSPropertyTextOverlineMode:
- case CSSPropertyTextOverlineStyle:
- case CSSPropertyTextRendering:
- case CSSPropertyTextTransform:
- case CSSPropertyTextUnderlineMode:
- case CSSPropertyTextUnderlineStyle:
- case CSSPropertyVisibility:
- case CSSPropertyWebkitAppearance:
- case CSSPropertyWebkitBackfaceVisibility:
- case CSSPropertyWebkitBorderAfterStyle:
- case CSSPropertyWebkitBorderBeforeStyle:
- case CSSPropertyWebkitBorderEndStyle:
- case CSSPropertyWebkitBorderFit:
- case CSSPropertyWebkitBorderStartStyle:
- case CSSPropertyWebkitBoxAlign:
-#if ENABLE(CSS_BOX_DECORATION_BREAK)
- case CSSPropertyWebkitBoxDecorationBreak:
-#endif
- case CSSPropertyWebkitBoxDirection:
- case CSSPropertyWebkitBoxLines:
- case CSSPropertyWebkitBoxOrient:
- case CSSPropertyWebkitBoxPack:
- case CSSPropertyWebkitColumnBreakAfter:
- case CSSPropertyWebkitColumnBreakBefore:
- case CSSPropertyWebkitColumnBreakInside:
- case CSSPropertyColumnFill:
- case CSSPropertyColumnRuleStyle:
- case CSSPropertyFlexDirection:
- case CSSPropertyFlexWrap:
- case CSSPropertyWebkitFontKerning:
- case CSSPropertyWebkitFontSmoothing:
- case CSSPropertyWebkitHyphens:
- case CSSPropertyWebkitLineAlign:
- case CSSPropertyWebkitLineBreak:
- case CSSPropertyWebkitLineSnap:
- case CSSPropertyWebkitMarginAfterCollapse:
- case CSSPropertyWebkitMarginBeforeCollapse:
- case CSSPropertyWebkitMarginBottomCollapse:
- case CSSPropertyWebkitMarginTopCollapse:
- case CSSPropertyWebkitMarqueeDirection:
- case CSSPropertyWebkitMarqueeStyle:
- case CSSPropertyWebkitNbspMode:
-#if ENABLE(ACCELERATED_OVERFLOW_SCROLLING)
- case CSSPropertyWebkitOverflowScrolling:
-#endif
- case CSSPropertyWebkitPrintColorAdjust:
-#if ENABLE(CSS_REGIONS)
- case CSSPropertyWebkitRegionBreakAfter:
- case CSSPropertyWebkitRegionBreakBefore:
- case CSSPropertyWebkitRegionBreakInside:
- case CSSPropertyWebkitRegionFragment:
-#endif
- case CSSPropertyWebkitRtlOrdering:
- case CSSPropertyWebkitRubyPosition:
-#if ENABLE(CSS3_TEXT)
- case CSSPropertyWebkitTextAlignLast:
-#endif // CSS3_TEXT
- case CSSPropertyWebkitTextCombine:
-#if ENABLE(CSS3_TEXT)
- case CSSPropertyWebkitTextJustify:
-#endif // CSS3_TEXT
- case CSSPropertyWebkitTextSecurity:
- case CSSPropertyTransformStyle:
- case CSSPropertyWebkitTransformStyle:
- case CSSPropertyWebkitUserDrag:
- case CSSPropertyWebkitUserModify:
- case CSSPropertyWebkitUserSelect:
- case CSSPropertyWebkitWritingMode:
- case CSSPropertyWhiteSpace:
- case CSSPropertyWordBreak:
- case CSSPropertyWordWrap:
-#if ENABLE(TOUCH_EVENTS)
- case CSSPropertyTouchAction:
-#endif
-#if ENABLE(CSS_SCROLL_SNAP)
- case CSSPropertyWebkitScrollSnapType:
-#endif
-#if ENABLE(CSS_TRAILING_WORD)
- case CSSPropertyAppleTrailingWord:
-#endif
- // These properties should be handled before in isValidKeywordPropertyAndValue().
- ASSERT_NOT_REACHED();
- return false;
#if ENABLE(CSS_DEVICE_ADAPTATION)
// Properties bellow are validated inside parseViewportProperty, because we
// check for parser state inViewportScope. We need to invalidate if someone
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
