[webkit-changes] [136754] trunk/Source/WebCore

Wed, 05 Dec 2012 13:37:05 -0800

Title: [136754] trunk/Source/WebCore
Revision
136754
Author
[email protected]
Date
2012-12-05 13:38:52 -0800 (Wed, 05 Dec 2012)

Log Message

REGRESSION (r136683): css3/calc/background-position-parsing.html failing on EFL Linux 64-bit Debug WK2
https://bugs.webkit.org/show_bug.cgi?id=104131

Reviewed by Antti Koivisto.

css3/calc/background-position-parsing.html assert in debug because we
call CSSParser::validUnit multiple times in a row. The problem was with
validUnit which check calc() values and save the result inside
m_parsedCalculation for later usage. validUnit expects you to
use m_parsedCalculation therefore calling validUnit again with
m_parsedCalculation being set asserts. As parseFillBackgroundPosition
just want to check wether the current value is maybe valid for
background-position we can just relax the check to allow either the
valid keywords or any other units (we will anyway filter the incorrect
values later in the parsing). The most important check at this point
for the shorthand is the validity of the keyword.

No new tests : the assert was covered by css3/calc/background-position-parsing.html.

* css/CSSParser.cpp:
(WebCore::CSSParser::isPotentialPositionValue):
(WebCore::CSSParser::parseFillBackgroundPosition):
* css/CSSParser.h:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (136753 => 136754)


--- trunk/Source/WebCore/ChangeLog	2012-12-05 21:37:34 UTC (rev 136753)
+++ trunk/Source/WebCore/ChangeLog	2012-12-05 21:38:52 UTC (rev 136754)
@@ -1,3 +1,29 @@
+2012-12-05  Alexis Menard  <[email protected]>
+
+        REGRESSION (r136683): css3/calc/background-position-parsing.html failing on EFL Linux 64-bit Debug WK2
+        https://bugs.webkit.org/show_bug.cgi?id=104131
+
+        Reviewed by Antti Koivisto.
+
+        css3/calc/background-position-parsing.html assert in debug because we
+        call CSSParser::validUnit multiple times in a row. The problem was with
+        validUnit which check calc() values and save the result inside
+        m_parsedCalculation for later usage. validUnit expects you to
+        use m_parsedCalculation therefore calling validUnit again with
+        m_parsedCalculation being set asserts. As parseFillBackgroundPosition
+        just want to check wether the current value is maybe valid for
+        background-position we can just relax the check to allow either the
+        valid keywords or any other units (we will anyway filter the incorrect
+        values later in the parsing). The most important check at this point
+        for the shorthand is the validity of the keyword.
+
+        No new tests : the assert was covered by css3/calc/background-position-parsing.html.
+
+        * css/CSSParser.cpp:
+        (WebCore::CSSParser::isPotentialPositionValue):
+        (WebCore::CSSParser::parseFillBackgroundPosition):
+        * css/CSSParser.h:
+
 2012-12-05  Stephen White  <[email protected]>
 
         Unreviewed, rolling out r136735.

Modified: trunk/Source/WebCore/css/CSSParser.cpp (136753 => 136754)


--- trunk/Source/WebCore/css/CSSParser.cpp	2012-12-05 21:37:34 UTC (rev 136753)
+++ trunk/Source/WebCore/css/CSSParser.cpp	2012-12-05 21:38:52 UTC (rev 136754)
@@ -3857,9 +3857,9 @@
 #endif
 }
 
-inline bool CSSParser::isPositionValue(CSSParserValue* value)
+inline bool CSSParser::isPotentialPositionValue(CSSParserValue* value)
 {
-    return isBackgroundPositionKeyword(value->id) || validUnit(value, FPercent | FLength);
+    return !value->id || isBackgroundPositionKeyword(value->id);
 }
 
 void CSSParser::parseFillBackgroundPosition(CSSParserValueList* valueList, RefPtr<CSSValue>& value1, RefPtr<CSSValue>& value2)
@@ -3867,7 +3867,7 @@
     unsigned numberOfValues = 0;
     for (unsigned i = valueList->currentIndex(); i < valueList->size(); ++i, ++numberOfValues) {
         CSSParserValue* current = valueList->valueAt(i);
-        if (isComma(current) || !current || (current->unit == CSSParserValue::Operator && current->iValue == '/') || !isPositionValue(current))
+        if (isComma(current) || !current || (current->unit == CSSParserValue::Operator && current->iValue == '/') || !isPotentialPositionValue(current))
             break;
     }

Modified: trunk/Source/WebCore/css/CSSParser.h (136753 => 136754)


--- trunk/Source/WebCore/css/CSSParser.h	2012-12-05 21:37:34 UTC (rev 136753)
+++ trunk/Source/WebCore/css/CSSParser.h	2012-12-05 21:38:52 UTC (rev 136754)
@@ -119,7 +119,7 @@
     PassRefPtr<CSSValue> parseFillPositionY(CSSParserValueList*);
     void parseFillPosition(CSSParserValueList*, RefPtr<CSSValue>&, RefPtr<CSSValue>&);
 #if ENABLE(CSS3_BACKGROUND)
-    bool isPositionValue(CSSParserValue*);
+    bool isPotentialPositionValue(CSSParserValue*);
     void parseFillBackgroundPosition(CSSParserValueList*, RefPtr<CSSValue>&, RefPtr<CSSValue>&);
     void parse3ValuesBackgroundPosition(CSSParserValueList*, RefPtr<CSSValue>&, RefPtr<CSSValue>&, PassRefPtr<CSSPrimitiveValue>, PassRefPtr<CSSPrimitiveValue>);
     void parse4ValuesBackgroundPosition(CSSParserValueList*, RefPtr<CSSValue>&, RefPtr<CSSValue>&, PassRefPtr<CSSPrimitiveValue>, PassRefPtr<CSSPrimitiveValue>);

_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to