| Summary: | Browser does not fall back to SVG attribute value when CSS style value is invalid or not supported | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Sara Soueidan <sara.soueidan> | ||||||||
| Component: | SVG | Assignee: | Antoine Quint <graouts> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | bdakin, commit-queue, dino, graouts, jond, sabouhallawa, simon.fraser, webkit-bug-importer, zimmermann | ||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||
| Version: | 528+ (Nightly build) | ||||||||||
| Hardware: | Unspecified | ||||||||||
| OS: | Unspecified | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Sara Soueidan
2015-08-12 01:44:06 PDT
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.
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.
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." 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. 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);
Created attachment 266184 [details]
Patch
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? Comment on attachment 266184 [details] Patch Clearing flags on attachment: 266184 Committed r192788: <http://trac.webkit.org/changeset/192788> All reviewed patches have been landed. Closing bug. |