Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Protocol Handler: SVG does not render when used in img HTML tag #54

Closed
lidel opened this issue Aug 7, 2018 · 10 comments
Closed

Protocol Handler: SVG does not render when used in img HTML tag #54

lidel opened this issue Aug 7, 2018 · 10 comments
Assignees

Comments

@lidel
Copy link
Contributor

lidel commented Aug 7, 2018

Summary

SVG does not render in HTML when loaded via <img src=, even when loaded from the same origin.

How to Reproduce

  1. Run demo:protocol from my svg-render-bug-demo branch:
    git clone -b svg-render-bug-demo --depth 1 https://github.com/lidel/libdweb.git
    cd libdweb
    yarn && yarn demo:protocol
    
  2. Open dweb://html/dot.svg directly – it renders just fine. Test SVG comes from commons.
  3. Open dweb://html/, you will see inline <svg> rendering ok but external one loaded via <img src="dweb://html/dot.svg" /> (same origin) failing to load with errors in console
    • If I try to open SVG directly via right click it loads fine again.
  4. Open dweb://foo/, you will see <img src="dweb://html/dot.svg" /> (loaded from different origin) fails to load just like in previous step

Screenshot

screenshot

@Gozala
Copy link
Contributor

Gozala commented Aug 7, 2018

  1. Open dweb://foo/, you will see (loaded from different origin) fails to load just like in previous step

    • What is different, if I try to open SVG directly via right click it does not work and produces additional error in console. Is this the same problem space as Protocol Handler: Cross-protocol Content #52, or should I create a separate issue for it?

No it's different from #52 as in this case they are under same protocol but different origins, created #55 to track that.

@Gozala
Copy link
Contributor

Gozala commented Aug 7, 2018

Here is what I see observing the output produced:

  • Request goes across to a handler

    { 
       "type":"request",
       "requestID":"dweb:2:Agent@2#58998",
       "url":"dweb://html/dot.svg",
       "scheme":"dweb"
    }
  • Head of the response makes it across to the content process

     {"type":"head","requestID":"dweb:2:Agent@2#58998", "head":{}}
  • Request listener is started listener.onStartRequest(this, context) with default content metadata:

    {
      "contentType": "application/x-unknown-content-type",
      "contentLength": -1,
      "contentCharset": "utf-8"
    }
  • Then response Body makes it over to the content process and nsIArrayBufferInputStream is created.

  • Now what's strange part is that cancel with code 2152988677 is called on the corresponding channel / request and that happens before previous call is does listener.onDataAvailable, which makes me wonder if something is spawning a nested event loop there.

  • Then the NS_BINDING_ABORTED is forwarded to the protocol handler

  • Protocol handler responds with {"type": "end"} message

  • End message makes it's way to content process channel but attempt to loadGroup.removeRequest throws exception.

  • Only after all this it seems that listener.onDataAvailable is called but it throws exception NS_BINDING_ABORTED, presumably because listener.onStopRequest got invoked by End message that somehow managed to happen before.

I'll continue digging through this to better determine what is going on.

@Gozala
Copy link
Contributor

Gozala commented Aug 7, 2018

Looking further into this it appears that maybe call to listener.onDataAvailable triggers a cancel() call.

@Gozala
Copy link
Contributor

Gozala commented Aug 7, 2018

Turns if contentType: "image/svg+xml" is used things work as expected. Presumably default contentType: "application/x-unknown-content-type" causes this behavior.

@lidel
Copy link
Contributor Author

lidel commented Aug 7, 2018

Thanks for looking into this.
IIUC, basically if we solve #56 then this should be a non-issue?

@Gozala
Copy link
Contributor

Gozala commented Aug 8, 2018

Thanks for looking into this.
IIUC, basically if we solve #56 then this should be a non-issue?

I have resolved #56 which should provide a way to address #56 as you pointed out. That being said I'd still keep this open as I would like to:

  1. Find out how same html with embedded image would've behaved if contentType was not set by the server. Ideally protocol behavior should match exactly that.
  2. Find out if we can get same behavior as with file:/// protocol.

@Gozala
Copy link
Contributor

Gozala commented Aug 8, 2018

I did preliminary digging into file: protocol implementation and it seems to be handled very differently, it seems to use nsIFileChannel which does not extend nsIChannel so I presume that code path for file: is very different and not applicable to this use case.

@Gozala
Copy link
Contributor

Gozala commented Aug 8, 2018

It looks like if headers are omitted in HTTP server firefox correctly sniffs the content type, here is the code I used for testing:

const http = require('http')

const server = http.createServer((request, response) => {
  response.writeHead(200)
  if (request.url.endsWith('dot.svg')) {
    response.end(`<?xml version="1.0" encoding="UTF-8"?>
    <!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">
    <svg version="1.1" xmlns="http://www.w3.org/2000/svg" x="0px" y="0px" width="100px" height="150px" viewBox="0 0 10 15" enable-background="new 0 0 10 15" xml:space="preserve">
    <g>
      <path fill="#FFFFFF" d="M5,11c-1.929,0-3.5-1.571-3.5-3.5S3.071,4,5,4s3.5,1.571,3.5,3.5S6.929,11,5,11z"/>
      <path fill="#808080" d="M5,4.588c1.609,0,2.912,1.303,2.912,2.912S6.609,10.412,5,10.412S2.088,9.109,2.088,7.5 S3.391,4.588,5,4.588 M5,3C2.519,3,0.5,5.019,0.5,7.5S2.519,12,5,12s4.5-2.019,4.5-4.5S7.481,3,5,3L5,3z"/>
    </g>
    </svg>`)
  } else {
    response.end(`<h1>HTML</h1><p><img src="dot.svg" alt="dot.svg" border="1" style="width:100px;height:150px;" /></p>`)
  }
})

server.listen(8090)

Digging through the nsIHttpChannel implementation it appears that it first checks the Content-Type headers and if not present it uses sniffs through the content to figure out and set appropriate contentType. Which is pretty much what we are proposing to do here.

@Gozala
Copy link
Contributor

Gozala commented Aug 8, 2018

I end up landing #60 to which does more or less what nsIHttpChannel does. Which is if response does not contain contentType it will use nsIContentSniffer from first data chunk as it arrives.

@lidel With this change your original example seems to work as expected. If I recall correctly you also mentioned to me in person that there was some issue with text file / or maybe markdown file mime type detection. I would expect this to be handled by this change as well.

@lidel
Copy link
Contributor Author

lidel commented Aug 13, 2018

Ack, #60 fixed rendering of SVGs:

slrvlyd

Regarding Markdown: it opens fine if content type is set to text/plain.
If set to text/markdown browser tries to download it instead of rendering, but the same is true for http://, so no issue here, I think.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants