Bug 147932 – Browser does not fall back to SVG attribute value when CSS style value is invalid or not supported

• Status: RESOLVED
• Resolution: FIXED
• Component: SVG

[Sara Soueidan]

An SVG element that has a fill color specified using both a fill attribute and a style attribute with fill property declaration, the browser needs to be able to fall back to the fill attribute value if the value in the style attribute is not valid.

In this particular case, an SVG element is styled using CSS variables inside a style attribute. Since Safari does not support CSS variables yet, it is supposed to ignore the fill value in the style tag (using the variable) and fall back to the fill value in the fill attribute.

Live example: http://codepen.io/SaraSoueidan/pen/c30e270090b2460ba6e6833c611e5a76

The robots should all be colored and look alike in Safari, as they do in Chrome, for example. But instead, Safari falls back to all black colors (the user agent defaults) because it does not use the fill attribute fallback provided.

More reference on this technique, how it works, and what's supposed to happen here: http://tympanus.net/codrops/2015/07/16/styling-svg-use-content-css/

[Sara Soueidan]

Created attachment 258817 [details]
Screenshot of what the demo should look like when CSS variables are not supported

This screenshot shows how the robots should look when CSS variables are not supported; i.e. if Safari does fall back appropriately. It currently shows all-black robots instead.

[Dean Jackson]

<rdar://problem/22261140>

[Antoine Quint]

Created attachment 266130 [details]
Testcase

Simple test case showing that setting style="fill: whatever", which is invalid, means that the valid fill="green" attribute won't get picked up.

[Antoine Quint]

For some background on this, I think this is the relevant part of the SVG 1.1 specification:

http://www.w3.org/TR/SVG11/styling.html#UsingPresentationAttributes

In particular, this excerpt:

"For user agents that support CSS, the presentation attributes must be translated to corresponding CSS style rules according to rules described in Precedence of non-CSS presentational hints ([CSS2], section 6.4.4), with the additional clarification that the presentation attributes are conceptually inserted into a new author style sheet which is the first in the author style sheet collection. The presentation attributes thus will participate in the CSS2 cascade as if they were replaced by corresponding CSS style rules placed at the start of the author style sheet with a specificity of zero. In general, this means that the presentation attributes have lower priority than other CSS style rules specified in author style sheets or ‘style’ attributes."

[Antoine Quint]

As far as I can understand so far, the MatchResult passed to StyleResolver::applyCascadedProperties has all the properties needed to make the right decision and fall back on a valid value. It doesn't use the valid property for reasons that aren't clear yet.

[Antoine Quint]

However, still in StyleResolver::applyCascadedProperties(), the cascaded properties don't have the expected values, so my guess is that something is going wrong when setting up the cascade in StyleResolver::applyMatchedProperties():

    CascadedProperties cascade(direction, writingMode);
    cascade.addMatches(matchResult, false, 0, matchResult.matchedProperties().size() - 1, applyInheritedOnly);
    cascade.addMatches(matchResult, true, matchResult.ranges.firstAuthorRule, matchResult.ranges.lastAuthorRule, applyInheritedOnly);
    cascade.addMatches(matchResult, true, matchResult.ranges.firstUserRule, matchResult.ranges.lastUserRule, applyInheritedOnly);
    cascade.addMatches(matchResult, true, matchResult.ranges.firstUARule, matchResult.ranges.lastUARule, applyInheritedOnly);

[Antoine Quint]

Created attachment 266184 [details]
Patch

[Antoine Quint]

Another fix I had in mind was to `return false` if `valueWithCalculation.value().id == CSSValueInvalid` was true in `CSSParser::parseSVGValue()`. This would mean that any value that is deemed invalid would be dropped. But since it was a much broader fix, I went for a more specialised fix for the case of SVGPaint values. I also wasn't entirely sure of the meaning of `CSSValueInvalid`, does it inform us with absolute certainty that the value is invalid, or only that with checks applied at this point that the value would likely be invalid?

[WebKit Commit Bot]

Comment on attachment 266184 [details]
Patch

Clearing flags on attachment: 266184

Committed r192788: <http://trac.webkit.org/changeset/192788>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.