From b02754adecc23fe67c32a8945a4a98eaaba8b1bd Mon Sep 17 00:00:00 2001
From: Alex Vandiver <alexmv@zulip.com>
Date: Thu, 30 Sep 2021 15:14:28 -0700
Subject: [PATCH] html_diff: Handle empty differences between empty strings.

`rendered_content` in historical messages may be empty; examining the
history of them may thus require diff'ing two empty strings, which
itself produces an empty string.

Use `lxml.html.fragment_fromstring` to be able to successfully parse
these, rather than 500.

Part of #19559.
---
 zerver/lib/html_diff.py           |  2 +-
 zerver/tests/test_message_edit.py | 45 ++++++++++++++++++++++++++-----
 2 files changed, 40 insertions(+), 7 deletions(-)

diff --git a/zerver/lib/html_diff.py b/zerver/lib/html_diff.py
index d612ee6e51..2c73ee4ec1 100644
--- a/zerver/lib/html_diff.py
+++ b/zerver/lib/html_diff.py
@@ -6,7 +6,7 @@ from lxml.html.diff import htmldiff
 
 def highlight_html_differences(s1: str, s2: str, msg_id: Optional[int] = None) -> str:
     retval = htmldiff(s1, s2)
-    fragment = lxml.html.fromstring(retval)
+    fragment = lxml.html.fragment_fromstring(retval, create_parent=True)
 
     for elem in fragment.cssselect("del"):
         elem.tag = "span"
diff --git a/zerver/tests/test_message_edit.py b/zerver/tests/test_message_edit.py
index ebeeb184a5..c0111e3c6b 100644
--- a/zerver/tests/test_message_edit.py
+++ b/zerver/tests/test_message_edit.py
@@ -520,10 +520,10 @@ class EditMessageTest(EditMessageTestCase):
         self.assertEqual(
             message_history_1[1]["content_html_diff"],
             (
-                "<p>content "
+                "<div><p>content "
                 '<span class="highlight_text_inserted">after</span> '
                 '<span class="highlight_text_deleted">before</span>'
-                " edit</p>"
+                " edit</p></div>"
             ),
         )
         # Check content of message before edit.
@@ -571,9 +571,9 @@ class EditMessageTest(EditMessageTestCase):
         self.assertEqual(
             message_history_2[1]["content_html_diff"],
             (
-                "<p>content before edit, line 1<br> "
+                "<div><p>content before edit, line 1<br> "
                 'content <span class="highlight_text_inserted">after edit, line 2<br> '
-                "content</span> before edit, line 3</p>"
+                "content</span> before edit, line 3</p></div>"
             ),
         )
         self.assertEqual(
@@ -581,6 +581,39 @@ class EditMessageTest(EditMessageTestCase):
             "<p>content before edit, line 1</p>\n<p>content before edit, line 3</p>",
         )
 
+    def test_empty_message_edit(self) -> None:
+        self.login("hamlet")
+        msg_id = self.send_stream_message(
+            self.example_user("hamlet"),
+            "Scotland",
+            topic_name="editing",
+            content="We will edit this to render as empty.",
+        )
+        # Edit that manually to simulate a rendering bug
+        message = Message.objects.get(id=msg_id)
+        message.rendered_content = ""
+        message.save(update_fields=["rendered_content"])
+
+        self.assert_json_success(
+            self.client_patch(
+                "/json/messages/" + str(msg_id),
+                {
+                    "message_id": msg_id,
+                    "content": "We will edit this to also render as empty.",
+                },
+            )
+        )
+        # And again tweak to simulate a rendering bug
+        message = Message.objects.get(id=msg_id)
+        message.rendered_content = ""
+        message.save(update_fields=["rendered_content"])
+
+        history = self.client_get("/json/messages/" + str(msg_id) + "/history")
+        message_history = orjson.loads(history.content)["message_history"]
+        self.assertEqual(message_history[0]["rendered_content"], "")
+        self.assertEqual(message_history[1]["rendered_content"], "")
+        self.assertEqual(message_history[1]["content_html_diff"], "<div></div>")
+
     def test_edit_link(self) -> None:
         # Link editing
         self.login("hamlet")
@@ -616,11 +649,11 @@ class EditMessageTest(EditMessageTestCase):
         self.assertEqual(
             message_history_1[1]["content_html_diff"],
             (
-                '<p>Here is a link to <a href="http://www.zulipchat.com"'
+                '<div><p>Here is a link to <a href="http://www.zulipchat.com"'
                 ">zulip "
                 '<span class="highlight_text_inserted"> Link: http://www.zulipchat.com .'
                 '</span> <span class="highlight_text_deleted"> Link: http://www.zulip.org .'
-                "</span> </a></p>"
+                "</span> </a></p></div>"
             ),
         )