Closed Bug 1199610 Opened 8 years ago Closed 7 years ago
various CSS parser methods incorrectly assume Parse
Variant and friends consume no tokens on failure
Testcase : http://jsfiddle.net/ntim/os1ev3ur/
:heycam, can you please look at this ? Thanks.
(I'm not sure if the "0" instead of "0px" should work with calc(), but even fixing that doesn't make the testcase work.)
Steps to reproduce: 1. load this testcase 2. hover over the tabbar Expected results: 1. white bars between Foo | Bar | Baz are not the whole height of the red strip 2. when hovering, white bars are now the whole height of the red strip Actual results: 1. shows expected results for (2)
I think the issue is similar to one we've had before; when we're in the middle of parsing the calc() value, we encounter the "var(" token in the middle of ParseNonNegativeVariant, which triggers a SkipUntil(')'). The false return false of ParseNonNegativeVariant isn't treated as an error -- it's really assuming that we left the state of scanner before the value we attempted to parse. We then arrive at the ';' token, which ExpectEndProperty up in ParseProperty takes to mean we completely parsed a successful property. You can get the same problem without variables, with e.g. background-size: 10px calc(0px + rubbish);
Assignee: nobody → cam
Status: NEW → ASSIGNED
Summary: CSS variables don't work with calc() → background-size does not handle parse errors inside calc() expressions correctly
That other similar bug was bug 1099448.
From code inspection (but without testing) it looks like there are other examples of this error: CSSParserImpl::ParseGridTrackBreadth CSSParserImpl::ParseColorStop CSSParserImpl::ParseBoxCornerRadius CSSParserImpl::ParseBoxCornerRadiiInternals CSSParserImpl::ParseBoxPositionValues CSSParserImpl::ParsePositionValue CSSParserImpl::ParseBorderSpacing CSSParserImpl::ParseTransformOrigin CSSParserImpl::ParseShadowItem (for the optional radius/spread/color values) CSSParserImpl::ParsePaint I'm inclined to fix these by making the various ParseVariant methods be able to restore the input stream state if they encounter an error inside a calc(), rgb(), hsl(), etc., rather than copying the input position code that we added in bug 1099448 to all of the above methods.
Summary: background-size does not handle parse errors inside calc() expressions correctly → various CSS parser methods incorrectly assume ParseVariant and friends consume no tokens on failure
Bug 1179078 was also similar, and when I was prompted to look at it a few days ago due to bugmail, I filed bug 1213076 on the general problem with roughly that idea, and also one alternative.
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.