<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[205868] trunk/Source/WebCore</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/205868">205868</a></dd>
<dt>Author</dt> <dd>dbates@webkit.org</dd>
<dt>Date</dt> <dd>2016-09-13 12:01:32 -0700 (Tue, 13 Sep 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>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):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCorecssparserCSSParsercpp">trunk/Source/WebCore/css/parser/CSSParser.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (205867 => 205868)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1,5 +1,26 @@
</span><span class="cx"> 2016-09-13 Daniel Bates <dabates@apple.com>
</span><span class="cx">
</span><ins>+ 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 <dabates@apple.com>
+
</ins><span class="cx"> Organize CSS keyword properties in WebCore::isKeywordPropertyID()
</span><span class="cx"> https://bugs.webkit.org/show_bug.cgi?id=161917
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceWebCorecssparserCSSParsercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/css/parser/CSSParser.cpp (205867 => 205868)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -3170,124 +3170,6 @@
</span><span class="cx"> ASSERT(RuntimeEnabledFeatures::sharedFeatures().isCSSGridLayoutEnabled());
</span><span class="cx"> return parseItemPositionOverflowPosition(propId, important);
</span><span class="cx"> #endif
</span><del>- 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;
</del><span class="cx"> #if ENABLE(CSS_DEVICE_ADAPTATION)
</span><span class="cx"> // Properties bellow are validated inside parseViewportProperty, because we
</span><span class="cx"> // check for parser state inViewportScope. We need to invalidate if someone
</span></span></pre>
</div>
</div>
</body>
</html>