[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    branches/KDE/4.1/kdelibs/khtml/css
From:       Maks Orlovich <maksim () kde ! org>
Date:       2008-08-24 19:11:48
Message-ID: 1219605108.776667.30300.nullmailer () svn ! kde ! org
[Download RAW message or body]

SVN commit 851859 by orlovich:

Fix a couple of cases where background-position: parsing accepted invalid settings.
BUG: 169612


 M  +62 -25    cssparser.cpp  
 M  +9 -1      cssparser.h  


--- branches/KDE/4.1/kdelibs/khtml/css/cssparser.cpp #851858:851859
@@ -1478,30 +1478,29 @@
     return 0;
 }
 
-CSSValueImpl* CSSParser::parseBackgroundPositionXY(bool& xFound, bool& yFound)
+CSSValueImpl* CSSParser::parseBackgroundPositionXY(BackgroundPosKind& kindOut)
 {
     int id = valueList->current()->id;
     if (id == CSS_VAL_LEFT || id == CSS_VAL_TOP || id == CSS_VAL_RIGHT || id == \
CSS_VAL_BOTTOM || id == CSS_VAL_CENTER) {  int percent = 0;
         if (id == CSS_VAL_LEFT || id == CSS_VAL_RIGHT) {
-            if (xFound)
-                return 0;
-            xFound = true;
+            kindOut = BgPos_X;
             if (id == CSS_VAL_RIGHT)
                 percent = 100;
         }
         else if (id == CSS_VAL_TOP || id == CSS_VAL_BOTTOM) {
-            if (yFound)
-                return 0;
-            yFound = true;
+            kindOut = BgPos_Y;
             if (id == CSS_VAL_BOTTOM)
                 percent = 100;
         }
-        else if (id == CSS_VAL_CENTER)
+        else if (id == CSS_VAL_CENTER) {
             // Center is ambiguous, so we're not sure which position we've found \
yet, an x or a y. +            kindOut = BgPos_Center;
             percent = 50;
+        }
         return new CSSPrimitiveValueImpl(percent, \
CSSPrimitiveValue::CSS_PERCENTAGE);  }
+    kindOut = BgPos_NonKW;
     if (validUnit(valueList->current(), FPercent|FLength, strict))
         return new CSSPrimitiveValueImpl(valueList->current()->fValue,
                                          \
(CSSPrimitiveValue::UnitTypes)valueList->current()->unit); @@ -1515,8 +1514,8 @@
     Value* value = valueList->current();
 
     // Parse the first value.  We're just making sure that it is one of the valid \
                keywords or a percentage/length.
-    bool value1IsX = false, value1IsY = false;
-    value1 = parseBackgroundPositionXY(value1IsX, value1IsY);
+    BackgroundPosKind value1pos;
+    value1 = parseBackgroundPositionXY(value1pos);
     if (!value1)
         return;
 
@@ -1529,9 +1528,9 @@
     if (value && value->unit == Value::Operator && value->iValue == ',')
         value = 0;
 
-    bool value2IsX = false, value2IsY = false;
+    BackgroundPosKind value2pos = BgPos_Center; // true if not specified.
     if (value) {
-        value2 = parseBackgroundPositionXY(value2IsX, value2IsY);
+        value2 = parseBackgroundPositionXY(value2pos);
         if (value2)
             valueList->next();
         else {
@@ -1544,13 +1543,39 @@
     }
 
     if (!value2)
-        // Only one value was specified.  If that value was not a keyword, then it \
                sets the x position, and the y position
-        // is simply 50%.  This is our default.
-        // For keywords, the keyword was either an x-keyword (left/right), a \
                y-keyword (top/bottom), or an ambiguous keyword (center).
-        // For left/right/center, the default of 50% in the y is still correct.
+        // Only one value was specified.  The other direction is always 50%.
+        // If the one given was not a keyword, it should be viewed as 'x',
+        // and so setting value2 would set y, as desired.
+        // If the one value was a keyword, the swap below would put things in order
+        // if needed.
         value2 = new CSSPrimitiveValueImpl(50, CSSPrimitiveValue::CSS_PERCENTAGE);
 
-    if (value1IsY || value2IsX) {
+    // Check for various failures
+    bool ok = true;
+
+    // Two keywords on the same axis.
+    if (value1pos == BgPos_X && value2pos == BgPos_X)
+        ok = false;
+    if (value1pos == BgPos_Y && value2pos == BgPos_Y)
+        ok = false;
+
+    // Will we need to swap them?
+    bool swap = (value1pos == BgPos_Y || value2pos == BgPos_X);
+
+    // If we had a non-KW value and a keyword value that's in the "wrong" position,
+    // this is malformed (#169612)
+    if (swap && (value1pos == BgPos_NonKW || value2pos == BgPos_NonKW))
+        ok = false;
+
+    if (!ok) {
+        delete value1;
+        delete value2;
+        value1 = 0;
+        value2 = 0;
+        return;
+    }
+
+    if (swap) {
         // Swap our two values.
         CSSValueImpl* val = value2;
         value2 = value1;
@@ -1648,17 +1673,29 @@
                     // unlike the other functions, parseBackgroundPosition advances \
the valueList pointer  break;
                 case CSS_PROP_BACKGROUND_POSITION_X: {
-                    bool xFound = false, yFound = true;
-                    currValue = parseBackgroundPositionXY(xFound, yFound);
-                    if (currValue)
-                        valueList->next();
+                    BackgroundPosKind pos;
+                    currValue = parseBackgroundPositionXY(pos);
+                    if (currValue) {
+                        if (pos == BgPos_Y) {
+                            delete currValue;
+                            currValue = 0;
+                        } else {
+                            valueList->next();
+                        }
+                    }
                     break;
                 }
                 case CSS_PROP_BACKGROUND_POSITION_Y: {
-                    bool xFound = true, yFound = false;
-                    currValue = parseBackgroundPositionXY(xFound, yFound);
-                    if (currValue)
-                        valueList->next();
+                    BackgroundPosKind pos;
+                    currValue = parseBackgroundPositionXY(pos);
+                    if (currValue) {
+                        if (pos == BgPos_X) {
+                            delete currValue;
+                            currValue = 0;
+                        } else {                    
+                            valueList->next();
+                        }
+                    }
                     break;
                 }
                 case CSS_PROP_BACKGROUND_REPEAT:
--- branches/KDE/4.1/kdelibs/khtml/css/cssparser.h #851858:851859
@@ -128,7 +128,15 @@
 
         CSSValueImpl* parseBackgroundColor();
         CSSValueImpl* parseBackgroundImage(bool& didParse);
-        CSSValueImpl* parseBackgroundPositionXY(bool& xFound, bool& yFound);
+
+        enum BackgroundPosKind {
+            BgPos_X,
+            BgPos_Y,
+            BgPos_NonKW,
+            BgPos_Center
+        };
+        
+        CSSValueImpl* parseBackgroundPositionXY(BackgroundPosKind& kindOut);
         void parseBackgroundPosition(CSSValueImpl*& value1, CSSValueImpl*& value2);
         CSSValueImpl* parseBackgroundSize();
 


[prev in list] [next in list] [prev in thread] [next in thread]