phoenixframework / phoenix_live_view

Rich, real-time user experiences with server-rendered HTML
https://hex.pm/packages/phoenix_live_view
MIT License
6.18k stars 930 forks source link

A stateful LiveComponent that renders an svg <image> breaks the svg #856

Closed axelson closed 4 years ago

axelson commented 4 years ago

Environment

Actual behavior

A stateful LiveComponent that renders an svg <image> ends up in a broken state. The initially rendered svg is fine, but then when the socket connects the svg is re-rendered and the image (and rest of the svg) go away. In the dev tools it appears that that the <image> has no turned into an <img> which seem to then cause the rest of the svg to not be interpreted correctly.

Full reproduction steps are in the repo: https://github.com/axelson/reproduce_live_view_dissapearing

Link to most relevant code: https://github.com/axelson/reproduce_live_view_dissapearing/commit/944ff4c187e2735657d6da5177b9d3d770c21fc3

Expected behavior

The image should continue to display.

Socket log (looks okay to me, although I'm not sure what's up with the "view crashed" error on the initial mount): phoenix-live-view-socket-debug

It is possible that this issue is related to #843 but I see the same issue when using the latest version of morphdom (by using a path dependency for phoenix_live_view, updating the morphdom version in it and updating the path to pheonix_live_view js in my project (although it's possible I did something wrong).

alvises commented 4 years ago

I've digged into this a little bit. As you said the new version of morphdom (2.6.1) doesn't seem to fix it.

I was able to replicate it in a js test I've added to assets/test/view_test.js in my phoenix_live_view fork: https://github.com/alvises/phoenix_live_view/commit/45f1ee9ca21dbef685473b3f02f45bc5949b2932

describe("SVG", () => {

  test("svg image is rendered correctly after join", () => {
    let liveSocket = new LiveSocket("/live", Socket)
    let el = liveViewSvgWithImageDOM()
    let view = new View(el, liveSocket)

    stubChannel(view)

    let joinDiff = {
      "0": {
        "0": 0,
        "s": [
          "<svg>",
          "</svg>"
        ]
      },
      "c": {
        "0": {
          "0": "https://elixir-lang.org/images/logo/logo.png",
          "s": [
            "<image href=\"",
            "\"></image>\n"
          ]
        }
      },
      "s": ["", ""]
    };

    view.onJoin({ rendered: joinDiff });
    expect(view.el.innerHTML.trim()).toBe(`<svg><image href=\"https://elixir-lang.org/images/logo/logo.png\" data-phx-component=\"0\" id=\"container-0-0\"></image></svg>`)
  })

})

I'm still trying to understand what causes this issue...

alvises commented 4 years ago

Something I've noticed and I didn't know (frontend JS is not my daily code, so maybe I'm saying something obvious 😅):

<image> inside <div>

//on the browser
var div = document.createElement("div");
div.innerHTML = `<image href="logo.png"></image>`;
console.log(div);
<div><img href="logo.png"></div>

<image> inside <svg>

var div = document.createElement("div");
div.innerHTML = `<svg version="1.1"><image href="logo.png"></image></svg>`;
console.log(div);
<div><svg version="1.1"><image href="logo.png"></image></svg></div>

it transforms the <image> tag to <img> when <image> is not inside an <svg>, maybe because <image> is an obsolete <img> alias.


And this moves outside the <img>

var div = document.createElement("div");
div.innerHTML = `<main><svg version="1.1"><img href="logo.png"></svg></main>`;
console.log(div);
<div>
  <main>
      <svg version="1.1"></svg>
      <img href="logo.png">
  </main>
</div>

This is what I think it happens: when the <image> LV component element with phx- attributes, it's created alone, not inside svg, so it's converted to <img>. A workaround could be wrap it into an <svg> tag, I've seen it works but honestly I don't know if it's correct to have an <svg> tag inside another svg.

page_live.html.eex

<svg version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" viewBox="0 0 800 640" preserveAspectRatio="xMidYMid meet">
  <!-- If you remove the "id" from this call it runs fine -->
  <%= live_component @socket, HideWeb.MenuLive, id: :menu, href: @href %>
</svg>

menu_live.ex

defmodule HideWeb.MenuLive do
  use Phoenix.LiveComponent

  def render(assigns) do
    ~L"""
    <svg version="1.1">
      <text><%= @id %></text>
      <image href="<%= @href %>" />
    </svg>
    """
  end
end
Screen Shot 2020-05-08 at 03 24 25
alvises commented 4 years ago

It seems ok to embed an svg inside another svg: https://www.w3.org/TR/SVG/struct.html#NewDocument

I've updated the test in my fork to represent this case: https://github.com/alvises/phoenix_live_view/blob/1b4b1887a7ac7d4cc8fa1a48ebe4fc14201a298b/assets/test/view_test.js#L637

axelson commented 4 years ago

Thanks for looking into this @alvises! I have a feeling that you're onto the core issue (the auto-translation of to ), although I don't know enough of the LiveView/Morphdom internals to know how that's triggered (or is the root issue). Maybe this should be raised as an issue on Morphdom?

alvises commented 4 years ago

Ok, I think I've found it. The issue seems to be in the LV js

https://github.com/phoenixframework/phoenix_live_view/blob/d0be2818a92d0af906102be7d72001dbe521c340/assets/js/phoenix_live_view.js#L236

When <image> is the root node of stateful component, LV needs to add some attributes. So it creates a <template> element and sets the "<image href=\"logo.png\"></image>" string to template.innerHTML. What happens is that the browser's JS converts the <image> tag (inside <template>) to an <img>.

let template = document.createElement("template");
template.innerHTML = `<image href="logo.png"></image>`;
console.log(template)
<template>
  <img href="logo.png">
</template>

I think that the way to fix this particular problem (without embedding the image into another svg) would be just using strings instead of a <template> element... but the template element is useful to iterate the child nodes. At the end of the function the template html string is returned.

axelson commented 4 years ago

@alvises thanks again for looking into this! From everything I can tell, you appear to be on the right track. Unfortunately I'm having a bit of a difficult time following all the logic in recursiveCIDToString, so I'm not sure how I'd translate that to a version that operates on strings. Do you have any pointers?

alvises commented 4 years ago

For the little I know about frontend JS, I see that template element has a lot of advantages and it seems to be the right way, in general, to generate the final html. Switching from template element to string concatenation will probably complicate the code. I'm just wondering if it's really worth to switch from template to a manual way, just for this specific problem that could be solved by wrapping the <image> tag with an <svg> tag (I know... it's a workaround... but...)

axelson commented 4 years ago

When you talk about "wrapping the tag with an ", do you mean doing that in the live view js? Or in the generated template?

josevalim commented 4 years ago

Awesome work debugging this @alvises! Unfortunately I don't believe there is a solution to this problem. Even if we use another API to parse the DOM, such as DOMParser, DOMParser expects the input type as argument and we would have to default to "html", meaning we would still convert the <image> to <img>.

The solution here is to wrap the <image> tag in an <svg> tag inside your component. I will write some notes to the docs. Thanks!

axelson commented 4 years ago

Okay makes sense. Thanks for the write up.