Summary: | REGRESSION (r136683): css3/calc/background-position-parsing.html failing on EFL Linux 64-bit Debug WK2 | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Thiago Marcos P. Santos <tmpsantos> | ||||||||||||
Component: | Tools / Tests | Assignee: | Alexis Menard (darktears) <menard> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | cmarcelo, jchaffraix, kling, koivisto, krit, macpherson, menard, ojan, webkit.review.bot | ||||||||||||
Priority: | P2 | Keywords: | LayoutTestFailure, MakingBotsRed, Regression | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
URL: | http://build.webkit.org/results/EFL%20Linux%2064-bit%20Debug%20WK2/r136683%20(6698)/results.html | ||||||||||||||
Bug Depends on: | |||||||||||||||
Bug Blocks: | 104014 | ||||||||||||||
Attachments: |
|
Description
Thiago Marcos P. Santos
2012-12-05 09:10:15 PST
*** Bug 104133 has been marked as a duplicate of this bug. *** Created attachment 177779 [details]
Patch
(In reply to comment #2) > Created an attachment (id=177779) [details] > Patch I'm wondering if the fix should go in isPositionValue instead. Thoughts? Created attachment 177782 [details]
Patch
Created attachment 177787 [details]
Fix comment
Comment on attachment 177787 [details] Fix comment View in context: https://bugs.webkit.org/attachment.cgi?id=177787&action=review > Source/WebCore/css/CSSParser.cpp:3865 > + m_parsedCalculation.release(); This should be m_parsedCalculation = nullptr; Comment on attachment 177787 [details] Fix comment 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. Created attachment 177811 [details]
Patch
Comment on attachment 177811 [details] Patch Clearing flags on attachment: 177811 Committed r136754: <http://trac.webkit.org/changeset/136754> All reviewed patches have been landed. Closing bug. |