Merge lp:~geoffers/beautifulsoup/html5lib-tests into lp:beautifulsoup

[Geoffrey Sneddon]

This makes TreeBuilderForHtml5lib strictly follow the html5lib API allowing it to be used directly from html5lib; this was done primarily as it allows one to run the html5lib testsuite against the BS4 tree builder (currently failing 216 tests).

The second commit then fixes those 216 tests by fixing foster parenting: appendChild wasn't setting the Element.parent, leading the walking up the tree to find the last table to fail. That then working, it became apparent that insertText with insertBefore=True fails, so that too was fixed.

[Thomas Güttler (hv-tbz-pariv)]

Is there a reason why this nice patch was not integrated yet?

We have the Element.parent in beautifulsoup.

[Leonard Richardson (leonardr)]

Geoffrey,

Thanks for this excellent branch. I've merged it as revision 436. My apologies for not getting to this earlier; my work on Beautiful Soup mainly revolves around the bug tracker and I generally don't check the merge requests.

I don't know if this affects your work on html5lib, but since you filed this merge request I've fixed two bugs in Beautiful Soup that only showed up when the html5lib tree builder was in use. Adding similar tests to the html5lib test suite might be useful in finding errors in other code that uses html5lib.

The test methods I added to verify that my fixes worked are test_reparented_markup_containing_identical_whitespace_nodes and
test_reparented_markup_containing_children, both in tests/test_html5lib.py.

review: Approve

=== modified file 'bs4/builder/_html5lib.py'
--- bs4/builder/_html5lib.py	2015-09-28 23:53:43 +0000
+++ bs4/builder/_html5lib.py	2015-12-08 16:07:14 +0000
@@ -4,6 +4,7 @@
 
 from pdb import set_trace
 import warnings
+import re
 from bs4.builder import (
     PERMISSIVE,
     HTML,
@@ -15,7 +16,10 @@
     whitespace_re,
 )
 import html5lib
-from html5lib.constants import namespaces
+from html5lib.constants import (
+    namespaces,
+    prefixes,
+    )
 from bs4.element import (
     Comment,
     Doctype,
@@ -59,7 +63,7 @@
 
     def create_treebuilder(self, namespaceHTMLElements):
         self.underlying_builder = TreeBuilderForHtml5lib(
-            self.soup, namespaceHTMLElements)
+            namespaceHTMLElements, self.soup)
         return self.underlying_builder
 
     def test_fragment_to_document(self, fragment):
@@ -69,8 +73,12 @@
 
 class TreeBuilderForHtml5lib(html5lib.treebuilders._base.TreeBuilder):
 
-    def __init__(self, soup, namespaceHTMLElements):
-        self.soup = soup
+    def __init__(self, namespaceHTMLElements, soup=None):
+        if soup:
+            self.soup = soup
+        else:
+            from bs4 import BeautifulSoup
+            self.soup = BeautifulSoup("", "html.parser")
         super(TreeBuilderForHtml5lib, self).__init__(namespaceHTMLElements)
 
     def documentClass(self):
@@ -93,7 +101,8 @@
         return TextNode(Comment(data), self.soup)
 
     def fragmentClass(self):
-        self.soup = BeautifulSoup("")
+        from bs4 import BeautifulSoup
+        self.soup = BeautifulSoup("", "html.parser")
         self.soup.name = "[document_fragment]"
         return Element(self.soup, self.soup, None)
 
@@ -107,6 +116,56 @@
     def getFragment(self):
         return html5lib.treebuilders._base.TreeBuilder.getFragment(self).element
 
+    def testSerializer(self, element):
+        from bs4 import BeautifulSoup
+        rv = []
+        doctype_re = re.compile(r'^(.*?)(?: PUBLIC "(.*?)"(?: "(.*?)")?| SYSTEM "(.*?)")?$')
+
+        def serializeElement(element, indent=0):
+            if isinstance(element, BeautifulSoup):
+                pass
+            if isinstance(element, Doctype):
+                m = doctype_re.match(element)
+                if m:
+                    name = m.group(1)
+                    if m.lastindex > 1:
+                        publicId = m.group(2) or ""
+                        systemId = m.group(3) or m.group(4) or ""
+                        rv.append("""|%s<!DOCTYPE %s "%s" "%s">""" %
+                                  (' ' * indent, name, publicId, systemId))
+                    else:
+                        rv.append("|%s<!DOCTYPE %s>" % (' ' * indent, name))
+                else:
+                    rv.append("|%s<!DOCTYPE >" % (' ' * indent,))
+            elif isinstance(element, Comment):
+                rv.append("|%s<!-- %s -->" % (' ' * indent, element))
+            elif isinstance(element, NavigableString):
+                rv.append("|%s\"%s\"" % (' ' * indent, element))
+            else:
+                if element.namespace:
+                    name = "%s %s" % (prefixes[element.namespace],
+                                      element.name)
+                else:
+                    name = element.name
+                rv.append("|%s<%s>" % (' ' * indent, name))
+                if element.attrs:
+                    attributes = []
+                    for name, value in element.attrs.items():
+                        if isinstance(name, NamespacedAttribute):
+                            name = "%s %s" % (prefixes[name.namespace], name.name)
+                        if isinstance(value, list):
+                            value = " ".join(value)
+                        attributes.append((name, value))
+
+                    for name, value in sorted(attributes):
+                        rv.append('|%s%s="%s"' % (' ' * (indent + 2), name, value))
+                indent += 2
+                for child in element.children:
+                    serializeElement(child, indent)
+        serializeElement(element, 0)
+
+        return "\n".join(rv)
+
 class AttrList(object):
     def __init__(self, element):
         self.element = element
@@ -158,8 +217,10 @@
             child = node
         elif node.element.__class__ == NavigableString:
             string_child = child = node.element
+            node.parent = self
         else:
             child = node.element
+            node.parent = self
 
         if not isinstance(child, basestring) and child.parent is not None:
             node.element.extract()
@@ -224,11 +285,11 @@
     attributes = property(getAttributes, setAttributes)
 
     def insertText(self, data, insertBefore=None):
+        text = TextNode(self.soup.new_string(data), self.soup)
         if insertBefore:
-            text = TextNode(self.soup.new_string(data), self.soup)
-            self.insertBefore(data, insertBefore)
+            self.insertBefore(text, insertBefore)
         else:
-            self.appendChild(data)
+            self.appendChild(text)
 
     def insertBefore(self, node, refNode):
         index = self.element.index(refNode.element)

=== modified file 'bs4/tests/test_html5lib.py'
--- bs4/tests/test_html5lib.py	2015-09-28 23:53:43 +0000
+++ bs4/tests/test_html5lib.py	2015-12-08 16:07:14 +0000
@@ -96,3 +96,8 @@
         a1, a2 = soup.find_all('a')
         self.assertEqual(a1, a2)
         assert a1 is not a2
+
+    def test_foster_parenting(self):
+        markup = b"""<table><td></tbody>A"""
+        soup = self.soup(markup)
+        self.assertEqual(u"<body>A<table><tbody><tr><td></td></tr></tbody></table></body>", soup.body.decode())