mirror of https://github.com/zulip/zulip.git
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.
This commit is contained in:
parent
f6c78a35a4
commit
b02754adec
|
@ -6,7 +6,7 @@ from lxml.html.diff import htmldiff
|
||||||
|
|
||||||
def highlight_html_differences(s1: str, s2: str, msg_id: Optional[int] = None) -> str:
|
def highlight_html_differences(s1: str, s2: str, msg_id: Optional[int] = None) -> str:
|
||||||
retval = htmldiff(s1, s2)
|
retval = htmldiff(s1, s2)
|
||||||
fragment = lxml.html.fromstring(retval)
|
fragment = lxml.html.fragment_fromstring(retval, create_parent=True)
|
||||||
|
|
||||||
for elem in fragment.cssselect("del"):
|
for elem in fragment.cssselect("del"):
|
||||||
elem.tag = "span"
|
elem.tag = "span"
|
||||||
|
|
|
@ -520,10 +520,10 @@ class EditMessageTest(EditMessageTestCase):
|
||||||
self.assertEqual(
|
self.assertEqual(
|
||||||
message_history_1[1]["content_html_diff"],
|
message_history_1[1]["content_html_diff"],
|
||||||
(
|
(
|
||||||
"<p>content "
|
"<div><p>content "
|
||||||
'<span class="highlight_text_inserted">after</span> '
|
'<span class="highlight_text_inserted">after</span> '
|
||||||
'<span class="highlight_text_deleted">before</span>'
|
'<span class="highlight_text_deleted">before</span>'
|
||||||
" edit</p>"
|
" edit</p></div>"
|
||||||
),
|
),
|
||||||
)
|
)
|
||||||
# Check content of message before edit.
|
# Check content of message before edit.
|
||||||
|
@ -571,9 +571,9 @@ class EditMessageTest(EditMessageTestCase):
|
||||||
self.assertEqual(
|
self.assertEqual(
|
||||||
message_history_2[1]["content_html_diff"],
|
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 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(
|
self.assertEqual(
|
||||||
|
@ -581,6 +581,39 @@ class EditMessageTest(EditMessageTestCase):
|
||||||
"<p>content before edit, line 1</p>\n<p>content before edit, line 3</p>",
|
"<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:
|
def test_edit_link(self) -> None:
|
||||||
# Link editing
|
# Link editing
|
||||||
self.login("hamlet")
|
self.login("hamlet")
|
||||||
|
@ -616,11 +649,11 @@ class EditMessageTest(EditMessageTestCase):
|
||||||
self.assertEqual(
|
self.assertEqual(
|
||||||
message_history_1[1]["content_html_diff"],
|
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 "
|
">zulip "
|
||||||
'<span class="highlight_text_inserted"> Link: http://www.zulipchat.com .'
|
'<span class="highlight_text_inserted"> Link: http://www.zulipchat.com .'
|
||||||
'</span> <span class="highlight_text_deleted"> Link: http://www.zulip.org .'
|
'</span> <span class="highlight_text_deleted"> Link: http://www.zulip.org .'
|
||||||
"</span> </a></p>"
|
"</span> </a></p></div>"
|
||||||
),
|
),
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue