[webkit-reviews] review denied: [Bug 104131] REGRESSION (r136683): css3/calc/background-position-parsing.html failing on EFL Linux 64-bit Debug WK2 : [Attachment 177787] Fix comment

[bugzilla-daemon at webkit.org]

Antti Koivisto <koivisto at iki.fi> has denied Alexis Menard (darktears)
<alexis at webkit.org>'s request for review:
Bug 104131: 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

Attachment 177787: Fix comment
https://bugs.webkit.org/attachment.cgi?id=177787&action=review

------- Additional Comments from Antti Koivisto <koivisto at iki.fi>
View in context: https://bugs.webkit.org/attachment.cgi?id=177787&action=review


> Source/WebCore/css/CSSParser.cpp:3867
>  inline bool CSSParser::isPositionValue(CSSParserValue* value)
>  {
> -    return isBackgroundPositionKeyword(value->id) || validUnit(value,
FPercent | FLength);
> +    bool ret = isBackgroundPositionKeyword(value->id) || validUnit(value,
FPercent | FLength);
> +    // validUnit may encounter a CSS calc() and could potentially set
m_parsedCalculation to a value. As we may call later isPositionValue again
> +    // we need to unset it otherwise the next call to validUnit will assert
when it will check a calc again. parseFillPositionComponent also workaround
this behavior.
> +    m_parsedCalculation.release();
> +    return ret;
>  }

It seems wrong that a isFoo function has a side effect like this.