html_to_vdom breaks on whitespaces and null HTML tags

[Archmonger]

Current Situation

If a HTML string has ...

• Additional whitespace at the tail of the HTML document
• A null (non-terminated) HTML tag such as <hr> within the document

... then html_to_vdom will silently fail without raising an exception

See my Django IDOM PR for how I caused the script tag issue to occur at this LOC.

Proposed Actions

Whitespace issue can be fixed by simple regex and/or using str.strip().

Script tag issue needs further investigation. This might open up a can of worms to test html_to_vdom with all idom.html types.

[Archmonger]

@rmorshea Would like you to chime in on this one too, while you still have availability.

[rmorshea]

I'll take a look at this over the weekend.

[Archmonger]

The issue I categorized as a "script tag" issue appears to be related to HTML doesn't have end tags (such as <hr>) break the HTMLParser implementation that is currently developed. It was just coincidence that my script tag was following a <hr>. I am looking into a solution.

[Archmonger]

Googling and testing shows me there doesn't appear to be a fool proof way of resolving this with python's built-in HtmlParser. It's technically a syntax tree parser and not a DOM tree generator. The combination of handle_starttag, handle_endtag, and handle_startendtag are not robust enough to handle these scenarios without committing some really ugly code.

In the interest of maintainability, I'm going to switch us to using lxml or BeautifulSoup using the lxml backend. A nice benefit of the switch is it technically should be much faster since lxml is C based.

[rmorshea]

That target parser interface of lxml seems like it maps nicely onto what we already do. I wasn't able to find something similar in bs4, but I didn't look that hard.

[Archmonger]

Using that interface would likely make us fall into the same pitfalls, since it doesn't provide access to the DOM tree. I've been testing out performing these transformations recursively.

Seems to work off my rudimentary test, and is a lot more readable.

from io import StringIO
from pprint import pprint
from typing import Union
from lxml import etree

my_html = """
<div id="view_to_component_middleware">
    view_to_component_middleware: Success
    <div class="inner"> this is text </div>
</div>\n

<hr>
<script>
    var dog = 'memes';
</script>"""


def _set_if_val_exists(object, key, value):
    """Sets a key on a dictionary if the value's length is greater than 0."""
    if len(value):
        object[key] = value


def html_to_vdom(html: Union[str, etree._Element], *transforms):
    """Convert an lxml.etree node tree into a VDOM dict."""
    # If the user provided a string, convert it to an lxml.etree node.
    if isinstance(html, str):
        parser = etree.HTMLParser()
        tree = etree.parse(StringIO(html), parser)
        node = tree.getroot()
    elif isinstance(html, etree._Element):
        node = html
    else:
        raise TypeError("html_to_vdom expects a string or lxml.etree._Element")

    # Convert the lxml.etree node to a VDOM dict.
    vdom = {"tagName": node.tag}
    _set_if_val_exists(vdom, "attributes", dict(node.items()))
    _set_if_val_exists(
        vdom, "children", [html_to_vdom(child) for child in node.iterchildren(None)]
    )

    # Apply any transforms.
    for transform in transforms:
        vdom = transform(vdom)

    return vdom


pprint(html_to_vdom(my_html))