Created attachment 64476 [details] testcase 1 STEPS TO REPRODUCE: Load attached testcase. EXPECTED RESULTS: The teal line should have no arrowhead. ACTUAL RESULTS: In WebKit, the teal line has an arrowhead. NOTE: My example uses a data URI for the stylesheet, for simplicity of bug-attaching. However, this happens for "real" (non-data-URI) external stylesheets as well. EXPLANATION: In the testcase, the reference "url(#myMarker)" ends up looking for that node in our our SVG document. This is incorrect -- per specification, that identifier should use the URI of the stylesheet instead. See in particular: http://www.w3.org/TR/CSS21/syndata.html#uri " In order to create modular style sheets that are not dependent on the absolute location of a resource, authors may use relative URIs. Relative URIs (as defined in [RFC3986]) are resolved to full URIs using a base URI. RFC 3986, section 5, defines the normative algorithm for this process. For CSS style sheets, the base URI is that of the style sheet, not that of the source document. " Filing this based on the following (invalid) Mozilla bugs: (they were filed for Mozilla *not* placing the arrowhead, but that's in fact the correct behavior) https://bugzilla.mozilla.org/show_bug.cgi?id=587490 https://bugzilla.mozilla.org/show_bug.cgi?id=309612 See those bugs for more discussion of this issue. Recent Firefox & Opera both give EXPECTED RESULTS. Webkit* gives ACTUAL RESULTS. * I tested these WebKit-based browsers on Ubuntu 10.04: Chromium 6.0.495.0 (56150) Midori 0.2.2 (WebKitGTK+ 1.1.21)
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.