Bug 44047 – Stylesheets that use local references are treated as local to the SVG document

• Component: SVG
• Status: RESOLVED CONFIGURATION CHANGED

[Daniel Holbert]

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)

[Rob Buis]

Created attachment 65602 [details]
WIP patch

[Rob Buis]

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.

[Nikolas Zimmermann]

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.

[Ahmad Saleem]

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!

[Ryosuke Niwa]

Thanks for confirming. I've checked as well, and this indeed seem to be interoperable now.