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();