Summary: | Stylesheets that use local references are treated as local to the SVG document | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Daniel Holbert <dholbert> | ||||||||
Component: | SVG | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED CONFIGURATION CHANGED | ||||||||||
Severity: | Normal | CC: | ahmad.saleem792, ap, rniwa, rwlbuis, zalan, zimmermann | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | PC | ||||||||||
OS: | Linux | ||||||||||
Attachments: |
|
Description
Daniel Holbert
2010-08-16 01:24:58 PDT
Created attachment 65602 [details]
WIP patch
It seems to me these checks can be done on three levels: 1 at the css parser level, perfectly possible but computed values for url(#foo) will not show and that is not what I see in Opera and FF. 2 at the css style selector level, checking the stylesheet baseURL with the doc URL seems not reliable though when doing run-webkit-tests, I suspect the static cache in StyledElement.cpp. 3 at rendering time, this has the drawback of slowing down painting. Since 2 seems the most reasonable (computed values ok and no painting speed loss) I went for that in my WIP patch. I think right now checking for data: is enough as xml-stylesheet referncing an external stylesheet seems to wok in all implementations (see for instance LayoutTests/svg/batik/paints/gradientLimit.svg). I have a testcase handy, just wanted to show the patch and maybe get some feedback on whether this is the right approach. Cheers, Rob. Comment on attachment 65602 [details]
WIP patch
WebCore/css/SVGCSSParser.cpp:97
+ parsedValue = CSSPrimitiveValue::create(m_styleSheet->completeURL(value->string), CSSPrimitiveValue::CSS_URI);
I'm not sure about this approach, as CSSParser.cpp contains lots of
// FIXME: The completeURL call should be done when using the CSSImageValue,
// not when creating it.
for each instance of the code which uses m_styleSheet->completeURL(value->string)
That indicates it's not the right way to go for new code.
WebCore/css/SVGCSSStyleSelector.cpp:237
+ if (svgPaint->uri().startsWith("data:"))
I don't get that check? Where is that used? What implications does it have if you wouldn't handle it here?WebCore/css/SVGCSSStyleSelector.cpp:346
+ if (s.startsWith("data:"))
Ditto.
WebCore/css/SVGCSSStyleSelector.cpp:365
+ if (s.startsWith("data:"))
Ditto.
WebCore/css/SVGCSSStyleSelector.cpp:384
+ if (s.startsWith("data:"))
Ditto.
WebCore/css/SVGCSSStyleSelector.cpp:426
+ if (s.startsWith("data:"))
Ditto.
WebCore/css/SVGCSSStyleSelector.cpp:445
+ if (s.startsWith("data:"))
Ditto.
WebCore/css/SVGCSSStyleSelector.cpp:464
+ if (s.startsWith("data:"))
Ditto.
If it's somehow absolutely needed (can't tell w/o a test or ChangeLog), then you should at least refactor it into a shared function.
Created attachment 460210 [details]
Safari 15.5 matches other browsers
I am unable to reproduce the "Expected Results" in Safari 15.5 on macOS 12.4 but it matches all other browsers (Chrome Canary 105 and Firefox Nightly 103).
I think it got fixed along the lines or spec changed and all other browsers started matching Webkit. Can this be marked as "RESOLVED INVALID" and "RESOLVED CONFIGURATION CHANGED". Thanks!
Thanks for confirming. I've checked as well, and this indeed seem to be interoperable now. |