Phoenix_live_view: Live View Javascript Bug: Forget tags in rendering

Created on 25 Jul 2019  ·  7Comments  ·  Source: phoenixframework/phoenix_live_view

Environment

  • Elixir version: 1.9.0
  • Phoenix version: 1.4.9
  • NodeJS version: 10.16.0
  • NPM version: 6.9.0
  • Operating system: Ubuntu 18.04 LTS

Actual behavior


I created a table in Live View and also implemented the head as a nested Live View. The code looks like this:

~L"""
            <% displayed_data = @changeset.changes |> Enum.filter(fn {_, val} -> val end) |> Enum.map(fn {key, _} -> key end) %>
            <table class="table">
                <thead>
                    <%= live_render(@socket, Ui.ToolbarLive, container: {:tr, []}, session: assigns) %> 
                </thead>
                <tbody>
                    <%= for entry <- assigns.data do %>
                        <tr>
                            <%= for key <- @menubar do %>
                                <%= if key in displayed_data do %>
                                    <td><%= Map.get(entry, key)%></td>
                                <%end%>
                            <% end %>
                        </tr>
                    <%end%>
                </tbody>
            </table>
        """

The code for the nested Live View looks like this:

 ~L"""
            <% displayed_data = @changeset.changes |> Enum.filter(fn {_, val} -> val end) |> Enum.map(fn {key, _} -> key end) %>
            <%= for entity <- @menubar, entity in displayed_data do %>
            <th scope="col">
                <%= entity%>

                <button class="btn btn-default dropdown-toggle" data-toggle="dropdown" aria-haspopup="true" aria-expanded="false"></button>
                <div class="dropdown-menu">
                    <button class="dropdown-item" phx-click="<%= entity%>-sort-ascending">Sort Ascending</button>
                    <button class="dropdown-item" phx-click="<%= entity%>-sort-descending">Sort Descending</button >
                    <button class="dropdown-item btn btn-default dropdown-toggle" data-toggle="dropdown">Columns</button>

                    <div class="dropdown-menu">
                        <%= form_for @changeset, "#", [phx_change: :activate], fn f -> %>
                            <%= for entry <- @menubar do %>
                                <div class="dropdown-item"><%= checkbox f, entry %><label><%= entry%></label></div>
                            <%end%>
                        <% end %>
                    </div>
                    <div class="dropdown-item">
                        <%= form_for @changeset, "#", [phx_change: :query], fn _ -> %>
                            <%= text_input(:search, entity) %>
                        <% end %>
                    </div>
                </div>
            </th> 
            <%end%>
        """ 

The HTML that Phoenix returns looks like this excerpt:

<div data-phx-session="SFMyNTY..." data-phx-view="Ui.UsersLive" id="phx-k7cfkMm3" class="phx-connected">    
    <table class="table">
        <thead>
            <tr data-phx-parent-id="phx-k7cfkMm3" data-phx-session="SFMyNTY...">    
            id
                <button class="btn btn-default dropdown-toggle" data-toggle="dropdown" aria-haspopup="true" aria-expanded="false"></button>
                <div class="dropdown-menu">...</div>

Expected behavior

The problem with this is that the <th> tags are missing. This leads to the whole UI being moved and not working properly.
The HTML should looks like this instead:

<div data-phx-session="SFMyNTY..." data-phx-view="Ui.UsersLive" id="phx-k7cfkMm3" class="phx-connected">    
    <table class="table">
        <thead>
            <tr data-phx-parent-id="phx-k7cfkMm3" data-phx-session="SFMyNTY...">    

        <th>
            id
                <button class="btn btn-default dropdown-toggle" data-toggle="dropdown" aria-haspopup="true" aria-expanded="false"></button>
                <div class="dropdown-menu">...</div>
        </th>

In Wireshark it turned out that Phoenix sent the right HTML in any case. But in the frontend part some javascript block has modified some HTML. So I disabled the LiveSocket connection and the HTML was displayed correctly.

I've looked into phoenix_live_view.js in connect() Function and it lead me to joinRootViews(). It seems like the joining process of the views causing this misbehavior.

bug

All 7 comments

Can you provide a minimal LV and template that reproduces the issue so we can better track this down? Thanks!

I wonder if this could be applicable to as well. I could also investigate with a minimum reproduction!

@chrismccord , @snewcomer
Here is the example.

@Menkir I found the issue. Luckily your repro made this easy to find :). So thanks for that.

Effectively, we are wrapping your th tags with a div. So a div inside of a tr is invalid according to the spec. The browser is just tossing out your th's in this case and saying you meant to just do this 😆. I'm guessing there are unexpected divs lying around in a lot of apps as well.

We have a few options:

  1. add the defined container of your LV view to the socket and on mount, add to diff with something like Map.put(:diff, :container, container) so we can pull off of it client side and do something with it.

or:

  1. Get the "fromNode"'s tag name (e.g. container found here) and make that the wrapping container as the "toNode" instead of defaulting to "div".

Any thoughts @chrismccord?

We also have another problem in morphdom. We get an html string from Phoenix and we have to convert that into real html. To do so, we leverage createContextualFragment which is a DOM API that will throw out stuff that doesn't have the right "context". Like tr or td without a table. I'll be putting up a fix for this. However, the previous comment also needs a solution.

Here is a PR to fix.
https://github.com/patrick-steele-idem/morphdom/pull/159

@snewcomer thanks for digging into this! I thought the childrenOnly: true option of morphdom would ignore our root div, but I may have misinterpreted the docs. I like the tag name idea because it avoids extra data on the wire, and we should be able to reference it without issue. I think it's simply a matter of:

-   morphdom(container, `<div>${html}</div>`, {
+   morphdom(container, `<${container.tagName}>${html}</${container.tagName}>`, {

right?

You are right about the childrenOnly flag. I'll dig into this 👍. My feeling is that it works correctly but creating a document fragment like this does toss out invalid html (but doesn't require context, which the reference PR fixed). So assuming the diff you have above is the correct answer. I tested on the repro and all works!

x = document.createElement('template')
x.innerHTML = "<div><td>1</td></div>"
log x.content
> <div>​1​</div>​
Was this page helpful?
0 / 5 - 0 ratings