1157953 - Cannot resolve SVG local FuncIRI when injected in IFrame ("#foo" relative URI with "about:blank" base URI)

Status: RESOLVED WORKSFORME

(Core :: Networking, defect, P3)

Description

[Martijn Vermaat]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:36.0) Gecko/20100101 Firefox/36.0 Iceweasel/36.0.1
Build ID: 20150309005701

Steps to reproduce:

Inject a document into an IFrame element using JavaScript. The document should contain an SVG element with a local FuncIRI reference (e.g., a fill:url(#mask) style value) and the definition that is being referenced.

For an example, see: http://stackoverflow.com/questions/28702218/svg-with-mask-injected-in-iframe-is-broken-in-firefox


Actual results:

The FuncIRI cannot be resolved.

A workaround seems to be prefixing the local IRI with "about:blank" (e.g., a fill:url(about:blank#mask) style value), which seems to be the default baseURI of the IFrame document.


Expected results:

The FuncIRI should be resolved by finding a definition in the IFrame document with the referenced id.

Comment 1

[Martijn Vermaat]

Here's a JSFiddle with some of the possible combinations:

https://jsfiddle.net/4uasecqq/

This bug report is about the "IFrame contentDocument" combination, which I would expect to also show the circle with a pattern fill.

Comment 2

[Boris Zbarsky [:bzbarsky]]

> a fill:url(#mask) style value

So if you do this in an about:blank iframe, that tries to create a relative URI using "about:blank" as a base URI... but the about: protocol handler doesn't allow relative URIs.

Maybe we should have a special case here like we do for data: for cases when the relative URI starts with '#'.

Comment 3

[Daniel Holbert [:dholbert]]

(FWIW, you can work around this in Firefox by explicitly giving the iframe a blank data URI as its "src" attribute, as shown here:
  https://jsfiddle.net/4uasecqq/1/

This doesn't work in Chrome, though; with this tweak, Chrome starts blocking access to the iframe's 'contentDocument' property, because it treats the data URI as being non-same-origin with the outer page, due to the protocol mismatch.)

Comment 4

[Boris Zbarsky [:bzbarsky]]

So here's a question.  Why can't we just hoist the "check for starting with '#' and if so do the clone + setref" logic into the ioservice?  Seems to me like it should just apply to all URI schemes.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Patrick, who'd be the right person to ask about comment 4?

Comment 6

[Patrick McManus [:mcmanus]]

comment 4 makes sense to me.. what does valentin think?

Comment 7

[Valentin Gosu [:valentin] (he/him)]

It sounds like a good plan, but it doesn't seem to work.
nsIOService::NewURI doesn't get called with an about:blank baseURI. I am digging into why that is.

Comment 8

[Valentin Gosu [:valentin] (he/him)]

Actually, nevermind - NewURI did get called for about:blank, but setting the ref failed, because that URI is immutable by default.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Even with that, I bet one issue is that the base URI of an about:blank iframe is not in fact about:blank: it's the base URI of the parent document.  At least in Gecko.  Not sure what other browsers do there, though I _think_ that used to be the case for IE too.

It looks like we made the base URI behavior of about:blank into what it is now in bug 482659.  So there's fun web compat bits involving interaction with document.open/write there too...

Comment 10

[Valentin Gosu [:valentin] (he/him)]

Attachment: about:blank#pattern can't be resolved because the about: protocol is non-relative

Comment 11

[Boris Zbarsky [:bzbarsky]]

What we should really do, given that we're not in a world in which all URIs are immutable and the setters create new objects, is add a cloneWithNewRef or whatever that lets you clone an existing URI but set a different ref on it.

Comment 12

[Valentin Gosu [:valentin] (he/him)]

It seems that the base URI for the <IFrame contentDocument> section in the testcase is about:blank, and the url should resolve to about:blank#pattern. In Chrome it seems to resolve to https://fiddle.jshell.net/4uasecqq/1/show/#pattern

The attached patch fixes this case, but I'm not sure if we want to take it, or make the baseURI not be about:blank anymore. Taking the patch, shouldn't worsen compatibility though.

I also tested in IE. The <IFrame contentDocument> case works, just like in Chrome. However, the <IFrame srcdoc> iframe was completely empty and the <src="data:text/html,"> iframe showed an error page.

Comment 13

[Boris Zbarsky [:bzbarsky]]

> It seems that the base URI for the <IFrame contentDocument> section in the testcase is
> about:blank

That's very surprising.  And https://jsfiddle.net/4uasecqq/2/ disagrees...

Are you sure you didn't mix up Chrome and Firefox in comment 12?

Comment 14

[Valentin Gosu [:valentin] (he/him)]

(In reply to Not doing reviews right now from comment #13)
> > It seems that the base URI for the <IFrame contentDocument> section in the testcase is
> > about:blank
> 
> That's very surprising.  And https://jsfiddle.net/4uasecqq/2/ disagrees...
> 
> Are you sure you didn't mix up Chrome and Firefox in comment 12?

I got Chrome's URL from the devtools style.
For Firefox, from what I can tell, if it weren't about:blank, my patch shouldn't do anything. Or am I missing something?

Comment 15

[Boris Zbarsky [:bzbarsky]]

> I got Chrome's URL from the devtools style.

Interesting.  That's not the base URL Chrome reports for that document, though.

> if it weren't about:blank, my patch shouldn't do anything.

Does it do anything?  I just tried it on the jsfiddle testcase and it doesn't change any behavior there for me.

Comment 16

[Valentin Gosu [:valentin] (he/him)]

(In reply to Not doing reviews right now from comment #15)
> > I got Chrome's URL from the devtools style.
> 
> Interesting.  That's not the base URL Chrome reports for that document,
> though.

Is it possible that the internal baseURI when the HTML gets parsed is different from the one we get afterwards?

> 
> > if it weren't about:blank, my patch shouldn't do anything.
> 
> Does it do anything?  I just tried it on the jsfiddle testcase and it
> doesn't change any behavior there for me.

It works for me. I haven't pushed it to try yet, that test case works with the patch, and doesn't without it.
I'll write up a ref-test based on the jsfiddle and push it to try.

Comment 17

[Boris Zbarsky [:bzbarsky]]

Ah, looks like Chrome lies about its base URIs.  Consider http://jsfiddle.net/n0gdh5y4/3/ which is this testcase:

  <iframe></iframe>
  <script>
    onload = function() {
      var doc = frames[0].document;
      doc.body.innerHTML = '<a href=""></a>'
      alert(doc.baseURI);
      alert( doc.querySelector('a').href);
    }
  </script>

So I checked the spec again.  https://html.spec.whatwg.org/#terminology-2 says:

  The fallback base URL of a Document object is the absolute URL obtained by running these
  substeps:
...
    If the document's address is about:blank, and the Document's browsing context has a
    creator browsing context, then return the document base URL of the creator Document,
    and abort these steps.

So our behavior is correct in terms of making the base URI of the iframe be the URI of the parent document, and some of the tests in should not in fact work.  The ones that should work are "Parent document", "IFrame contentDocument with about:blank", "IFrame contentDocument with baseURI", and "IFrame contentDocument with baseURI and about:blank".  Of those, the only one that doesn't work right now is "IFrame contentDocument with baseURI", and that's what the about:blank base URI discussion is about.

Comment 18

[Boris Zbarsky [:bzbarsky]]

I filed https://code.google.com/p/chromium/issues/detail?id=484930 on the buggy baseURI getter in Chrome.

Comment 19

[Daniele "Mte90" Scasciafratte]

Same problem here http://output.jsbin.com/xixutupimu/ i don't understand how to fix or a workaround, this svg don't have #pattern :-/

Comment 21

[Boris Zbarsky [:bzbarsky]]

Is this still an issue now that bug 1280584 is fixed?  It seems to be, but it's not clear to me why....

At least seems to be an issue based on http://jsbin.com/xixutupimu/1/edit?html,output and https://jsfiddle.net/4uasecqq/ the "IFrame contentDocument" case...

Comment 22

[:Gijs (he/him)]

(In reply to Boris Zbarsky [:bz] from comment #21)
> Is this still an issue now that bug 1280584 is fixed?  It seems to be, but
> it's not clear to me why....
> 
> At least seems to be an issue based on
> http://jsbin.com/xixutupimu/1/edit?html,output and
> https://jsfiddle.net/4uasecqq/ the "IFrame contentDocument" case...

Testing with the jsfiddle, the base URI used to resolve #pattern against is the parent document's, so https://fiddle.jshell.net/4uasecqq/show/ (or my local modified copy's equivalent), rather than about:blank. I assume that explains why this is broken. That's a bug in either our CSS or our SVG code, though, I expect? The path through which we hit NewURI is:

#0	0x00000001030a58d3 in mozilla::net::nsIOService::NewURI(nsACString_internal const&, char const*, nsIURI*, nsIURI**) at /Users/gkruitbosch/dev/firefox-unified/netwerk/base/nsIOService.cpp:631
#1	0x00000001030b32cc in NS_NewURI(nsIURI**, nsACString_internal const&, char const*, nsIURI*, nsIIOService*) at /Users/gkruitbosch/dev/firefox-unified/netwerk/base/nsNetUtilInlines.h:115
#2	0x00000001053d2587 in mozilla::css::URLValueData::GetURI() const at /Users/gkruitbosch/dev/firefox-unified/layout/style/nsCSSValue.cpp:2676
#3	0x000000010544e7f5 in mozilla::FragmentOrURL::SetValue(nsCSSValue const*) [inlined] at /Users/gkruitbosch/dev/firefox-unified/layout/style/nsStyleStruct.cpp:133
#4	0x000000010544e7e6 in SetSVGPaint(nsCSSValue const&, nsStyleSVGPaint const&, nsPresContext*, nsStyleContext*, nsStyleSVGPaint&, nsStyleSVGPaintType, mozilla::RuleNodeCacheConditions&) at /Users/gkruitbosch/dev/firefox-unified/layout/style/nsRuleNode.cpp:9326
#5	0x0000000105434d9c in nsRuleNode::ComputeSVGData(void*, nsRuleData const*, nsStyleContext*, nsRuleNode*, nsRuleNode::RuleDetail, mozilla::RuleNodeCacheConditions) at /Users/gkruitbosch/dev/firefox-unified/layout/style/nsRuleNode.cpp:9421
#6	0x000000010542e0e6 in nsRuleNode::WalkRuleTree(nsStyleStructID, nsStyleContext*) at /Users/gkruitbosch/dev/firefox-unified/layout/style/nsRuleNode.cpp:2541
#7	0x00000001056d5213 in nsStyleSVG const* nsRuleNode::GetStyleSVG<true>(nsStyleContext*, unsigned long long&) [inlined] at /Users/gkruitbosch/dev/builds/opt/dist/include/nsStyleStructList.h:73
#8	0x00000001056d5203 in nsStyleSVG const* nsStyleContext::DoGetStyleSVG<true>() [inlined] at /Users/gkruitbosch/dev/builds/opt/dist/include/nsStyleStructList.h:73
#9	0x00000001056d5203 in nsStyleContext::StyleSVG() [inlined] at /Users/gkruitbosch/dev/builds/opt/dist/include/nsStyleStructList.h:73
#10	0x00000001056d5203 in nsIFrame::StyleSVG() const [inlined] at /Users/gkruitbosch/dev/builds/opt/dist/include/nsStyleStructList.h:73
#11	0x00000001056d5203 in nsSVGUtils::GetGeometryHitTestFlags(nsIFrame*) at /Users/gkruitbosch/dev/firefox-unified/layout/svg/nsSVGUtils.cpp:1618
#12	0x00000001056d2ef1 in nsSVGPathGeometryFrame::ReflowSVG() at /Users/gkruitbosch/dev/firefox-unified/layout/svg/nsSVGPathGeometryFrame.cpp:426
#13	0x00000001056a8d36 in nsSVGDisplayContainerFrame::ReflowSVG() at /Users/gkruitbosch/dev/firefox-unified/layout/svg/nsSVGContainerFrame.cpp:359
#14	0x00000001056a8d36 in nsSVGDisplayContainerFrame::ReflowSVG() at /Users/gkruitbosch/dev/firefox-unified/layout/svg/nsSVGContainerFrame.cpp:359
#15	0x00000001056cf7c8 in nsSVGOuterSVGFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, unsigned int&) at /Users/gkruitbosch/dev/firefox-unified/layout/svg/nsSVGOuterSVGFrame.cpp:453


Is this enough to be helpful? Maybe dholbert can take a look?

Comment 23

[:Gijs (he/him)]

Attachment: Simplified testcase

This is a simplified HTML testcase. If you set a breakpoint in http://searchfox.org/mozilla-central/rev/6536590412ea212c291719d1ed226fae65a0f917/netwerk/base/nsIOService.cpp#634

the codepath that resolves #pattern should be one of only a few hits when loading this doc.

Comment 24

[Boris Zbarsky [:bzbarsky]]

Oh, right.  about:blank inherits base URIs.  This is back to the thing where relative URIs need to be preserved as relative URIs to make stuff work, but the specs very much say to not do that....  :(

Comment 25

[jancsika]

I'm putting together an SVG tutorial that leverages about:blank as a testing environment. Unfortunately, this bug means I cannot use FF in the screencast about patterns and gradients.

Comment 26

[Boris Zbarsky [:bzbarsky]]

Your real issue here is with the spec, not with Firefox...

Comment 27

[jancsika]

"Works in two open source browsers" would certainly be optimal. But "works in Chromium" is certainly enough to keep me from following your ellipses down the HTML5 rabbit hole. :)

Comment 28

[Boris Zbarsky [:bzbarsky]]

If you don't care whether you're writing "SVG" or "ChromiumVG", sure.

Comment 29

[jancsika]

> If you don't care whether you're writing "SVG" or "ChromiumVG", sure.

Like I said, it means I can't leverage Firefox as an about:blank testing environment.  And again-- it's unfortunate, but certainly not going to keep me from using another browser to productively test and create SVGs.

The SVG I create in the about:blank environment of Chromium is valid, cross-platform compatible SVG when used outside the context that triggers this bug. I don't think you meant that it wouldn't be. But your response is ambiguous enough to imply that the SVG generated from what you're calling "ChromiumVG" isn't production-ready, valid SVG when used in a more typical context (or at least that I don't care that it is valid SVG, neither of which must necessarily be true).

Comment 30

[Boris Zbarsky [:bzbarsky]]

I'm saying that per the SVG spec as currently written that SVG should not work in about:blank, that's all.  If you're assuming it will, then you're not actually writing SVG, but something that depends on particular browsers not following the spec in some sort of unspecified ways.

Comment 31

[Firefox Bug Husbandry Bot]

Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258

Comment 32

[Firefox Bug Husbandry Bot]

Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258

Comment 33

[Valentin Gosu [:valentin] (he/him)]

From what I can tell, the testcase works now, right? Can we close this?

Comment 34

[jancsika]

@Valentin: I can confirm it indeed works for the case of SVG FuncIRIs in about:blank.

Comment 35

[Valentin Gosu [:valentin] (he/him)]

Thanks for the update. Closing as WORKSFORME. Probably fixed by bug 1280584 and/or bug 922464.

Comment 36

[mlbli]

Hi!

The issue doesn't seem to be fixed with <animate /> tags. The animation doesn't start in Firefox. Works in Chrome and Safari.

Repro:

<!DOCTYPE html>
<html>
  <head>
    <style type="text/css">
      iframe {
        width: 225px;
        height: 125px;
        border: 1px solid red;
      }
    </style>

    <title></title>

    <script>
    window.onload=function(){
  var svg = '\
  <svg xmlns:xlink="http://www.w3.org/1999/xlink" style="overflow: visible;" height="48" width="48" viewBox="0 0 64 64" xmlns="http://www.w3.org/2000/svg"><g fill="none" stroke="#4CB877"><g><circle id="circle-558014329836164" cx="32" cy="32" opacity="0" r="10" stroke-width="1.5"></circle><animate attributeName="r" dur="200ms" fill="freeze" from="10" to="30" xlink:href="#circle-558014329836164"></animate><animate attributeName="opacity" dur="200ms" fill="freeze" from="0" to="1" xlink:href="#circle-558014329836164"></animate></g><g><path id="left-558014329836164" d="M14.0376525,34.5987974 L14.0376525,34.5987974" stroke-linecap="round" stroke-linejoin="round" stroke-width="3"></path><animate id="left-anim-558014329836164" attributeName="d" begin="200ms" dur="150ms" fill="freeze" from="M14.0376525,34.5987974 L14.0376525,34.5987974" to="M14.0376525,34.5987974 L25.2452965,45.8064415" xlink:href="#left-558014329836164"></animate><path id="right-558014329836164" d="M25.2452965,45.8064415 L25.2452965,45.8064415" opacity="0" stroke-linecap="round" stroke-linejoin="round" stroke-width="3"></path><animate attributeName="d" begin="350ms" dur="150ms" fill="freeze" from="M25.2452965,45.8064415 L25.2452965,45.8064415" to="M25.2452965,45.8064415 L52.3204117,18.7313263" xlink:href="#right-558014329836164"></animate><animate attributeName="opacity" begin="350ms" dur="16ms" fill="freeze" from="0" to="1" xlink:href="#right-558014329836164"></animate></g></g></svg>';

  var div = document.createElement("div");
  div.innerHTML = svg;
  
  document.getElementById('contentDocument').contentDocument.body.appendChild(div);
}
</script>
  </head>

  <body>
    <h1>IFrame contentDocument</h1>
    <iframe id="contentDocument" src="about:blank"></iframe>
  </body>
</html>

Comment 37

[:Gijs (he/him)]

(In reply to mlbli from comment #36)

Hi!

The issue doesn't seem to be fixed with <animate /> tags. The animation doesn't start in Firefox. Works in Chrome and Safari.

Please file a new bug and attach your testcase there. Thank you!

Comment 38

[mlbli]

done in https://bugzilla.mozilla.org/show_bug.cgi?id=1553430